2

I am trying to learn a bit about optimizing my jQuery code...

Is there a way to improve the code below so that the variable $selected is declared outside of the function, but still accessible, so that the DOM is not traversed every time?

Or is this code about as optimized as it could be?

Or... I suppose I may be misunderstanding how and when jquery DOM traversal happens.

$('#full-width-layout_c1_col-1-1_1').on(
    'mouseenter mouseleave click', 
    'a.project_open, a.song_open', 
    function(e) {
        var $selected = $(this).closest('tr').find('div');
        if (e.type == 'mouseenter') {
            $selected.addClass("hovered");
        }
        else if (e.type == 'mouseleave'){
            $selected.removeClass("hovered");
        }
        else if (e.type == 'click'){
            $selected.addClass('opened');
        }
    }
);
Improve this question
3
  • sorry I've overlooked performance optimization requirement. I've removed my answer Commented Nov 18, 2011 at 12:02
  • 1
    The js code is looking good, further optimizations (potentially) would require you to post the HTML itself. Commented Nov 18, 2011 at 12:04
  • The HTML is all dynamically generated via a jquery template and fed with json data creating a table... which is why when i hover I need to traverse up to the specific row then down again the div and add a class to it. I guess this really is as far as I can go with optimization unless the .data method turns out to be a winner. Commented Nov 18, 2011 at 12:11

6 Answers 6

Reset to default
2

Don't use jQuery.data() if you want every last bit of performance when storing arbitrary data in the DOM elements. That would perform much better:

var $selected = this.closestdiv;

if(!$selected) { // first time
    $selected = $(this).closest('tr').find('div');
    this.closestdiv = $selected;
}

From this simple benchmark http://jsperf.com/jquerydata/4 you can see how faster it is to store data directly on DOM rather then using jQuery.data(). On my machine using jQuery.data() is 97% slower than storing data directly. Again, fair word of warning - jQuery.data() has certain amount of overhead because it's trying to be smart about things and prevent potential memory leaks.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Indeed, I'm experiencing it being 45 times as fast! Good to know.
.data is 98% slower for me, so plain storing is in fact 4900% faster (50 times faster).
Fantastic stuff WTK... Many thanks for this.. It's really opened my eyes! Huge props to both you and pimvdb for getting me to a solution that will work fantasticll throughout my app and not just in the example code I gave. Sorry pimvdb but I have the give the 'tick' to WTK for this. Cheers both of you!
2

You can use some sort of caching - when an element is clicked, store $selected as data to the element, so that any next times $selected is fetched from the element, instead of by doing a DOM traversal:

var $selected = $(this).data("closestdiv");

if(!$selected) { // first time
    $selected = $(this).closest('tr').find('div');
    $(this).data("closestdiv", $selected);
}
Improve this answer

6 Comments

Would be good, except that jQuery.data() is horribly slow when compared to storing directly on DOM nodes. On the other hand, there's reason to it - preventing memory leaks.
At first glance this looks great... So, just to clarify, using the above technique I can assign the data on hover, mousenter and click... Then when the event takes place again, it doesn't have to traverse the DOM, making repeat hovers/clicks faster? If I am understanding this correctly it is exactly what I am looking for.
@gordyr: Yes, you're correct. Note though that when you change the DOM, this is not reflected since simply no DOM traversals are done anymore after the first time. Plus, according to @WTK .data is still slow, but I'd have to test out what weighs more.
@gordyr: OK well caching is about 50% faster on Chrome: jsperf.com/using-cache-to-avoid-dom-traversal.
To backup my point about data being slow: jsperf.com/jquerydata/4 If you're not doing something weird with the divs that would trigger memory leaks, check my answer for even better performing solution.
|
0

Since you're selecting on an ID, and I assume that you have only one element with this ID, you could have this outside your event handler:

var $selected = $('#full-width-layout_c1_col-1-1_1').closest('tr').find('div');

This also assumes that no DOM manipulation will be going on that might change what is returned by the above statement.

EDIT: Never mind. I misread. Sorry.

Improve this answer

Comments

0

Without seeing your HTML it is difficult to be sure, but I suspect that you can't look up which DIV to modify because it's different for each anchor that is clicked on. That being the case, you need to traverse the DOM from the clicked/hovered element each time.

Improve this answer

1 Comment

You're entirely correct, my apologies i should have been more speciic in my question
0

This is not the answer, but there is little opportunity to reduce the code:

$('#full-width-layout_c1_col-1-1_1').on(
    'mouseenter mouseleave click', 
    'a.project_open, a.song_open', 
    function(e) {
        var $selected = $(this).closest('tr').find('div');
        if (e.type === 'click'){
            $selected.addClass('opened');
        } else {
            $selected.toggleClass('hovered');
    }
);
Improve this answer

Comments

0

You could use a closure:

(function () {//scoping function
    var $selected = $(this).closest('tr').find('div');

    $('#full-width-layout_c1_col-1-1_1').on(
    'mouseenter mouseleave click', 
    'a.project_open, a.song_open', 
    function(e) {
        if (e.type == 'mouseenter') {
            $selected.addClass("hovered");
        }
        else if (e.type == 'mouseleave'){
            $selected.removeClass("hovered");
        }
        else if (e.type == 'click'){
            $selected.addClass('opened');
        }
    }
    );


}()); //end scoping function

This would make the search for the div happen only once.

Improve this answer

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.