3

I want to add an event listener to all of my tags, each passing a reference to itself as a parameter when the even is triggered. Here is the function I wrote:

function validateDigitsFeature()
{
    //  Add the event listeners to input tags
    //      Get the array of input tags
    var inputTags = document.getElementsByClassName('validateInput');
    var tagId;
    //      Loop through them, adding the onkeypress event listener to each one
    for (var i = 0; i < inputTags.length; i++)
    {
        //  Give each input element an id
        tagId = inputTags[i].id = 'input_id_' + i;
        inputTags[i].addEventListener('keyup', function(){isNumberOrDot(event, tagId);}, false);
    }
}

Basically the function should do the following:

  1. Store all the input tags with the specified classname in an array
  2. Loop through the array, adding an id to each tag and
  3. Adding the onkeyup event listener with the isNumberOrDot(event, tagId) handler.

Problem

The onkeyup event is added, but they handlers of each one is always referencing the tagIdof the last element of the array.

Question

What is wrong with the code/logic? And how can it be fixed?

Note

Sure this problem is related to JavaScript Closure in loops, while this question could have a more general answer, it is specific to event listeners being used. To more advanced developers, it might be easy to apply the general solution to this problem. But to me the other solutions still didn't provide a full explanation or even worked.

Thank you in advance.

sargas
  • 5,820
  • 7
  • 50
  • 69
  • 1
    It's because you're dealing with closure issues. – frenchie Jan 31 '14 at 18:17
  • 1
    This question has been asked a billion times. You need to bind the value of the variable in a closure for each loop iteration if it's going to be accessed in a callback. – Andrew Mao Jan 31 '14 at 18:17
  • Thank you for point me to the right direction. I didn't know which key words to use to find a solution. I'll properly flag my question as a duplicate after getting my answer. – sargas Jan 31 '14 at 18:20
  • @andrew that's not very precise. The event handler already is a closure. The solution is to create a new scope by executing a function (whether this involves yet another closure or not doesn't matter). – Felix Kling Jan 31 '14 at 18:30

1 Answers1

7

Because the actual event occurs sometime in the future after your for loop has already finished running and thus its index is at the last value and any local variables in your function like tagId are also at their last value. You need to create some sort of closure that preserves the value of i or tagId uniquely for each event handler so they each have access to their own value.

There are several different ways to do that, but all involve passing the i value to a function for each event handler.

Here's one using an IIFE (immediately invoked function expression):

function validateDigitsFeature()
{
    //  Add the event listeners to input tags
    //      Get the array of input tags
    var inputTags = document.getElementsByClassName('validateInput');
    //      Loop through them, adding the onkeypress event listener to each one
    for (var i = 0; i < inputTags.length; i++)
    {
        //  Give each input element an id
        (function() {
            // creates a unique function context for each event handler so the
            // value of tagId is unique for each event handler
            var tagId = inputTags[i].id = 'input_id_' + i;
            inputTags[i].addEventListener('keyup', function(){isNumberOrDot(event, tagId);}, false);
        })();
    }
}

A little more common way to do this is to pass the index from the for loop into the closure and do any calculation based on it inside the event handler (though either method works fine) like this:

function validateDigitsFeature()
{
    //  Add the event listeners to input tags
    //      Get the array of input tags
    var inputTags = document.getElementsByClassName('validateInput');
    //      Loop through them, adding the onkeypress event listener to each one
    for (var i = 0; i < inputTags.length; i++)
    {
        //  Give each input element an id
        (function(index) {
            // passes the `for` loop index into a function closure
            // so it is uniquely preserved for each event handler
            inputTags[index].addEventListener('keyup', function(){
                isNumberOrDot(event, inputTags[index].id = 'input_id_' + index);
            }, false);
        })(i);
    }
}
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • 1
    That was the solution I needed. Thank you for the explanation. Too bad this question was marked as a duplicate of other questions which answers didn't provide enough inside to this specific situation. Even though they are targeting the same topic. Thank you. – sargas Jan 31 '14 at 20:55
  • 1
    @sargas - yeah this is a common root issue that is asked about a lot, but the best solution is often a bit different for different situations so it's not always obvious when "duplicate" is really the best way to answer the question. Anyway, glad you now know how to solve your issue. – jfriend00 Jan 31 '14 at 20:59