addEventListener using for loop and passing values [duplicate]

Question

I'm trying to add event listener to multiple objects using a for loop, but end up with all listeners targeting the same object --> the last one.

If I add the listeners manually by defining boxa and boxb for every instance, it works. I guess it's the addEvent for-loop that's not working the way I hoped for. Maybe I use the wrong approach altogether.

Example using 4 of the class="container" Trigger on container 4 works the way it´s supposed to. Trigger on container 1,2,3 trigger event on container 4, but only if trigger has already been activated.

// Function to run on click:  function makeItHappen(elem, elem2) {    var el = document.getElementById(elem);    el.style.backgroundColor = "red";    var el2 = document.getElementById(elem2);    el2.style.backgroundColor = "blue";  }    // Autoloading function to add the listeners:  var elem = document.getElementsByClassName("triggerClass");    for (var i = 0; i < elem.length; i += 2) {    var k = i + 1;    var boxa = elem[i].parentNode.id;    var boxb = elem[k].parentNode.id;      elem[i].addEventListener("click", function() {      makeItHappen(boxa, boxb);    }, false);    elem[k].addEventListener("click", function() {      makeItHappen(boxb, boxa);    }, false);  }

<div class="container">    <div class="one" id="box1">      <p class="triggerClass">some text</p>    </div>    <div class="two" id="box2">      <p class="triggerClass">some text</p>    </div>  </div>    <div class="container">    <div class="one" id="box3">      <p class="triggerClass">some text</p>    </div>    <div class="two" id="box4">      <p class="triggerClass">some text</p>    </div>  </div>

Answer 1

Closures! :D

This fixed code works as you intended:

// Function to run on click:  function makeItHappen(elem, elem2) {      var el = document.getElementById(elem);      el.style.backgroundColor = "red";      var el2 = document.getElementById(elem2);      el2.style.backgroundColor = "blue";  }    // Autoloading function to add the listeners:  var elem = document.getElementsByClassName("triggerClass");    for (var i = 0; i < elem.length; i += 2) {      (function () {          var k = i + 1;          var boxa = elem[i].parentNode.id;          var boxb = elem[k].parentNode.id;          elem[i].addEventListener("click", function() { makeItHappen(boxa,boxb); }, false);          elem[k].addEventListener("click", function() { makeItHappen(boxb,boxa); }, false);      }()); // immediate invocation  }

<div class="container">    <div class="one" id="box1">      <p class="triggerClass">some text</p>    </div>    <div class="two" id="box2">      <p class="triggerClass">some text</p>    </div>  </div>    <div class="container">    <div class="one" id="box3">      <p class="triggerClass">some text</p>    </div>    <div class="two" id="box4">      <p class="triggerClass">some text</p>    </div>  </div>

---

Why does this fix it?

for(var i=0; i < elem.length; i+=2){     var k = i + 1;     var boxa = elem[i].parentNode.id;     var boxb = elem[k].parentNode.id;      elem[i].addEventListener("click", function(){makeItHappen(boxa,boxb);}, false);     elem[k].addEventListener("click", function(){makeItHappen(boxb,boxa);}, false); } 

Is actually non-strict JavaScript. It's interpretted like this:

var i, k, boxa, boxb; for(i=0; i < elem.length; i+=2){     k = i + 1;     boxa = elem[i].parentNode.id;     boxb = elem[k].parentNode.id;      elem[i].addEventListener("click", function(){makeItHappen(boxa,boxb);}, false);     elem[k].addEventListener("click", function(){makeItHappen(boxb,boxa);}, false); } 

Because of variable hoisting, the var declarations get moved to the top of the scope. Since JavaScript doesn't have block scope (for, if, while etc.) they get moved to the top of the function. Update: as of ES6 you can use let to get block scoped variables.

When your code runs the following happens: in the for loop you add the click callbacks and you assign boxa, but its value gets overwritten in the next iteration. When the click event fires the callback runs and the value of boxa is always the last element in the list.

Using a closure (closing the values of boxa, boxb etc) you bind the value to the scope of the click handler.

---

Code analysis tools such JSLint or JSHint will be able to catch suspicious code like this. If you're writing a lot of code it's worthwhile to take the time to learn how to use these tools. Some IDEs have them built-in.