Event handlers inside a Javascript loop - need a closure?

13,691

You do, indeed, need to implement a closure here. This should work (let me know - I didn't test it)

var blah = xmlres.getElementsByTagName('blah');
for(var i = 0; i < blah.length; i++) {
    var td = document.createElement('td');
    var select = document.createElement('select');
    select.setAttribute("...", "...");
    select.onchange = function(s,c,a)
    {
        return function()
        {
            onStatusChanged(s,c,a);
        }
    }(select, callid, anotherid);
    td.appendChild(select);
}
Share:
13,691
matt b
Author by

matt b

Hello, world!

Updated on June 17, 2022

Comments

  • matt b
    matt b almost 2 years

    I'm working with a bit of html and Javascript code that I've taken over from someone else. The page reloads a table of data (via an asynchronous request) every ten seconds, and then re-builds the table using some DOM code. The code in question looks something like this:

    var blah = xmlres.getElementsByTagName('blah');
    for(var i = 0; i < blah.length; i++) {
        var td = document.createElement('td');
        var select = document.createElement('select');
        select.setAttribute("...", "...");
        select.onchange = function() {
            onStatusChanged(select, callid, anotherid);
        };
        td.appendChild(select);
    }
    

    When the onchange event is fired for a <select> element however, it seems like the same values are being passed to the onStatusChanged() method for every <select> in the table (I've verified that in each iteration of the loop, callid and anotherid are being given new, distinct values).

    I suspect this is occuring because of the nature of how I am setting the event handler, with the select.onchange = function() syntax. If I understand how this is working correctly, this syntax sets a closure for the onchange event to be a function which refers to these two references, which end up having a final value of whatever they are set to on the last iteration of the loop. When the event fires, the value referenced by callid and anotherid is the value set in the last iteration, not the value set at the individual iteration.

    Is there a way that I can copy the value of the parameters I am passing to onStatusChanged()?

    I've changed the title to better reflect the question and the accepted answer.

  • matt b
    matt b over 15 years
    Also another solution I discovered - a workaround, really - would be to store the callid and anotherid as attributes in the DOM element, and inside the eventhandler just reference those via this.getAttribute()
  • dkretz
    dkretz over 15 years
    I wouldn't call it a workaround. What I would expect is to have the handler set once (not every time through the loop) and have it discover the variables itself.
  • Jo So
    Jo So over 10 years
    Note that what OP has is a closure, too. And indeed OP has the more representative form where your closure shares its environment with other closures (one closure is created per loop run). But that's exactly what gives the unexpected effect.