1
\$\begingroup\$

I have this code:

var submit = document.getElementById('submit');
var error = document.getElementById('error');
var url;

submit.onclick = function(){
    url = document.getElementById('url').value;
    if(url == ''){
        error.style.display = 'block';
        return false;
    }   
};

Now, it's very simple in functionality. But I'm wondering if caching the error selector, for example, is worth caching or not? is it a perf hit to cache a selector if its only accessed once?

\$\endgroup\$

3 Answers 3

2
\$\begingroup\$

My general rule of thumb is to cache a selector lookup within a given function so you only look it up once for each function invocation, but to not cache it globally.

If something was being called hundreds or thousands of times in a loop, you would want to cache it outside the loop just for common sense efficiency.

Caching things globally is pretty much never needed for user event types of things because the performance of looking up a few objects in the DOM is super fast when compared to any sort of user action time. In other words, you wouldn't be able to tell the difference so you might as well go with the most maintainable and reliable code which avoids unnecessary globals.

\$\endgroup\$
2
  • \$\begingroup\$ I agree about unnecessary globals. When coding I try to make use of closures to avoid having globals hanging around after variables are used if they are no longer required or accessed. \$\endgroup\$
    – Travis J
    Commented May 23, 2012 at 23:10
  • \$\begingroup\$ @TravisJ - One can certainly use closures to avoid globals. Since I work on a lot of dynamic web pages (objects created and removed as the user interacts with the page), I find it cleaner and better not to persistently cache things (beyond the lifetime of the specific operation) that might be dynamic unless a performance issue specifically calls for a cached object. \$\endgroup\$
    – jfriend00
    Commented May 23, 2012 at 23:30
0
\$\begingroup\$

No. It only adds up after about 10,000 iterations. That is when you will see a savings from caching it. For a single selection you will be fine without caching. In the case of your code, it is definitely more readable to have the definition of error in there though.

Either way, caching or not, with a single selection there will be basically no difference, we are talking nanoseconds.

\$\endgroup\$
0
\$\begingroup\$

If you only access the element once, then yes, you have a real performance hit due to the added lookup time by keeping the variable out of local scope. However, you may improve perceived performance this way since the lookup is being done at some other time then when clicking the button (not good if done at load time).

All in all though, the element access is so quick I doubt anyone will even notice. Save it for when you access the same element many times and can tell the effect of the cache.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.