1

Possible Duplicate:
Javascript closure inside loops - simple practical example

I have an array of 4 objects (that.pairs), and each object has a .t property which is a jQuery object/element. I'm trying to set an event on each t being clicked.

The problem is that when one of the them gets clicked, it's always the last pair (index 3) that gets passed into my doToggle() function.

Why is this happening? How can I fix it?

for (var i = 0; i < that.pairs.length; i++) {
    var p = that.pairs[i];
    p.t.click(function() {
        that.doToggle(p);
    });
}
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Steve Robbins
  • 13,672
  • 12
  • 76
  • 124
  • 1
    I don't think this question deserves a down vote, it is well explained. I know this has been asked a million times, but how are you supposed to know what to search for? – Ruan Mendes Nov 29 '12 at 00:33
  • @JuanMendes: Eh, I don't know about that. It has been asked so many times that any generic phrasing of the problem will probably yield the right answer. For example, "javascript event loop function always last pair" "javascript loop always last item" "javascript callback last element" "javascript loop same value" — heck, even just "javascript loop problem". It is hard *not* to find the answer in a search. – Chuck Nov 29 '12 at 00:38
  • @Chuck mmm... I searched for `Losing Scope of Array on Click Event Loop` and the third result was relevant... so you do have point, but it's not as obvious as you claim. I'm still not sure the OP could have figured it out if they are kind of clueless about closures. I reserve downvotes for serious problems, like not including any code, not showing any effort, not showing error messages, saying "it doesn't work"... – Ruan Mendes Nov 29 '12 at 01:00

4 Answers4

4

It's because the p variable is shared by your closures, there's just one p variable. By the time your handlers are called, p has changed.

You have to use a technique I call freezing your closures

for (var i = 0; i < that.pairs.length; i++) {
    // The extra function call creates a separate closure for each
    // iteration of the loop
    (function(p){
        p.t.click(function() {
            that.doToggle(p);
        });
    })(that.pairs[i]); //passing the variable to freeze, creating a new closure
}

A easier to understand way to accomplish this is the following

function createHandler(that, p) {
    return function() {
       that.doToggle(p);
    }
}

for (var i = 0; i < that.pairs.length; i++) {
    var p = that.pairs[i];
    // Because we're calling a function that returns the handler
    // a new closure is created that keeps the current value of that and p
    p.t.click(createHandler(that, p));
}

Closures Optimization

Since there was a lot of talk about what a closure is in the comments, I decided to put up these two screen shots that show that closures get optimized and only the required variables are enclosed

This example http://jsfiddle.net/TnGxJ/2/ shows how only a is enclosed Closures without eval

In this example http://jsfiddle.net/TnGxJ/1/, since there's an eval, all the variables are enclosed. Closures with eval

Ruan Mendes
  • 90,375
  • 31
  • 153
  • 217
  • Every function in JavaScript binds its execution context to its outer execution context. This makes it a closure, whether or not you actually reference the available variables. All JavaScript functions are closures. That said, Juan is right. There's the execution context of the IIFE *(the outer)*, and then there's the execution context of each invocation of the handler. The context of each handler invocation is permanently bound to that of the IIFE. – I Hate Lazy Nov 29 '12 at 00:48
  • @zerkms: That's just invalid syntax, but if you give it a name, its execution context is bound to the global execution context. – I Hate Lazy Nov 29 '12 at 00:50
  • JuanMendes: functions that aren't part of an expression require a name, so `function(){}` gives a SyntaxError. – I Hate Lazy Nov 29 '12 at 00:56
  • @user1689607 If you actually look at google chrome's closure stack, you'll notice that a variable is only available to inspect if it's been used in the closure, so the compiler actually optimizes the closure. If the function contains an `eval` though, all the variables are enclosed since the compiler can't guarantee that only some variables are needed. See the screenshots in my answer – Ruan Mendes Nov 30 '12 at 02:46
3

Use $.each instead of a for loop so that you get a new variable scope with each iteration.

$.each(that.pairs, function(i, p) {
    p.t.click(function() {
        that.doToggle(p);
    });
});

This way each click handler closes over a unique variable scope instead of the shared outer variable scope.

I Hate Lazy
  • 47,415
  • 13
  • 86
  • 77
1
for (var i = 0; i < that.pairs.length; i++) {
    var p = that.pairs[i];
    (function(p){
        p.t.click(function() {
            that.doToggle(p);
        });
    }(p));
}

This trick with IIFE would solve the closure "issue" you're experiencing now.

zerkms
  • 249,484
  • 69
  • 436
  • 539
1
for (var i = 0; i < that.pairs.length; i++) {
    (function(num){
       var p = that.pairs[num];
       p.t.click(function() {
          that.doToggle(p);
       });
    })(i)
}

Classic closure issue

Enclose them in an anonymous function and assign the current iteration in context. That should solve the problem..

Sushanth --
  • 55,259
  • 9
  • 66
  • 105