CODEWITHSUNDEEP
javascript
Asperger
Asperger

Reputation: 3222

Best practice with caching. Avoid redundant caching?

Ok this question might seem a bit basic but I wonder if I should cache variables in functions like these:

function fooBar(target) {
  var elem = target.children[0].children[1];

  if(n == 1) {
    elem.style.color = "red";
  }
  else {
    elem.style.color = "blue";
  }
}

vs

function fooBar(target) {   
  if(n == 1) {
    target.children[0].children[1].style.color = "red";
  }
  else {
    target.children[0].children[1].style.color = "blue";
  }
}

There is no real performance gain is there? I assume apart from type safety the latter is better since I need less lines. What exactly is considered best practice in cases like these? Should I still cache the object eventhough its not needed?

So unless my if statements included:

if(elem.className == "something")

I personaly wouldnt bother caching.

At the other hand my brain is in conflict with coding style / consistency.

Assuming I have something like this:

function fooBar(target) {   
  if(n == 1) {
    target.children[0].children[1].style.color = "red";
  }
  if else (n == 2) {
    target.children[0].children[1].style.color = "blue";
  }
  if else (n == 3) {
    target.children[0].children[1].style.color = "yellow";
  }
  else {
    target.children[0].children[1].style.color = "green";
  }
}

Then I would have to cache the object due to typesafety which brings me back to the issue of consistency...

Upvotes: 2

Views: 134

Answers (3)

RomanPerekhrest
RomanPerekhrest

Reputation: 92854

What exactly is considered best practice in cases like these?

The "best practice" in "such" cases is to eliminate read(access) operations upon array/object.
In your case you have 4 read operations for both two variants.
- To avoid multiple read/access operations you should save the crucial element(reference) into a local variable
- To avoid multiple if else statements - use switch operator instead(it should go faster)
You should also consider the code readability and code simplicity.
But if you need "the less lines" - I would suggest the following simple and scalable solution for your last example:

function fooBar(target) {
  var styles = ["green", "red", "blue", "yellow"];  // of course, if "n" increases consecutively (can be also transformed into object)  
  target.children[0].children[1].style.color = styles[n] || styles[0];
}

Upvotes: 2

Ivan Yarych
Ivan Yarych

Reputation: 2073

The issue here is not performance, but readability. It is often a good practice to perform assignments that let you or someone else read code easier and avoid making a mistake in future.

I personally would use a shortcut even if that variable is used only once, like this:

function fooBar(target) {
    var element = target.children[0].children[1];

    element.style.color = "red";
}

It may also help to give the variable meaningful name if you know what will be stored there. For example: bodyElement, dropdownElement or shortcuts like bodyEl, dropdownEl and so on.

Upvotes: 0

Daniel Gruszczyk
Daniel Gruszczyk

Reputation: 5612

It really depends on practices in your workplace. Or if it is for your private projects, on what you like.
I personally don't like repeating myself, especially with a long lines of code, and so I would go with the first approach.
The first approach also gives you a benefit of changing just one line of code, if at some point in the future you need to change different variable. In the second approach a bad developer might change one line and leave the other (albeit provided a good suit of tests this should not matter as you would expect tests to fail).
I would say: If you are accessing the same deeply nested variable in multiple places, make your life easier and protect yourself against silly errors by assigning that deeply nested variable to a local variable with a good descriptive name. At the end of the day elementColor is better than target.children[0].children[1].style.color.

Upvotes: 2

Related Questions