0

I have an HTML page with several forms (created dynamically), so I get them in JS into an array, and then I add an EventListener to each one, like this:

var forms = document.getElementsByTagName('form');

for(i=0; i < forms.length; i++) {
    forms[i].addEventListener('change', function(){
        checkAllFilled(i);
    });
}

So in function checkAllFilled I do some stuff.

The problem is that if I have 7 forms (from forms[0] to forms[6]) and I work over forms[2], it is always called checkAllFilled(7), instead of calling checkAllFilled(2). It's like the variable i was set in the last value over the for loop.

How would I do this?

Thanks!

SimpleBeat
  • 747
  • 1
  • 10
  • 20
Ommadawn
  • 2,450
  • 3
  • 24
  • 48

4 Answers4

4

It's like the variable "i" was set in the last value over the for loop.

This is exactly what's happening.

Think about it. Your event handler runs later, after the for-loop has been and gone. At that time, the variable has whatever value the loop left it with - hence, the event will always report the same, single value.

Instead, you need to bind the iterative value in via a closure. There's a few ways. One is via an immediately-executed function (IEF).

forms[i].addEventListener('change', (function(i) { return function(){
    checkAllFilled(i);
}; })(i));

We pass the iterative value to our IEF as a function variable. This way, it's passed to the handler via a closure. It's worth reading up on these.

Community
  • 1
  • 1
Mitya
  • 33,629
  • 9
  • 60
  • 107
  • Thanks for the explanaiton and reference :) But it doesn't work! :( – Ommadawn May 06 '17 at 18:49
  • The IIF needs to take the passed argument and that is what needs to be used when invoking `checkAllFilled` instead of `i`. As this code example now reads, it doesn't do anything different from the original code. – Lennholm May 07 '17 at 02:57
  • Typo - missed `i` as the IEF argument. Resolved and works. [Fiddle](https://jsfiddle.net/) – Mitya May 07 '17 at 12:14
2

Another way is just to use let from ECMAScript 2015.

for(let i=0; i < forms.length; i++) {
    forms[i].addEventListener('change', function(){
        checkAllFilled(i);
    });
}
Stanislav Mayorov
  • 4,298
  • 5
  • 21
  • 44
1

You are always calling the last generated event listener, since you generate first these listeners (all with same name) and then call the last one of them.

The best solution here is to assign names for the functions handling the events based on the counter value i.

Oussama Ben Ghorbel
  • 2,132
  • 4
  • 17
  • 34
1

Use forEach()instead of a for loop:

var forms = document.getElementsByTagName('form');

[].slice.call(forms).forEach(function(form, i) {
    form.addEventListener('change', function(){
        checkAllFilled(i);
    });
});
Lennholm
  • 7,205
  • 1
  • 21
  • 30