jQuery and closure.

Question

I have a multiple menu's on my page which all use the same mouseover and click events, so I decided to get it into a function. However vars seem to always be assigned to the last of the arguments for the hover(function, function) function.

$(document).ready( function() {
menuMouseOver = function() {
    for(i=0, u=arguments.length; i<u; i++){
        var parent = arguments[i].parent;
        var active = arguments[i].active;
        var childSelect = arguments[i].childSelect;
        console.log(active); //logs the correct active
            $(parent).children(childSelect)
                .not('.'+active).each( function(i, e) {console.log(active);})
 //The above console.log logs the correct active 
                    .hover( function() {
                            console.log(active); //this one always logs menu2_active
                            $(this).addClass(active);
                        }, function() {
                            $(this).removeClass(active);
                        });
    }
}
menuMouseOver( { parent: '#menu1',
                active: 'menu1_active',
                childSelect: ':gt(0)'},
            { parent: '#menu2',
                active: 'menu2_active',
                childSelect: ':gt(0)'});
});

Why is is that the last console.log will always logs the last active instead of the one that belongs to arguments[i].active. (In this example it always logs the active of arguments[1].active) What am I doing wrong?

Also, the real function is more complex, but the problem is also present in this variant.

Answer 1

Your problem is that the hover event occurs outside of the scope of your executing method. So by the time the hover executes your active variable has already gone through the entire set and rests at the active state of your last element. So you are seeing this problem because that last log is an event, which is out of scope, and the other two are in-scope in the loops.

Try this:

        $(parent).children(childSelect)
            .not('.'+active).each( function(i, e) {
                console.log(active);
                $(this).data("active", active);
            })
            .hover( function() {
                $(this).addClass($(this).data("active"));
            }, function() {
                $(this).removeClass($(this).data("active")));
            });

This will actually store the 'active' value inside of the DOM element so that it can be accessed in scope.

Answer 2

I'm racking my brains because it's a weird issue but I refactored the function a bit, might be useful (oh, someone much cleverer answered in the meantime):

$("#menu1,#menu2").each(function(){
    var id = $(this).attr("id");
    $(">li",this).not("."+id+"_active,:eq(0)").hover(function(){
        $(this).addClass(id+"_active");
    },function(){
        $(this).removeClass(id+"_active");
    });
});

Answer 3

JavaScript doesn't have block scope, so those variables you declare in the for loop have their values changed each iteration and all those functions reference the same variables. The trick is to create a new function scope within the for loop so that the variables you declare are bound during that iteration.

You can accomplish this by executing an anonymous function inside the loop:

menuMouseOver = function() {
    for(i=0, u=arguments.length; i<u; i++){
      (function(){ // anonymous function to create new scope
        var parent = arguments[i].parent;
        var active = arguments[i].active;
        var childSelect = arguments[i].childSelect;
        console.log(active); //logs the correct active
            $(parent).children(childSelect)
                .not('.'+active).each( function(i, e) {console.log(active);})
 //The above console.log logs the correct active 
                    .hover( function() {
                            console.log(active); //this one always logs menu2_active
                            $(this).addClass(active);
                        }, function() {
                            $(this).removeClass(active);
                        });
       })(); // execute the anonymous function
    }
}

The way you had it before, all of you functions closed over the same variable references, and so used what ever the last value was, not the value of when the function was created. Using the function scope will have it behave as you intended.