Hacker News Reader

Comment by emehrkay | Hacker News Reader

People at my job call all JavaScript jQuery, it is so annoying. Even more annoying is that their code (jQuery or not) plain sucks

View on HN

When I use jQuery (MooTools is my lib of choice) I find myself using it in ways that is very unlike the majority of jQuery that is seen around the web.

I absolutely hate the plugin system simply because you cannot easily get the instance of the plugin an refer to it later. To answer this I use function constructors with the module pattern (its easy and doesnt require an extra lib to get going) to create my plugins. Another thing that I do is when I query for a collection of objects, create an array that represents every item in that collection already wrapped with the jQuery object. This may not seem like a big deal, but there is a difference between

    var collection = jQuery('a');
    collection.each(function(i, l){
        var link = jQuery(l);
        link.bind('click', function(){//do suff});
    });

and

    var collection = jQuery('a'),
        collected = (function(){
            var c = [];
            $.each(collection, function(i, item){
                c.push(jQuery(item));
            });
            return c;
        })();

this second way allows you to select an item from collected without having to rewrap it with the jquery object. I dont have any data on it, but it seems like rerunning jQuery() with every mouseover/mouseleave/event etc seems like a waste of processing.

Anyway, that is just a few ways that I use jQuery to make javascript a bit easier. These little tricks have the people that I work with thinking that I'm some sort of genius.

Isn't that second code snippet actually creating a jQuery object for each "a" - even if it is never clicked on? Isn't that rather more of a "waste of processing" than the first, where a jQuery object is created in the event handler?

Why wouldn't you write the first one as:

   $("a").click(function() { // do stuff });

if you need to do stuff with a inside of the function, then you have to do $(this), so every time an a in that collection is clicked $() is run.

In my first example I actually took that problem out of the equation by looping through the collection and storing the jQuery'd a as a var accessible via the event closure. However, if you want to reference an element in that collection later in your code, you have to jump though the jQuery selection hoops.

But you are right, sometimes you may not need to have every element in your selection wrapped in the jQuery object and it is up to you to determine which solution to use in a given case. But I do feel that most cases I see devs constantly wrapping $(this) inside of event handlers when they could have made an external reference/collection before hand and used that.

    $('a').hover(
        function(e){
           $(this).doSomething();
        },
        function(e){
           $(this).doSomething();
        }
    );

    var as = $('a'),
        collected //using the same method as above;

    as.each(function(i, e){
       collected[i].hover(
            function(e){
                collected[i].doSomething();
            },
            function(e){
               collected[i].doSomething();
            }
      );
    );

This is a very rudimentary example and I know that there is a simpler way to accomplish this, but imagine code with multiple collected items who all match up on a 1 to 1 basis collecting the pre-wrapped objects has its advantages.

But wouldn't multiple potentially long loops on initial load have a greater possible impact on user experience than one extra call to $() per event?

Great point. I think that my simplistic example is just that, simplistic. A better solution for huge collections would be to memoize the retrieval of the $() obj with something like

    var memd = {},
        getMem = function(obj){
           var o;
           if(memd[obj]){
              o = memd[obj];
           }else{
              o = memd[obj] = $(obj);
            }
           return o;
         };

and call that function instead of calling $() while inside of an event handler (I havent tested this code, but the concept is straight forward)

I don't want to appear snarky, but as you admit yourself ("I dont have any data on it") you don't know if creating a jQuery object is expensive or not (to be fair, I don't know either) - so why all the extra complexity if you don't know whether the thing you are trying to avoid (creating instance of jQuery during event handlers) actually causes problems or not?

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

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

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

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.

I guess it's just bit of cpu for memory trade-off

>>allows you to select an item from collected without having to rewrap it with the jquery object

I think the following does the same thing:

    var collected = $('a').map(function(element) {return $(element)})

Or even simpler:

    var collected = $('a').map($)

However, map as defined on jQuery objects takes a function that takes the index as the first argument, and the element as the second argument:

http://api.jquery.com/map/

So, what you actually want is:

    var collected = $('a').map(function(index, element) {return $(element)})

However, jQuery.map has the element as the first argument, and the index as the second argument:

http://api.jquery.com/jQuery.map/

I would consider this to be the more intuitive way round, although I'd prefer it if both maps were consistent. So, you could write:

    var collected = $.map($('a'), $)

which strikes me as the neatest version.

Nice, I didnt know that jQuery had map. Thanks.

Cant edit, but my second argument, creating an array containing already jQuery wrapped elements is null.

There is a method called .eq() that allows you to select from the collection and it returns the element wrapped for you.

I left a job where I'd built a simple UI to a complex back-end using jQuery. A former co-worker told me the guy who'd taken over that project kept complaining that he didn't understand "all this JSON" in the app.

Friend: "Can I see some of the JSON?" In-over-his-head Developer: "Sure, here." Friend: "That's not JSON, that's jQuery." Dev: "Whatever, it's the same thing." Friend: "No it's not and you thinking it is, is the real problem here."

Reply on news.ycombinator.com