11

I have this code:

var items = this.llistat.getElementsByTagName('a');

for( var i = 0; i < items.length; i++ ){    
  items[i].addEventListener('click', function(event) {
    alert( i );
  }, items[i]);
}

where the event is listened, but there are 3 items and the alert allways print 3 on any of the elements (it doesn't respect the index),

Dosen't items[i] shouldn't do the job as closure?

thanks!

m59
  • 43,214
  • 14
  • 119
  • 136
Toni Michel Caubet
  • 19,333
  • 56
  • 202
  • 378
  • The third argument to [`addEventListener`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget.addEventListener) is a boolean that indicates whether the event listener gets capture priority (e.g., to make it cancelable); it does not specify a `this` value. – apsillers Dec 14 '13 at 20:13
  • Also related, [Javascript infamous Loop problem?](http://stackoverflow.com/questions/1451009/javascript-infamous-loop-problem). – Jonathan Lonowski Dec 14 '13 at 20:19

2 Answers2

12

No, the third argument of addEventListener is the useCapture one. See MDN for more information.

But you can use:

for( var i = 0; i < items.length; i++ ){
    (function(i){
        items[i].addEventListener('click', function(event) {
            alert( i );
        }, false);
    })(i);
}

or

var handler = function(event) {
    var i = items.indexOf(this);
    alert( i );
};
for( var i = 0; i < items.length; i++ ){
    items[i].addEventListener('click', handler, false);
}

The first one creates a new event handler for each element, so it needs more memory. The second one reuses the same event listener, but uses indexOf, so it's more slow.

Oriol
  • 274,082
  • 63
  • 437
  • 513
9

That's a classical closure issue : you must create a new function bound, not to the 'i' variable, but to its value at the time of binding : 

var items = this.llistat.getElementsByTagName('a');

for( var i = 0; i < items.length; i++ ) {
        items[i].addEventListener('click', listener.bind( null, i) );
}

function listener(index) {
         alert(index);
}
GameAlchemist
  • 18,995
  • 7
  • 36
  • 59
  • Thanks for your anser, this works. Is it posible using anonimous functions only? just curious.. thanks! – Toni Michel Caubet Dec 14 '13 at 20:31
  • You're welcome. The function that is stored inside listener is anonymous. You might not want to create this intermediate var and just replace, inside addEventListener, 'listener' by its value. Yet i think it's more easily understood like this. – GameAlchemist Dec 14 '13 at 20:37
  • Javascript interpreters do not optimise so it's more efficient to define `function listener(index) { return function() {...}; }` outside the loop then have the single statement `items[i].addEventListener('click', listener(i));` inside the loop. The efficiency arises from defining the outer function once. As written above the outer function is defined (and executed) at every iteration of the loop. – Beetroot-Beetroot Dec 15 '13 at 02:46
  • @Beetroot-Beetroot : i edited, but more for clarity than for performances : a click handler is anyway very slow, but yes, now only one function will be created per element instead of two, which will save a few nano-seconds :-) – GameAlchemist Dec 15 '13 at 05:15