8

Ok, here's a problem script.

var links = [ 'one', 'two', 'three' ];

for( var i = 0; i < links.length; i++ ) {
    var a = document.createElement( 'div' );
    a.innerHTML = links[i];
    a.onclick = function() { alert( i ) }
    document.body.appendChild( a );
}

This script generates three divs: one, two and three, using an array.
I've set a (Dom0 for simplicity) click handler on each div which alerts the index of its position in the array. - except it doesn't! It always alerts 3, the last index of the array.
This is because the 'i' in 'alert( i )' is a live reference to the outer scope (in this case global) and its value is 3 at the end of the loop. What it needs is a way of de-referencing i within the loop.

This is one solution and I tend to use it.

var links = [ 'one', 'two', 'three' ];

for( var i = 0; i < links.length; i++ ) {
    var a = document.createElement( 'div' );
    a.innerHTML = links[i];
    a.i = i; //set a property of the current element with the current value of i
    a.onclick = function() { alert( this.i ) }
    document.body.appendChild( a );
}

Does anyone else do anything different?
Is there a really smart way of doing it?
Does anyone know how the libraries do this?

meouw
  • 41,754
  • 10
  • 52
  • 69
  • Actually, in your first code sample above, the alert displays 3 rather than 2 as i incrementing to 3 causes the loop to stop -- I know it's nit picky, but just thought I'd bring up! Good question, though! – Peter Meyer Jan 14 '09 at 14:03

4 Answers4

20

You need to use this little closure trick - create and execute a function that returns your event handler function.

var links = [ 'one', 'two', 'three' ];

for( var i = 0; i < links.length; i++ ) {
    var a = document.createElement( 'div' );
    a.innerHTML = links[i];
    a.onclick = (function(i) { return function() { alert( i ) } })(i);
    document.body.appendChild( a );
}
Greg
  • 316,276
  • 54
  • 369
  • 333
  • Works like a charm, thanks RoBorg! Let's see what else turns up. – meouw Jan 14 '09 at 13:49
  • One should use closures sparingly - they might screw with the garbage collector, especially when assigned as event handlers as they'll most likely live indefinetly... – Christoph Jan 14 '09 at 14:10
  • you can always use addEventListener, stash the routine and then call removeEventListener when you're leaving the page (onunload() ?) – Jason S Jan 14 '09 at 14:14
  • 3
    You can achieve the same thing without passing 'i' around by creating a locally scoped variable: (function() { var local = i; return function() { alert(local); } })(); Six-of-one and all that, but I prefer this approach because otherwise I tend to forget the variable as a parameter somewhere. – Grant Wagner Jan 14 '09 at 14:52
  • @Christoph, I would like to know why you think that is the case. Leveraging closures is a fundamental tool in javascript and if your events are removed from an element correctly when the element is removed from the DOM I've never seen evidence of leaks. – Prestaul Jan 15 '09 at 00:32
  • @Prestaul: indefinetly means 'as long as the page stays open' - which can mean for weeks in my usage pattern thanks to suspend-to-disk... – Christoph Jan 19 '09 at 15:44
  • @Prestaul: I consider the following thins bad practice: using closures when you don't have to; creating new function objects if you don't have to (creating them in loops is especially bad) – Christoph Jan 19 '09 at 15:46
  • @Prestaul: look at your code: for every div element you create two function objects which each hold a reference to the outermost scope - ie none of the variables in it can be collected! – Christoph Jan 19 '09 at 15:49
  • @Christoph ;): the last statement is only true for primitive javascript implementations - better ones could check for `eval()` in the function's body and only hold references to the actually... referenced ;) variables – Christoph Jan 19 '09 at 15:55
4

I'd stay with your own solution, but modify it in the following way:

var links = [ 'one', 'two', 'three' ];

function handler() {
    alert( this.i );
}

for( var i = 0; i < links.length; i++ ) {
    var a = document.createElement( 'div' );
    a.innerHTML = links[i];
    a.i = i; //set a property of the current element with the current value of i
    a.onclick = handler;
    document.body.appendChild( a );
}

This way, only one function object gets created - otherwise, the function literal will be evaluated on every iteration step!

A solution via closure is even worse performance-wise than your original code.

Christoph
  • 164,997
  • 36
  • 182
  • 240
  • Why pollute the global namespace when the function can be declared inline? What does it gain you? – Prestaul Jan 15 '09 at 00:35
  • @Prestaul: Less objects, less memory, faster. Put it inside a function and the global namespace won't be polluted by the handler nor the array with links. – some Jan 15 '09 at 06:36
1

I recommend Christophs way with one function since it uses less resources.

Below is another way that stores the value on the function (that is possible because a function is an object) and users argument.callee to get a reference to the function inside the function. In this case it doesn't make much sense, but I show the technique since it can be useful in other ways:

var links = [ 'one', 'two', 'three' ];

for( var i = 0; i < links.length; i++ ) {
    var a = document.createElement( 'div' );
    a.innerHTML = links[i];
    a.onclick = function() { alert( arguments.callee.i ) }
    a.onclick.i = i;
    document.body.appendChild( a );
}

The technique is useful when your function needs to store persistent information between calls. Replace the part above with this:

a.id="div"+i;
a.onclick = function() {
    var me = arguments.callee;
    me.count=(me.count|0) + 1;
    alert( me.i );
}

and you can later retrieve how many times it was called:

for( var i = 0; i < links.length; i++ ){
    alert(document.getElementById("div"+i).onclick.count);
}

It can also be used to cache information between calls.

Community
  • 1
  • 1
some
  • 48,070
  • 14
  • 77
  • 93
0

RoBorg's method is definitely the way to go, but I like a slightly different syntax. Both accomplish the same thing of creating a closure that preserves 'i', this syntax is just clearer to me and requires less modification of your existing code:

var links = [ 'one', 'two', 'three' ];

for( var i = 0; i < links.length; i++ ) (function(i) {
    var a = document.createElement( 'div' );
    a.innerHTML = links[i];
    a.onclick = function() { alert( i ) }
    document.body.appendChild( a );
})(i);
Prestaul
  • 83,552
  • 10
  • 84
  • 84