Comment by emehrkay | Hacker News Reader

Comment by emehrkay | Hacker News Reader

Parent Full thread

Well given that a lot of the "pro" jQuery tips say that one should avoid rerunning the selector in favor of some sort of memoization:

    $('.class').doSomehting();
    $('.class').doSomethingElse();
    $('.class').thirdThing();

in favor of

    var ele = $('.class');
    ele.doSomething();
    ele.doSomethingElse();
    ele.thirdThing();

I would only assume that re-querying with the jQuery object inside of event handlers has the same adverse effects.

Lets say you have a very simple tabbed thing

    //this is code that i've seen around
    var tabs = $('a.tab'),
        containers = $('div.container');
        
    tabs.click(function(e){
        var index = tabs.indexOf($(this));
        tabs.removeClass('active');
        $(this).addClass('active');
        containers.css('display', 'none');
        $(containers[index]).css('display', 'block'); 
    });

vs //this is how id handle a simple tabber var tabs = $('a.tab'), containers = $('div.container'), all_containers = $.map(containers, function(i, c){ return $(c); });

    $.each(all_tabs, function(i, tab){
        var t = $(tab);
        
        t.bind('click', function(e){
            tabs.removeClass('active');
            t.addClass('active');
            containers.css('display', 'none');
            all_containers[i].css('display', 'block');
        });
    });

I just feel that the first one, while more concise (has it obvious areas of improvement, but general idea) would wind up being more expensive than the second.

If running the jQuery object isnt too expensive, why is the practice of rerunning the same selector frowned upon?

Interacting with the DOM is what's slow. It's especially slow if the native querySelectorAll method isn't available because any non-trivial selections necessitate iterating over the DOM tree. That's why you don't want to re-query a selection multiple times.

That's also why it's recommended that all selections start with an id. The size of the tree to iterate over can be quickly reduced using the native getElementByID method.

In the case of $(this), the DOM node is already passed to the event handler, so wrapping it in a jQuery object doesn't touch the DOM tree at all. Furthermore, I always cache it (var $this = $(this);) if I'm going to use it more than once.

Also, in your first example, you don't have to rewrap containers[index] in a jQuery object. All the elements in the selection are already wrapped.

> Also, in your first example, you don't have to rewrap containers[index] in a jQuery object. All the elements in the selection are already wrapped.

Really? I think that is the whole point of this long comment thread -- that the elements in a collection arent already wrapped.

http://jsfiddle.net/8nqtA/

This doesnt work unless i wrap a[i] in $()

But like you're saying, it may be fruitless since the dom is the bottleneck and we're bypassing it by already having the element.

Well there's a few problems with that example:

1) You've got a "global" named a and then a local variable (via the argument) inside the each function called "a"...so that really confuses things.

2) a[i] where a is a jQuery object returns an unwrapped DOM element...what you want is .index().

3) What you're doing is sort of roundabout...

Why not:

    var d = $('div'),
        a = $('a');

    a.each(function(i, el){
         d.text(d.text() +' '+ $(this).text() );
    });

Or if you really want to do it your way:

    var d = $('div'),
        a = $('a');

    a.each(function(i, el){
         d.text(d.text() +' '+ a.index(i).text() );
    });

Shouldn't the second one be:

    var d = $('div'),
        a = $('a');

    a.each(function(i, el){
         d.text(d.text() +' '+ a.eq(i).text() );
    });

Using eq(i) rather than index(i)?

I feel like this should have been the first replay to my rant. The nodes are already wrapped in the jQuery object, just need to use eq to access them by index. I've never seen that before, thanks

you can also pull these out as standard DOM objects if you access as an array, ie. $('a')[0]

They actually aren't already wrapped. The .eq() method pulls an element out of the collection and wraps it in a new jQuery object.

Ah yeah, I forgot you had to use .eq().

The selector engine runs right to left so even if you start with an id but end it with an element it will still be an expensive query. I think the key is to think about your selectors and not go overboard because it's easy.

>why is the practice of rerunning the same selector frowned upon?

Rerunning the same selector is frowned upon for (at least) 2 reasons:

1) It can make for more readable code.

Sometimes it makes more sense semantically to have a local variable that describes the role of an element in the particular block of code you are working on, rather than just what selector you are using to get at the element.

Also, it makes the code shorter and less complex, in your example "tabs" is shorter and easier to read than "$('a.tabs')" etc.

2) ...because it's a stupid simple optimization to make.

This goes in general for ANY JS variable that is not in local scope, is the property of an object, or is returned as the result of a function.

It's so easy to just cache it as a local variable, you should probably just do that once you are accessing it a few times.

Even that has the whiff of premature optimization, but it's so easy and has such a low impact (or even improvement) on readability, that it's no big deal.

What you're talking about is much harder to justify imho.

Re-running the same selector is an instance where you are re-running a function that will definitely return the same result that you just got.

Effectively it's the same as

    function get5(){ return 5; };
    var x = 4 + get5();
    var y = 35 + get5();

...etc etc...so obviously it's better to just cache the return value and save the work of running a function.

It's hard to say because your examples aren't entirely clear to me, but I don't think your version as is is any better, perhaps even worse in some respects.

$('a.tab') returns A jQuery object that has a context that is a collection of DOM elements.

Your map function (which has the arguments reversed fyi) return's an array of jQuery objects (plural); each with a context of a single DOM element.

So you're creating a bunch of new instances of jQuery objects to possibly save an insignificant amount of context lookup time (getting the context of a jQuery object is not as heavy as selector lookup).

Again in the each you're creating a new jQuery object for each tab element, even though it was already in one. A jQuery object which will stick around via the closure.

So this is all to save looking up the index each click event for the tab (btw, something like $(this).index() would probably be cleaner) which may or may not be worth it.

I can see where you're going, and think you've got the right idea, but I also think what you use really depends on the situation.

I would say just write clean and idiomatic code first...that should be the default.... and then if you hit problems you can start optimizing based on the situation.

If looking up the index and context each click really is a problem (could be in some situations) you could find a solution based on the situation using strategies like event delegation, strategic naming, caching the index, associating them in a data structure of some kind, of any combination thereof.

Thanks for the reply, I get what you're saying and agree to an extent, the question was rhetorical though.

> Your map function (which has the arguments reversed fyi)

Oh god, jquery is php.

Well, I think I'm going to spend some time tomorrow looking at the jQuery source code to find out what really does happen but what I suspect is that using a selector is going to be a lot more expensive than creating a jQuery instance directly from an element - presumably using a selector means querying the DOM for matching elements (which could be quite complex) rather than simply wrapping a supplied element in a new jQuery object.

So if you do have some complex selector then I can see that avoiding re-running it makes a lot of sense - for DRY as much as efficiency. However, in an event handler you've been handed the element you want to work with and all you are doing is wrapping it in jQuery to make it easier to work with.

Or something like that :-)

Re-running a selector and wrapping an element in the jQuery object aren't even close to the same thing:

https://github.com/jquery/jquery/blob/master/src/core.js#L67

Wrapping elements in event handlers is completely fine.

Reply on news.ycombinator.com