How can I make sure the addEventListener doesn't ignore the iteration of the i var in my for loop?

Question

I'm working on a userscript that will make lots of buttons, and I can't seem to give them all an unique function.

I already tried

 downArrow.onclick = function (){downVote(id, username)};<br>

and

 downArrow.onclick = "downVote(\"" + id + "\", \"" + username + "\")";

But they don't work. Then I read somewhere that only the following works:

 downArrow.addEventListener('click', downVote(id, username), false);

This causes that all the buttons will only downvote the last ID and username of the iteration.

I want them all to have unique onclick functions.

Entire for loop:

var targetPosts = document.getElementsByClassName("thing message");
for (var i=0;i<targetPosts.length;i++) 
    {
    try
      {
        id = targetPosts[i].getAttribute("data-fullname");
        username = targetPosts[i].childNodes[4].childNodes[1].childNodes[0].childNodes[1].childNodes[1].childNodes[0].innerHTML;

        var upArrow=document.createElement("DIV");
        upArrow.className = "arrowUp";
        upArrow.id = id + "up";
        upArrow.addEventListener('click', function(){ upVote(id, username)} , false);

        var downArrow=document.createElement("DIV");
        downArrow.className = "arrowDown";
        downArrow.id = id + "down";
        downArrow.addEventListener('click', function(){ downVote(id, username)} , false

      targetPosts[i].childNodes[3].appendChild(upArrow);
      targetPosts[i].childNodes[3].appendChild(downArrow);
      }
    catch(err){}
    }

possible duplicate of [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) and [Closures in a for loop](http://stackoverflow.com/questions/2192348/closures-in-a-for-loop). — Raymond Chen, May 31 '13 at 13:59

GameAlchemist · Accepted Answer · 2013-05-31T13:54:23.953

4

bind was invented for exactly such a case :

 upArrow.addEventListener('click', upVote.bind(upArrow, id, username), false);

should do the job.

edited May 31 '13 at 13:54

answered May 31 '13 at 13:45

GameAlchemist

18,995
7
36
59

You probably want `upVote.bind(upVote, id, username)` because event handlers' value of `this` refers to the element it's bound to – Ian May 31 '13 at 13:49
Yeah you're right. I'm used to doing this where the handler doesn't **just** call a function, and needs the value of `this`. I was thinking of something else, sorry :) – Ian May 31 '13 at 13:54
This one works. I don't know why it works, but it does. What does bind do different than all the other things? – Maarten Boogaard May 31 '13 at 13:54
@lan : you made a typo, but still you were right : i changed : upVote.bind(null, id, username) to upVote.bind(upArrow, id, username). – GameAlchemist May 31 '13 at 13:55
@VincentPiel Ahh yeah, sorry. I'm confusing myself because they both start with "up"! – Ian May 31 '13 at 13:56
1

@Marteen : google "MDN bind" for the details, but basically bind creates a new function that has 'this' set to first element, and its arguments set to the arguments that follows. Since binding arguments are evaluated when bind is called, the bound function 'remembers' the value of id. – GameAlchemist May 31 '13 at 13:59
only beware that it can be not supported in some browsers (but can be easily added if missing). – lupatus May 31 '13 at 14:04
@lupatus : yes, MDN provides the code to support bind in all browsers. Note also that since 'bind' creates a new function, you have to store it somewhere if you plan on removing the event handler later.@lan : i was confused so i checked on MDN : the value of this is set when adding an event listener in html, not in javascript. Another weird rule... – GameAlchemist May 31 '13 at 14:06

Johannes Egger · Answer 2 · 2013-05-31T13:50:05.893

3

Since JavaScript has no block scope (will come with ES6) try the following:

downArrow.addEventListener('click', function(uid, uname) {
    return function() {
        downVote(uid, uname);
    };
}(id, username), false);

When invoking the anonymous function (happens immediately) you capture the current state of id and username and use it when the inner function is invoked (happens when the user clicks the button).

In ES6 you can use let id = ... to define a variable with block scope and your posted code should work fine.

edited May 31 '13 at 13:50

answered May 31 '13 at 13:38

Johannes Egger

3,874
27
36

I'm going to try this now. – Maarten Boogaard May 31 '13 at 13:43
Your inner function call needs to be `downVote(uid, uname);` since those are the parameters of the IIFE – Ian May 31 '13 at 13:47

lupatus · Answer 3 · 2013-05-31T13:55:11.467

I'm not sure how you're trying to do that (would be better to show loop code), but in general its not very secure to create functions within the loop because you can by mistake reference to variable being changed inside a loop so finally you'll get all functions using same id. solution for that is to use some helper function:

var bindClick = function (element, id, username) {
        element.addEventListener('click', function () {
            downVote(id, username);
        }, false);
    },
    i, mybutton;
for (i = 0 ; i < amountOfUsers ; ++i) {
    mybutton = document.getElementById('downvote-handle-' + i);
    bindClick(mybutton, i, usernames[i]);
}

this way i value is copied (passed by value) and is safe.

it can be done also with anonymous function, result will be the same (but IMHO less readable):

for (i = 0 ; i < amountOfUsers ; ++i) {
    mybutton = document.getElementById('downvote-handle-' + i);
    (function (element, id, username) {
        element.addEventListener('click', function () {
            downVote(id, username);
        }, false);
    }(mybutton, i, userames[i])); 
}

So basically that would add unique functions to all those buttons after they're created, instead of being in the same loop as them being created? — Maarten Boogaard, May 31 '13 at 13:47
@MaartenBoogaard No, you call the `bindClick` function inside the original loop, which passes things to the function and binds the event. So there's nothing unnecessary. I prefer this way — Ian, May 31 '13 at 13:53
Nope, sorry, what I wrote is a little bit stupid ;) - you'll still have same problem as I described at first. I'll update my answer. — lupatus, May 31 '13 at 13:53

How can I make sure the addEventListener doesn't ignore the iteration of the i var in my for loop?

3 Answers3