I’m coming up on the two-month mark for my course of studies with The Odin Project, and upon finishing (for now) one of the projects, I decided to run it past my old friend James Ducker , who currently works as a Lead Engineer in Sydney. Being friends, I knew I could expect honesty from him, and his feedback was both flattering and helpful.
Aside from giving me a stern nudge about creating my README file (oops!), I was given three pieces of feedback which I think would be useful to many other junior and student developers out there.
1. Apply styling via CSS classes and avoid applying styling via JavaScript
Considering myself a bit of a clever chook, I had written a simple getColor function which would read the priority level of a todo element, and return a colour code, which I could then apply as a border and as a text colour to elements. I had read about ‘keeping styling separate’ but, being a beginner, that slipped my mind and I went merrily on my way, calling my little function whenever a todo was created, and whenever the priority level was updated by the user.
Let’s see how James put it:
"Good practice is to separate your visual concerns from your logic…Make some classes in CSS with names like .urgent {}, .important{}, etc….then use JS to add/remove those classes."
Exactly! Rather than fiddling around with using JavaScript to remove inline style and then replace it with something else, simply put all possible styles into your CSS stylesheet and access them at will. But wait, he offered me one more nugget which could make this even sexier…
"You get an opportunity for a really nice little optimisation – if you make your class names match the value of your priority [selector], then you don’t even need to think about them, you just say something like todoBox.addClass(priorityDropdownValue)"
In other words, we now only need two variables: the todo element we want to style, and the priority level name. Compare this to my code, which required no fewer than four customised elements: three functions (getColor, getWording), and two variables (the todo element and the priority level). Needless to say, following this principle would have saved me a lot of lines of code, and is an easy fix I plan to implement in future.
Later in our talks, he cycled back to this point about utilising values and added a nice practical tip which I will be implementing from now on, especially in the planning stages:
"whenever you write down a value (like a priority for instance) think how can I keep leveraging this little piece of information I’ve added, to avoid having to write code"
Those last six words are said by many different people in many different ways, but hearing it from someone at James’ level and put so directly, really helped me as a novice to comprehend exactly how ‘laziness’ can actually be ‘efficiency’ when it comes to programming, bringing us smoothly into his second major tip…
2. If you need to type something more than once, consider creating a helper function or a loop to do the work for you
I already knew about the DRY principle (“Do not repeat yourself” for those unfamiliar), but despite this my newbie butt went and wrote this piece of code. I’ve shortened it for the sake of this post, but I actually listed 20 (!) different elements:
const projectTitle = document.getElementById("projecttitle")
const todoContainer = document.getElementById("todocontainer")
const projectContainer = document.getElementById("projectcontainer")
const sortPriorityHighLow = document.getElementById("sortpriorityhighlow")
const sortPriorityLowHigh = document.getElementById("sortprioritylowhigh")
const sortSelect = document.getElementById("sort")
let addNewProjectButton
let addNewTodoButton
export {
projectTitle,
todoContainer,
projectContainer,
sortPriorityHighLow,
sortPriorityLowHigh,
sortSelect,
addNewProjectButton,
addNewTodoButton,
}
Funnily enough, I had written a helper function on a previous project which was a similar idea but for creating elements. If I’m being honest, I felt a bit ridiculous about trying to avoid such an easy piece of code, so didn’t implement something similar for DOM selectors on this project.
My little helper (used in an example restaurant page) looked like this:
function makeElement(elementType, id, textContent, parentNode) {
const newElement = document.createElement(elementType)
newElement.id = id;
newElement.textContent = textContent;
parentNode.appendChild(newElement);
return newElement
}
As you can see, I not only built the ability to create the element, but also to add text immediately, also returning the element created so I could put it to work elsewhere if need be.
So rather than exporting all those global variables (which, in the first place, I should have known to avoid doing since we don’t want to pollute the global namespace), I could have exported a little friend like this one, and just called it as needed through my code:
export function getEl(newConstantName) {
const lowercaseConstantName = newConstantName.toLowerCase();
const foundElement = document.getElementById(lowercaseConstantName);
return foundElement;
}
Alternatively, since the above doesn’t save a lot of typing, you could write a ‘DOM update’ function which loops through all the elements of a given document/window/parent and assigns them to previously declared variables. Truth be told, this isn’t a great example because the original .getElementById() function is already pretty streamlined, but it does serve as an example of something bigger. I already mentioned DRY, but I want to use James’ words again to put it another way…
"There are only three numbers in programming: 0, 1, and infinity … Do something 1 time, but soon as you’re doing it more than that it’s infinity and you switch to writing a loop"
In other words: the moment you sense that you’re going to have to type this all over again, start thinking “how can I be lazy about this?”. I previously wrote about the Open-Closed Principle and how all modules should be open to extension. In a similar vein, consider how your code can be written to ensure that you could expand it as easily as possible. If you combine this with the single-responsibility principle, then you should find it easy to use your existing code in new and interesting ways should you ever want to add new pieces to the puzzle.
3. Rather than calling on document for DOM manipulation, consider calling on the parent node instead
If you’ve made it this far, then you’ve earned this little nugget of gold. “A super common scenario in web dev,” says James, “is that you’re developing some module and it sits on a page alongside other modules”. Suddenly, your query selectors could be applicable to a whole range of things you had no intention of affecting, and document.getElementsByClassName("navitem") isn’t looking too hot! Although document.getDocumentById() is “only available as a method on the global document object” (MDN), querySelector(), querySelectorAll(), and getElementsByClassName() can all be called on parent elements or module containers instead. This little tidbit may not be as sexy as structuring code and optimising laziness, but it could help you avoid a lot of questionmarks down the line when you’re suddenly finding that your ‘perfect’ app is wreaking havoc in its natural environment.
