Closures affecting variables in a closed function within a loop

Question

this.config = {
    source: psource,
    _events: [
        'value1',
        'value2',
        'value3'
    ]
};

// Add callbacks to source
var that = this;
for (var i = this.config._events.length - 1; i >= 0; i--) {
    var name = this.config._events[i];
    console.log(name); // correct

    $(this.config.source).on(name, function() {
        console.log(name); // value1
        console.log(that.config._events[i]); // undefined
    });
}

I can't see what is wrong here. I removed all the complicated versions and put in the simplest, it just doesn't want to work at all. The first console.log correctly outputs all the correct names, but it acts like the loop happens all at once, then does it again for the inner console.log's.

Can anyone see what's wrong?

The statement "Closures are affecting everything, not just this" isn't describing a problem - that's how closures work. All variables in the enclosing scope are included in the closure. — nrabinowitz, Mar 15 '12 at 18:25
Please modify the title of the question with something more descriptive. I think you should mention the term `for loop` — viebel, Mar 15 '12 at 18:37
duplicate with http://stackoverflow.com/questions/2192348/closures-in-a-for-loop — viebel, Mar 16 '12 at 10:56

score 2 · Accepted Answer · answered Mar 15 '12 at 18:23

2

In that block

console.log(that.config._events[i]); // undefined

i would end up being -1 every time your closure is called.

You would have to do something of the sort in order to create a closure around i

$(this.config.source).on(name, function(i) { return function() {
        console.log(name); // value1
        console.log(that.config._events[i]); // undefined
    };
}(i) );

answered Mar 15 '12 at 18:23

Damp

3,328
20
19

Even better, pass `i` as an argument to the `.on` method, and access the variable using `event.i` . For more details on my suggestion, see the documentation of `.on()`: http://api.jquery.com/on/ – Rob W Mar 15 '12 at 18:26
@RobW This would probably work in this situation since i is a numeric primitive. However, I deliberately chose this solution so that the op can understand that keeping a reference to a variable in the closure does not equate to keeping a reference to the current value of it at the time of the closure creation. – Damp Mar 15 '12 at 18:31

viebel · Answer 2 · 2012-03-15T18:45:38.393

-1

In Javascript, it is not recommended to define a function inside a for loop.

Instead, You should use a javascript lib that provides each e.g. underscore. Then your code will look like this:

    _.each(this.config._events, function(e) {
       $(this.config.source).on(name, function() {
          console.log(e);
       });

You might want to reverse the array before.

Here is the doc for _.each

You can also use jQuery's $.each which provides a similar interface.

edited Mar 15 '12 at 18:45

answered Mar 15 '12 at 18:34

viebel

19,372
10
49
83

1

-1 It is perfectly safe to define a function inside a `for` loop. You just have to be aware of variable scope and how closures work. – Damp Mar 15 '12 at 18:38
1

I replaced `not safe` by `dangerous`. This leads to a lot of confusion. In fact, `jslint` doesn't allow it. Please vote up back – viebel Mar 15 '12 at 18:40
1

nothing dangerous about it either. It works just as it should. I stand by my -1. That `jslint` does not allow it is their choice, not a javascript issue. – Damp Mar 15 '12 at 18:44

Closures affecting variables in a closed function within a loop

2 Answers2