1

I have been trying to add click events to a series of divs using a for loop. The divs are dynamically created and loaded. Every div is supposed to call its own callback function. But it seems every div is attached to the final event listener and calls callback function of final event listener.

Below is my basic code:

for(index=0; index<divs.length; index++) {
  divs[index].addEventListener("click", function(){console.log(divs[index].getAttribute("id"));}, true); //capture click event
}

On click only the id of final div is displayed by every div.

Asur
  • 1,949
  • 4
  • 23
  • 41

3 Answers3

8

Phil's answer offers a good solution to the specific code you posted (+1), but doesn't explain what the problem was with your original code.

The problem is that the event handler closures get an enduring reference to the index variable, not a copy of it as of when they're created. So they all see the final value that index has (divs.length). For instance, this code

for (index = 0; index < 4; ++index) {
    setTimeout(function() {
        console.log(index);
    }, 100);
}

...will log "4" four times when the timeouts occur, not "0", "1", "2", and "3".

To correct it in the general case where you want to make sure your handler closes over a specific value, use a factory function that generates the event handler function for you, where the event handler closes over the argument you feed the factory function instead of the loop variable:

for(index=0; index<divs.length; index++) {
    divs[index].addEventListener("click", createHandler(divs[index], true); //capture click event
}

function createHandler(div) {
    return function(){
        console.log(div.getAttribute("id"));
    };
}

There, the event handler closes over div, which doesn't change.

Closures are one of the most powerful features of JavaScript. Once you understand how they work (and they're actually a lot simpler than people think), you can use them to really good effect. More: Closures are not complicated

Community
  • 1
  • 1
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • +1 for the great problem description. Your `createHandler` method would also work in IE's (< 9) `attachEvent` whereas mine would not (`this` support is broken) – Phil Jan 16 '12 at 06:02
  • @T.J. Crowder closure is interesting, thanx – Asur Jan 17 '12 at 15:00
3

This is because you're using divs[index] in the event handler which, after the loop has completed, is set to the last element.

Use this instead. Also, you can use the one event handler instead of creating a bunch of identical closures, eg

function logId(e) {
    console.log(this.getAttribute("id"));
}

... and in your loop ...

divs[index].addEventListener("click", logId, false);

See https://developer.mozilla.org/en/DOM/element.addEventListener#Memory_issues

Phil
  • 157,677
  • 23
  • 242
  • 245
2

You can use the this keyword as pointed out by @phil.

But, to explain what your problem was, consider this closure:

for(index=0; index<divs.length; index++) {
  (function(index){
      divs[index].addEventListener("click", function(){
          console.log(divs[index].getAttribute("id"));
      }, true);
  })(index);
}

Now, every function get's its own copy of the index variable. In your code, they all share the same variable; so in the end, index would be equals to divs.length -1 regardless of which div is actually being clicked.

Joseph Silber
  • 214,931
  • 59
  • 362
  • 292
  • your illustration works as well, but i still don't understand what (function(arg){ statements })(arg) is, thanx for your help – Asur Jan 17 '12 at 15:03