23

Refactoring standard onClick within html tag to listeners ,faced problem with my code:

var td;
    for (var t=1;t<8;t++){
        td = document.getElementById('td'+t);
        if (typeof window.addEventListener==='function'){
                td.addEventListener('click',function(){
                    console.log(td);
            })}
 }  

When td element is clicked on,it's assumed that clicked td with last index from loop,e.g. 7
Looks like ,eventListeners been populated for last element in this loop only.
Loop initialization looks correct.
Why so happened?

Here is live code

4
  • Can you try declaring var within loop? or try same code explicitely for td1 after loop. sounds stupid lets try it.
    – Azodious
    Jan 18, 2012 at 11:58
  • @Azodious,yeah,tried already,the result is the same
    – sergionni
    Jan 18, 2012 at 11:59
  • if specify explicitly(hardcode),e.g. td1 it works OK
    – sergionni
    Jan 18, 2012 at 12:00
  • try creating array of length 8. fill it with td objects and then try to attach listeners to indexes.
    – Azodious
    Jan 18, 2012 at 12:04

5 Answers 5

62

You need to wrap the assignment of the event listener in a closure, something like:

var td;
for (var t = 1; t < 8; t++){
    td = document.getElementById('td'+t);
    if (typeof window.addEventListener === 'function'){
        (function (_td) {
            td.addEventListener('click', function(){
                console.log(_td);
            });
        })(td);
    }
}
6
  • it works absoulutely OK.Thank you.Onr more question about closure: how it's happened,that _td is equal to td?
    – sergionni
    Jan 18, 2012 at 12:22
  • 4
    The closure is just a function which executes immediately, therefore preserving the state of the variables at the time of execution. We execute the function / closure by passing in td for the _td input parameter to the function.
    – jabclab
    Jan 18, 2012 at 12:27
  • 1
    Thank you for explanation.I just confused with _td and td naming.Looks like ,I can use just td
    – sergionni
    Jan 18, 2012 at 12:29
  • yeah it would be great to get a bit of explanation of what that _td is and also what that (td) at the end is.
    – john-jones
    Feb 19, 2014 at 9:16
  • 2
    @HermannIngjaldsson np, the _td is simply the input parameter for the function which forms the closure and td at the end is the value which is passed to the function. They don't actually have to have different names, e.g. they could both be td. Hope that helps :-)
    – jabclab
    Feb 19, 2014 at 9:28
36

What's happening

The variable td is defined outside of your event handlers, so when you click on a cell you are logging the last value it was set to.

More technically: each event handler function is a closure—a function that references variables from an outer scope.

The general solution

The general solution to this kind of problem is to return the event handler from a wrapper function, passing the variables you want to "fix" as arguments:

td.addEventListener('click', function(wrapTD) {
    return function() { console.log(wrapTD); }
}(td));

The arguments are now bound to the scope of the invoked wrapper function.

Simpler solution: use this

There's an even simpler option, however. In an event handler, this is set to the element on which the handler is defined, so you can just use this instead of td:

td.addEventListener('click', function() {
    console.log(this);
});

Even simpler: no loop!

Finally, you can get rid of the for loop entirely and set a single event handler on the whole table:

var table = document.getElementById('my-table');

table.addEventListener('click', function(e) {
    if (e.target.nodeName.toUpperCase() !== "TD") return;

    var td = e.target;
    console.log(td);
});

This is a much better solution for larger tables, since you are replacing multiple event handlers with just one. Note that if you wrap your text in another element, you will need to adapt this to check if the target element is a descendant of a td.

1
  • 10 years later, just wanted to say thanks for this: '...since you are replacing multiple event handlers with just one'.
    – nograde
    Mar 3, 2022 at 19:28
8

So what's happening here is that you are keeping the variable 'td' in scope for the event listener function. There is only 1 instance of 'td', and that is getting updated each time the for loop iterates. Thus, when the for loop finished, the value of td is now set to the element '#td7' and your event handler is simply logging the current value of td.

In the example above, you could simply log 'this':

var td;
for (var t=1;t<8;t++){
    td = document.getElementById('td'+t);
    if (typeof window.addEventListener==='function'){
      td.addEventListener('click',function(){
        console.log(this);
      });
    }
}

since 'this' is set to the element an event was bound on for the execution of an event handler.

I'm guessing you're looking for more of an answer about keeping hold of the iterator when creating closures from within a for loop. To do this you want to define a function outside of the for loop.

for (var t=1;t<8;t++){
  bind_event(t);
}

function bind_event(t) {
    var td = document.getElementById('td'+t);
    if (typeof window.addEventListener==='function'){
      td.addEventListener('click',function(){
        console.log(td);
      });
    }
}

This way, an instance of a variable named 'td' is created each time bind_event is run, and that instance will be kept in closure for the event listener function. It's worth noting that 't' in bind_event is also a new variable.

0
1

As i understand, it happens because of closure...you assign event handler within scope of FOR statement. When click happens, it takes the last version if TD variable in scope of FOR and writes it to log.

1
  • perhaps,it is.But I expected for every td:td1,td2,...td8 it will populated ok
    – sergionni
    Jan 18, 2012 at 12:02
0

the following should work as expected:

  for (var t=1;t<8;t++){
        var td;
        td = document.getElementById('td'+t);
        if (typeof window.addEventListener==='function'){
                td.addEventListener('click',function(){
                    console.log(this);
            })}
 } 

Not the answer you're looking for? Browse other questions tagged or ask your own question.