Javascript - use variable in a function, within a function

Question

this code was bothering me the whole day. Lets say ledCount = 9, the code gets the elements by ID without any problem, but since it would have to bind a seperate function onClick, and since the variable i is local, the writeLED function always gets the first parameter 10 (which is max i+1), but it would need to get the current i+1, like the getElementBy id i+1 does. Anyone can solve the puzzle?

function showLED(ledCount){
for(var i = 0;i<=(ledCount-1);i++){
    if(color[i] == 0){
        document.getElementById('buttonLED'+(i+1)).onclick = function(){writeLED((i+1),1); } ;
        document.getElementById('buttonLED'+(i+1)).value="light is on";
    }else{
        document.getElementById('buttonLED'+(i+1)).onclick = function(){writeLED((i+1),0); } ;
        document.getElementById('buttonLED'+(i+1)).value="light is off";
    }
}
}

Ben McCormick · Accepted Answer · 2013-03-09T22:07:20.553

3

You'll need to wrap it in a self-executing function to create a new scope where you can preserve the current value of the incrementing function.

You can do that inline like this:

document.getElementById('buttonLED'+(i+1)).onclick = function(loopincrement){   
    return function(){writeLED((loopincrement+1),1); } ;

}(i)

or as a pulled out function like this:

function writeLEDInNewScope(inc){
    return function(){ writeLED(inc,1)};
}

document.getElementById('buttonLED'+(i+1)).onclick = writeLEDinNewScope(i+1);

The outer function will preserve the i value's current value for the inner onclick function. It executes immediately and returns the inner function that you want bound to the onclick property. It will maintain the reference to the loopincrement variable, which won't be effected by future loop iterations.

edited Mar 09 '13 at 22:07

answered Mar 09 '13 at 21:40

Ben McCormick

25,260
12
52
71

Even though this is correct, the more modern way of doing it is with `func.apply()` -- either way, OP should read up about closures. – David Titarenco Mar 09 '13 at 21:42
You can just shadow `i` by passing it as an argument to the outer function. – Blender Mar 09 '13 at 21:42
@David - I though that apply will actually execute the function with an explicit `this` and array of arguments, are you sure it is O.K to assign it to callback? – Mar 09 '13 at 21:45
@bfavaretto wrote it too quickly. Fixed now – Ben McCormick Mar 09 '13 at 21:47
I'm picky (again): While the self executing function is a closure (every function in JS is (kind of)), this is not the reason why this solution works. The event handler the OP assigns is a closure as well (in fact, the way closures are implemented in JS is the source of the problem). What the self executing function does is creating a new scope. It would work as well if it wasn't a closure. Even more so, if you consider a closure to be a function with free variables, then your "wrapper" function is not a closure since there are no free variables. – Felix Kling Mar 09 '13 at 21:57
@pure_code: I meant to say `bind`, oops. I answered a similar question here: http://stackoverflow.com/questions/11821484/javascript-closure/11821603#11821603 – David Titarenco Mar 09 '13 at 21:57
just earlier today I was thinking I should update this pattern in my code base to bind. – Mar 09 '13 at 22:00
`your_function.bind(some_this,arg1,arg2....)` should do it. – Mar 09 '13 at 22:02
@FelixKling nothing wrong with being picky. I appreciate having it pointed out when my language is not as precise as it could be – Ben McCormick Mar 09 '13 at 22:02
@pure_code: from what I understand, `bind` has a slight performance hit, but I think it's going to be optimized soon(tm) in most mainstream browsers. It just looks/reads better than having all these weird dangling closures. – David Titarenco Mar 09 '13 at 22:03
well...yea...that's what I meant. – Mar 09 '13 at 22:04
@David: It works as long as you don't need access to `this` (e.g. the element the handler is bound to). – Felix Kling Mar 09 '13 at 22:04
this question has been asked 100+ times on SO I'm sure. – Mar 09 '13 at 22:05
and yes, this question is asked like 3 times a day :P should probably be closed. – David Titarenco Mar 09 '13 at 22:07

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

This is an infamous for loop problem. You have to rewrite your code like this:

function callback(x) {
    return function() { writeLED(x, 1); };
}

function showLED(ledCount) {
    for (var i = 0; i <= (ledCount - 1); i++)(function(i) {
        if (color[i] == 0) {
            document.getElementById('buttonLED' + (i + 1)).onclick = callback(i + 1);
            document.getElementById('buttonLED' + (i + 1)).value = "light is on";
        } else {
            document.getElementById('buttonLED' + (i + 1)).onclick = callback(i + 1);
            document.getElementById('buttonLED' + (i + 1)).value = "light is off";
        }
    })(i);
}

Javascript - use variable in a function, within a function

2 Answers2