How can I turn this mess in a single loop?

Question

I want to change this long piece of code in a single loop: Please help me.

itemAll[0].addEventListener('click', function () {
    klik(0)
}, false);
itemAll[1].addEventListener('click', function () {
    klik(1)
}, false);
itemAll[2].addEventListener('click', function () {
    klik(2)
}, false);
itemAll[3].addEventListener('click', function () {
    klik(3)
}, false);
itemAll[4].addEventListener('click', function () {
    klik(4)
}, false);
itemAll[5].addEventListener('click', function () {
    klik(5)
}, false);
itemAll[6].addEventListener('click', function () {
    klik(6)
}, false);
itemAll[7].addEventListener('click', function () {
    klik(7)
}, false);
itemAll[8].addEventListener('click', function () {
    klik(8)
}, false);
itemAll[9].addEventListener('click', function () {
    klik(9)
}, false);
itemAll[10].addEventListener('click', function () {
    klik(10)
}, false);
itemAll[11].addEventListener('click', function () {
    klik(11)
}, false);

I already tried this but it does not seem to work, I really dont get it why:

for (var i = 0; i < itemAll.length; i++) {
    var item = itemAll[i];
    item.addEventListener('click', function (item) {
        klik(item);
    })
}

Anyway thank you for your time, I hope someone can figure this out for me.

@Edd that would be a different problem with that suggestion. — epascarello, Jun 06 '16 at 13:42
Possible duplicate of [How do JavaScript closures work?](http://stackoverflow.com/questions/111102/how-do-javascript-closures-work) — John Dvorak, Jun 06 '16 at 13:42
Related: [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/q/750486/218196) — Felix Kling, Jun 06 '16 at 13:48

score 11 · Accepted Answer · answered Jun 06 '16 at 13:42

11

You can use .forEach():

itemAll.forEach(function(item, i) {
  item.addEventListener("click", klick.bind(undefined, i));
});

Better yet, you could make sure that all the elements you're working with have a common class, and then just bind a single event handler at the document level that handles clicks and checks for the target element's class. (Basically, the "event delegation" thing that jQuery allows.)

answered Jun 06 '16 at 13:42

Pointy

405,095
59
585
614

@Webinan correct. But If jQuery can move away from it so can we ;) – Spencer May Jun 06 '16 at 13:46
@Webinan that's true; there's a polyfill for it at [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach). – Pointy Jun 06 '16 at 13:46
1

@Pointy yes, and if he is using jQuery he already has a polyfill `$.each` – Positivity Jun 06 '16 at 13:57

Ivan Yurchenko · Answer 2 · 2017-04-16T14:08:25.823

3

It seems like you have an error in your loop. From your code I would assume that you should call your klik() function on i instead of item.

for (var i = 0; i < itemAll.length; i++) {
    (function(i) {
        itemAll[i].addEventListener('click', function () {
            klik(i);
    })(i);
}

edited Apr 16 '17 at 14:08

answered Jun 06 '16 at 13:43

Ivan Yurchenko

3,762
1
21
35

And when the OP uses this, they will have a problem since `i` will not be the index it was defined in the loop. – epascarello Jun 06 '16 at 13:44
@epascarello You're right, excuse me. Added an update with consideration of closures. – Ivan Yurchenko Jun 06 '16 at 13:46
That doesn't solve the issue. How can this get 3 upvotes? See http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – Felix Kling Jun 06 '16 at 13:49
@FelixKling Could you clarify what's the problem here? I'm adding a new variable `index` and then pass it to the `klik` function so the closures don't affect this. Doesn't it makes the behavior as expected? – Ivan Yurchenko Jun 06 '16 at 14:01
This did not really fix it. – Vera Perrone Jun 06 '16 at 14:02
Counter question: why are you not using `i` directly? What is the difference between `i` and `index`? – Felix Kling Jun 06 '16 at 14:03
As far as I understand, when we pass `i` to the function the closure will affect our behavior so when we finish our loop all `klik` will have `itemAll.length - 1` inside them. But when I make a temporary variable it stores inside the value that we need to pass to the function so we will closure what we need and each of `klik` functions will get an appropriate value. What's wrong with my assumptions? – Ivan Yurchenko Jun 06 '16 at 14:09
Lets recap what the issue is: There is a single variable `i`. Every closure refers to that variable `i`. The loop iterates a bunch of times, so `i` will have a value equal to `itemAll.length` at the end. Lets look at `index`. `i` and `index` have both the same scope (JavaScript doesn't have a block scope). I.e. there is a *single* variable `index`. In the first iteration `index` is set to `0` (`i = 0`). In the second iteration, `index` is set to `1` (`i = 1`), etc. When the loop terminates, `index` has the value `itemAll.length - 1`. You created an *alias* for `i`, which has the same problems. – Felix Kling Jun 06 '16 at 14:36
@FelixKling Thanks for explanations. If I encapsulate everything inside the loop into `function()` then all of the iterations should have different values inside `index` variable and it should work now, right? – Ivan Yurchenko Jun 06 '16 at 14:53
Yes. By calling a function you are creating a new scope per iteration, essentially simulating block scope. – Felix Kling Jun 06 '16 at 14:54

score 3 · Answer 3 · answered Jun 06 '16 at 13:44

Try this:

for (var i = 0; i < itemAll.length; i++) {
    var item = itemAll[i];

    (function(item)
    {
        item.addEventListener('click', function () {
            klik(item);
        });
    })(item);
}

You need to create a new scope or when you will call klik(item) item will always be equal to the last item.

EDIT: It seems you want to pass i as parameter not item, replace the item parameter by i

score 0 · Answer 4 · answered Jun 06 '16 at 21:25

May I suggest a functional programming example, just to share an alternative solution?

function assign_click_event_to_collection(elements){
  assign_click_event_to_element_and_move_to_next(elements, 0);
}

function assign_click_event_to_element_and_move_to_next(elements, index){
  if(index > elements.length) return false;
  itemAll[index].addEventListener('click', function() {
    klik(index);
  }, false);
  assign_click_event_to_element_and_move_to_next(elements, index+1);
}

assign_click_events_to_collection(itemAll);

How can I turn this mess in a single loop?

4 Answers4