Refactoring Advice? -- ES6: Create Strings using Template Literals

JavaScript
#1

My solution for the ES6 String Literals exercise is below. I’ve separated errorClassWarn and failureItems.push() into two separate operations.

Though this was a bit of a kludge (and I struggled a little), I’m curious, at this stage of our knowlege, if it is better practice to keep both operations separate for the sake of code readability, or to refactor into one operation, ie:

failureItemes.push(`<li class=“text-warning”>${result.failure[i]}</li>`);

My solution


const result = {
success: ["max-length", "no-amd", "prefer-arrow-functions"],
failure: ["no-var", "var-on-top", "linebreak"],
skipped: ["no-extra-semi", "no-dup-keys"]
};
function makeList(arr) {

// Only change code below this line

let failureItems = [];
let errorClassWarn = [];

for (let i = 0; i < result.failure.length; i++) {
  let errorClassWarn = `<li class="text-warning">${result.failure[i]}</li>`
  failureItems.push(errorClassWarn);
  // Only change code above this line
  };

return failureItems;
}

const failuresList = makeList(result.failure);

console.log(`failuresList: ${failuresList}`);

Your browser information:

User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 11_1_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.96 Safari/537.36.

Challenge: Create Strings using Template Literals

Link to the challenge:

#2

When I started coding (gather around kids, grandpa is going to tell a story) everything was a balance between speed and memory. Bother were very expensive and you had to constantly think about the two. Now (for the vast majority of things) speed and memory are cheap and not a factor. Now there are other things to think about: readability, security, accessibility, maintainability, scalability, etc.

So, yeah, readability is an important thing and is something worth thinking about. It’s good that you’re already thinking about it. Now, to the point, which is better?

  let errorClassWarn = `<li class="text-warning">${result.failure[i]}</li>`
  failureItems.push(errorClassWarn);

or

  failureItemes.push(`<li class=“text-warning”>${result.failure[i]}</li>`);

I had to do a lot of code review in my last job, a lot of junior developers. I would often make comments about readability. To be honest, I could live with either of these. I would probably lean towards the second one because I find it concise but still readable. Concision can add to readability but if taken too far, it can hurt it. But to me, this doesn’t go too far. But if the wind is Southerly, I might end up righting the first, especially if there was a lot of code around that made that look jumbled. But really, imho, they are both fine.

4 Likes
#3

It’s also about context. Here, you can see the object and arrays, the properties are descriptive, and so is the class used on the HTML. So it’s fairly clear what is happening which makes using intermediate variables less useful.

Sometimes using intermediate variables can be very helpful in communicating the context of the code. But then the variable name also has to be good because otherwise it just does more harm than good. To this point, I would maybe question your choice of the variable name.

1 Like
#4

@kevinSmith “Concision can add to readability but if taken too far, it can hurt it.” This is where I am, I suppose, in my journey—admittedly very near the beginning. Features like arrow functions make a lot of sense in practice, but I’m not yet at the stage where, at a glance, I’m able to quickly grok what’s going on. I thank you for your feedback; my primary goal right now, as I progress, is to try and build good habits with the aforementioned readability, maintainaility, etc, factors in mind.

@lasjorg That makes sense. I suppose in the moment I was thinking of those intermediate variables as a throwaway step, limited by scope, meant to pass the warning along while still maintaining some transparency to the operations.

Considering “the variable name also has to be good because otherwise it just does more harm than good,” I can see why it would be preferable to treat the variable name better—with less deference to it’s throwaway nature—to further support my idea of readability.

#5

Right, but that just comes from exposure. A standard function was confusing before you learned about it and used it several times.

1 Like
#6

Yeah, variable naming is a very important subject. If you name them right and your code is structured correctly, the code will tell a story and be much easier to read and maintain.

#7

Unless you plan to alter the content of the variable, so an intermediate “step”, an intermediate variable is really just for the reader of the code. It can help signal the intent and help explain the value of the right-hand side of the assignment.

There are times where it is not at all immediately clear what is happening on the right-hand side but the variable name makes it clear. You might not understand what is happening, but you know what should be happening because of the name of the variable.

1 Like