Javascript function doesn't receive an argument correctly

Question

Possible Duplicate:
JavaScript closures and variable scope
Assign click handlers in for loop

I have this script:

var MyClass = {
    MyArray: new Array(0, 1, 2, 3, 4),

    MyFunc1: function() {
        var i = 0;
        for (i = MyClass.MyArray.length - 1; i>=0; i--) {
            var cen = document.getElementById("cen_" + i); // It is an img element
                cen.src = "col.png";
                cen.className = "cen_act";
                cen.onclick = function() { MyClass.MyFunc1(i); };
            } else {
                cen.src = "no.png";
                cen.className = "cen";
                cen.onclick = null;
            }
        }
    },

    MyFunc2: function(id) {
        alert(id);
    }
}

My problem is that, at this line :cen.onclick = function() { MyClass.MyFunc1(i); }; the argument sent to MyFunc2 is always -1. The MyFunc1 function should create four images, each one with an onclick event. When you click on each image, the MyFunc2 function should show the corresponding i value. It looks like the i value is not "saved" for each event and image element created, but only its "pointer".

Thanks!

possible duplicate of [JavaScript closures and variable scope](http://stackoverflow.com/questions/2314175/javascript-closures-and-variable-scope) -- this kind of question has been asked a million times, but I understand that it is not easy to search for it if you don't know what the problem is. Also see [this comment](http://stackoverflow.com/questions/2314175/javascript-closures-and-variable-scope#comment2281960_2314175). — Felix Kling, May 17 '12 at 14:35

score 2 · Answer 1 · edited May 23 '17 at 11:55

You should be familiar with the concept of JavaScript closures to understand why this happens. If you are, then you should remember that every instance of the

function() { MyClass.MyFunc1(i); };

function closure contains i's value of -1 (since it is the final value of this variable after the entire loop finishes executing.) To avoid this, you might either use bind:

cen.onclick = (function(i) { MyClass.MyFunc1(i); }).bind(null, i);

or use an explicitly created closure with the proper i value.

score 1 · Accepted Answer · edited May 23 '17 at 10:09

1

It's a normal case and misunderstand of closures, see this thread and you may get some clue, the simply way to fix this problem is to wrap your for loop body with an Immediate Invoked Function Expression

MyFunc1: function() {
    var i = 0;
    for (i = MyClass.MyArray.length - 1; i>=0; i--) {
        (function(i) {
            var cen = document.getElementById("cen_" + i); // An img element
                cen.src = "col.png";
                cen.className = "cen_act";
                cen.onclick = function() { MyClass.MyFunc2(i); };
            } else {
                cen.src = "no.png";
                cen.className = "cen";
                cen.onclick = null;
            }
        }(i));
    }
}

edited May 23 '17 at 10:09

Community

1
1

answered May 17 '12 at 14:36

otakustay

11,817
4
39
43

I didn't down-vote because this isn't wrong, but probably not the best solution to the problem due to the performance hit from constantly adding to the scope stack by executing an IIFE every iteration through the loop. – kalisjoshua May 17 '12 at 14:42
Sometimes, I forget that JavaScript is not C++, either PHP. Thanks to everyone. I will accept your answer, since it is the first one. – ali May 17 '12 at 14:43

score 1 · Answer 3 · answered May 17 '12 at 14:37

You are capturing a variable that changes inside the loop, so you always get the last value of i.

You can easily fix that by creating a closure:

MyFunc1: function() {
    var i = 0;
    for (i = MyClass.MyArray.length - 1; i>=0; i--) {
        (function(i) {

        var cen = document.getElementById("cen_" + i); // An img element
            cen.src = "col.png";
            cen.className = "cen_act";
            cen.onclick = function() { MyClass.MyFunc2(i); };
        } else {
            cen.src = "no.png";
            cen.className = "cen";
            cen.onclick = null;
        }
       })(i);
    }
},

Javascript function doesn't receive an argument correctly

3 Answers3