loading...

Discussion on: What is your most versatile do-anything bit of code?

Collapse
traaidmark profile image
Adrian Kirsten Author

Hey Oleg,

Thanks for the response man. I don't mind the criticism at all, this is after all dev explicit space and I for one am on here to learn. I didn't expect the comment at all though, and now I kind of wish I used a different, more safe example to my question!

That snippet of code is at least 4 years old now, and was written when I was new to programming as a whole, nevermind javascript (I come from a design background). It has simply always worked for me since then, and I saw little need to change it since then.

That's not justification at all, I simply mention it because if I were to read your comment a year ago I would not have understood much of what you just said, probably deleted my account and hid under a couch for a week!

If you don't mind I would like to respond to your points, explain the thinking process and decisions I made, and I would hate for you to see them as arrogant or confrontational. I am genuinely interested in your opinions.

I think my response is too long, so breaking it up in bits.

Collapse
traaidmark profile image
Adrian Kirsten Author

Coding methodology and directions

If I click element A I want to add a class to element B. That's basically all this script does.

I have three problems to solve with this script:

1. The script shouldn't run on all links on a page, only the ones I wanted to attach the feature to.

Way back when I originally wrote this the only way I knew how to solve the first problem was to add a class to the link I want to the script to listen on. If I recall correctly, this was fairly common practise?

I decided that class should be c-reveal because I named this script Reveal.

2. I have to figure out how to target element B without having to declare it in js.

I figured using the html anchor method would be quite cool. The user clicks on something and goes to an element in a way, and I was able to get that anchor into myscript and match it to element B.

3. I need to be able to have multiple features on a single page using this script.

I didn't want to have the script as a function that I would have to call for every feature. I wanted it to just sit there and wait for a click and then toggle out a class.

The only way I knew how to achieve this was to use a global class. Global css is awful yes, but it was a compromise that up to today I've managed to deal with. lol.


Thinking about this now, I guess I could do something like:

<button data-reveal="#hidden-div">show the hidden div</button>
<div class="feature" data-reveal-class="feature--show" id="hidden-div">This div's default state is hidden through .feature.</div>

This removes any global classes, but still allows me to target it in a reusable way. The script would then look for the data-reveal rather than the class.

Does this make more sense to you?

You will end up writing a lot of similar css code per feature, but I guess you can mitigate a lot of that with a sass mixin or something.


To be honest, this is the first time in a long time I look at that code. It just always worked so why fix a working thing amirite?! Lol.

On that note

While this was so not the type of response I expected I do appreciate that you took the time to do it man.

At the very least it's given me a bit of a push to go relook at quite a few other snippets and rework them.

If you would be kind enough to respond to my thoughts above it would be awesome, even if it's just to continue the discussion!

I'm not going to change my original post, because that would make this thread irellevant, and I think it's important to keep it in tact.

That said: Your most versatile bit of code could also be your most versatile module, plugin or other similar form of $thing! Mind sharing yours?

Thread Thread
oleggromov profile image
Oleg Gromov

Adrian,

Now I feel obligated to explain myself. I got used to a criticizing manner of giving feedback because of my last team leader job - now I guess it might be not the best way to encourage people for changes because critics simply hurt. I didn't mean to do that to you at all; so I highly appreciate your thorough response. Without any doubts, you're one hell of a great developer, and I hope my reasoning only helps. Being able to perceive things from another standpoint is an incredibly rare skill, keep it and you'll benefit from it much more than the criticism might ever hurt you.

I'd like to make myself as clear as possible. The step-by-step answer is following below.

1. The script shouldn't run on all links on a page, only the ones I wanted to attach the feature to.
Way back when I originally wrote this the only way I knew how to solve the first problem was to add a class to the link I want to the script to listen on. If I recall correctly, this was fairly common practise?
I decided that class should be c-reveal because I named this script Reveal.

I get your idea and I was writing code like that for a long time. I might even write it now under certain circumstances like, for example, supporting a website that has been made long ago. And yes, I can confirm that it was a fairly common practice.

Your point number 1 is absolutely valid. However, the approach of hooking javascript logic to CSS classes has at least one downside. Even though you explicitly define the purpose of the elements in HTML by setting these classes, you still need an implicitly required javascript snippet. These are two points of failure: you can either forget about the class or get the script lost for some reason like changing the head/footer part of your template where all the dependencies are included via <script>. What I'd suggest is a reduction of this excessive construction to a single plugin/function invocation in JavaScript with all required hooks (classes, data attributes or whatever else) specified. Again, this particular example is too simple to realize how severely it might affect the future debugging process or feasibility of altering the functionality or appearance.

2. I have to figure out how to target element B without having to declare it in js.
I figured using the html anchor method would be quite cool. The user clicks on something and goes to an element in a way, and I was able to get that anchor into myscript and match it to element B.

Given that you've accepted the approach of reducing your plugin invocation to a one, supposedly javascript, point, you might just wrap both of your elements into another one and target your script to it. Like this:

<div class="revealer">
    <span>Click me</span>
    <div><!-- something being hidden and shown --></div>
</div>

In that case, you have a whole bunch of options of how to link elements to each other without using an anchor, which is in my opinion much better.

3. I need to be able to have multiple features on a single page using this script.
I didn't want to have the script as a function that I would have to call for every feature. I wanted it to just sit there and wait for a click and then toggle out a class.
The only way I knew how to achieve this was to use a global class. Global css is awful yes, but it was a compromise that up to today I've managed to deal with. lol.

Again, you still have to invoke your script by including it into a page and running unconditionally even in case you don't need it. If you could get rid of this step I'd accept the point, otherwise, you just increase the complexity of the solution (that will also affect the ability to comprehend what's going on) by making part of it covert.

While this was so not the type of response I expected I do appreciate that you took the time to do it man.
[...cut...]
That said: Your most versatile bit of code could also be your most versatile module, plugin or other similar form of $thing! Mind sharing yours?

I guess I was too precise in listing my points and failed to convey the main idea. The concept of having copy-and-pastable pieces of code without any boundaries (i.e. lacking isolation/incapsulation), which drift from project to project, looks absolutely wrong to me. As I wrote before, if I had a reusable piece of code I'd rather make it a module/plugin/component - with documentation, versioning, proper isolation and interface than just carry it around as a bunch of js, css and obscure conventions about class names or anything else.

An example might be a simple PubSub module I used in my previous project. It's not related to DOM manipulations at all but even if it was, I wouldn't change the way I shaped it. Another example might be a detect-media function from my last hobby project. If I need it somewhere else I will just make an npm-module (or a module of any other type), publish it somewhere and then reuse.

For sure, this approach requires more infrastructure that your project might not have at the moment. However, even with just plain scripts attached to a page the of carrying useful stuff around can be much neater and apparent.