Very strange behavior with assigning methods in a loop

Question

Consider this code below:

<a href="javascript:void(-1)" id="a1">a1</a>
<a href="javascript:void(-1)" id="a2">a2</a>

<script type="text/javascript">
    var buttons = []
    buttons.push("a1")
    buttons.push("a2")
    var actions = []
    for (var i in buttons)
    {
        actions[buttons[i]] = function() { alert(i) }
    }

    var elements = document.getElementsByTagName("a")
    for (var k = 0; k < elements.length; k++)
    {
        elements[k].onclick = actions[elements[k].id]
    }
    
</script>

Basically, it shows two anchors, a1 and a2, and I expect to see "1" and "2" popping up in an alert when clicking on corresponding anchor. It doesn't happen, I get "2" when clicking on either. After spending an hour meditating on the code, I decided that it probably happens because dynamic onclick methods for both anchors keep the last value of "i". So I changed that loop to:

for (var i in buttons)
{
    var local_i = i.toString()
    actions[buttons[i]] = function() { alert(local_i) }
}

hoping that each dynamic function will get its own copy of "i" with immediate value. But after this change I get "1" popping up when I click on either link.

What am I doing wrong? It's a huge show-stopper for me.

Possible dupes that I have answered: http://stackoverflow.com/questions/1552941 http://stackoverflow.com/questions/1579978/ — Christian C. Salvadó, Nov 04 '09 at 19:51
+1 Exactly the same problem as those you answered in those two posts — Andrey, Nov 04 '09 at 19:57

score 3 · Accepted Answer · answered Nov 04 '09 at 19:41

3

The last value is stored, you can use closures for this:

<a  href="#">blah</a><br>
<a  href="#">blah</a><br>
<a  href="#">foo</a><br>

<script>
    (function() {
    var anchors = document.getElementsByTagName('a');
    for ( var i = anchors.length; i--; ) {
        var link = anchors[i];
        (function(i) {
            link.onclick = function() {
                alert(i)
            }
        })(i);
    }
    })();
</script>

This solution binds the i to the function scope, the key trick is the executing of the function inside of the loop, otherwise you are left with the end result of iterating through and alerting the last value of i.

Reference: http://www.jibbering.com/faq/faq_notes/closures.html

answered Nov 04 '09 at 19:41

meder omuraliev

183,342
71
393
434

But I don't understand, why it doesn't bind local_i to the function scope, as I tried in the second example? – Andrey Nov 04 '09 at 19:43
1

You need to execute a function inside of the loop in order for this to work. – meder omuraliev Nov 04 '09 at 19:45
1

@Andrey - because Javascript does not have block level scoping, only function level scoping. When you declare a variable in a function, using var, it's scope to that function, even if it is inside a for loop. By doing what meder has above, you create a new scope. – Matt Nov 04 '09 at 19:48

score 2 · Answer 2 · answered Nov 04 '09 at 19:42

2

This blog post explains the problem pretty well. The thing is that loops doesn't have their own variable scopes in JavaScript, so the inner function is using the scope of the parent function.

answered Nov 04 '09 at 19:42

Lukáš Lalinský

40,587
6
104
126

Gumbo · Answer 3 · 2009-11-04T22:20:24.090

-1

Don’t use for (… in …) for arrays. Use the simple for loop instead:

for (var i=0; i<buttons.length; ++i) {
    actions[buttons[i]] = (function(i) {
        return function() {
            alert(i);
        };
    })(i);
}

edited Nov 04 '09 at 22:20

answered Nov 04 '09 at 19:40

Gumbo

643,351
109
780
844

But the point remains. +1. It could have been a comment though. – Chetan S Nov 04 '09 at 20:05
How is it a +1? How "for (var i = 0 ...)" is better than "for (var i in ...)"? For me, it's all syntax sugar - am I wrong? – Andrey Nov 04 '09 at 20:09
3

@Andrey: The `for...in ` statement is meant to be used to iterate *object properties*, the problem with this statement is that it crawls up the prototype chain, and if the `Array.prototype` has been extended, those extensions will be iterated also (if you don't use hasOwnProperty) and also, this statement doesn't ensure anyhow the order of iteration. – Christian C. Salvadó Nov 04 '09 at 20:31
I see your point. Unfortunately it does't let me remove my down vote - "vote is too old". – Andrey Nov 04 '09 at 20:55

Very strange behavior with assigning methods in a loop

3 Answers3