3

I know that this code doesn't work and I also know why. However, I do not know how to fix it:

JavaScript:

var $ = function(id) { return document.getElementById(id); };
document.addEventListener('DOMContentLoaded', function()
{
    for(var i = 1; i <= 3; i++)
    {
        $('a' + i).addEventListener('click', function()
        {
            console.log(i);
        });
    }
});

HTML:

<a href="#" id="a1">1</a>
<a href="#" id="a2">2</a>
<a href="#" id="a3">3</a>

I want it to print the number of the link you clicked, not just "4". I will prefer to avoid using the attributes of the node (id or content), but rather fix the loop.

Tyilo
  • 28,998
  • 40
  • 113
  • 198

3 Answers3

5

Wrap the loop block in its own anonymous function:

document.addEventListener('DOMContentLoaded', function()
{
        for(var i = 1; i <= 3; i++)
        {
            (function(i) {
                $('a' + i).addEventListener('click', function() {
                    console.log(i);
                })
            })(i);
        }
}

This creates a new instance of i that's local to the inner function on each invocation/iteration. Without this local copy, each function passed to addEventListener (on each iteration) closes over a reference to the same variable, whose value is equal to 4 by the time any of those callbacks execute.

Wayne
  • 59,728
  • 15
  • 131
  • 126
3

The problem is that the inner function is creating a closure over i. This means, essentially, that the function isn't just remembering the value of i when you set the handler, but rather the variable i itself; it's keeping a live reference to i.

You have to break the closure by passing i to a function, since that will cause a copy of i to be made.

A common way to do this is with an anonymous function that gets immediately executed.

        for(var i = 1; i <= 3; i++)
        {
            $('a' + i).addEventListener('click', (function(localI)
            {
                return function() { console.log(localI); };
            })(i);
        }

Since you're already using jQuery, I'll mention that jQuery provides a data function that can be used to simplify code like this:

        for(var i = 1; i <= 3; i++)
        {
            $('a' + i).data("i", i).click(function()
            {
                console.log($(this).data("i"));
            });
        }

Here, instead of breaking the closure by passing i to an anonymous function, you're breaking it by passing i into jQuery's data function.

Adam Rackis
  • 82,527
  • 56
  • 270
  • 393
  • I wasn't using jQuery just the basic $ = document.getElementById ;) – Tyilo Dec 01 '11 at 00:01
  • I was wondering why you were doing `$('a' + i).addEventListener` instead of `$('a' + i).click` -- oh well, some superfluous jQuery info :) – Adam Rackis Dec 01 '11 at 00:04
0

The closure captures a reference to the variable, not a copy, which is why they all result in the last value of the 'i'.

If you want to capture a copy then you will need to wrap it in yet another function.

Ed S.
  • 122,712
  • 22
  • 185
  • 265