Adding .onclick listeners in javascript loops

Question

I have a page getting JSON data via AJAX. I've managed to parse the data and create table rows from it. I'm now trying to add .onclick listeners to those rows using the following code:

function addResultsRows(tbody, jsonResults) {
    for (var i in jsonResults) {
        var tableRow = tbody.insertRow(0);
        var currRow = jsonResults[i];
        tableRow.onclick = (function () {
            window.location = '/otherscript?cusnum=' + currRow['cusnum'];
        });
        var currCell = tableRow.insertCell(0);
        currCell.innerHTML = currRow['cusnum'];
    }
}

etc.

The problem I'm running into is that ALL of the rows are ending up with listener functions that use the currRow['cusnum'] value from the LAST row added to the table.

JavaScript isn't (and won't likely ever be) my forte - only doing this because there isn't anyone else to do front-end code. Is the problem something to do with using an anonymous function in a loop?

try this $(tableRow).click(function(){ myFunction(); }); if u want to do it with javascript then attach some value with your listener function as argument — user786, May 13 '15 at 07:00
Thankyou for your comment Alex but as I'm only spending as much time working on the javascript end of things as is needed (plenty of other projects I need to spend time on) I'm not really looking to dive into jQuery at the moment (although I know if I do end up doing much more with javascript I probably will look to using jQuery as an overall time saver). I'm assuming the $ notation implies jQuery — Blair, May 13 '15 at 22:54
Not sure how I ever looked at that and didn't see it as a closure (which of course it is - function inside another referring to a variable existing in outer function's namespace) - especially since I [link](http://stackoverflow.com/questions/4430959/beautifulsoup-question/30180309#30180309)had just answered[/link] another question with code involving a closure (albeit a simpler one) hours ago. Goes to show what effect tight deadlines can have on perceptive programming :/ Thanks for showing me the obvious (and how to make the variable local to the function) @myTerminal :) — Blair, May 13 '15 at 23:14

score 1 · Accepted Answer · answered May 13 '15 at 06:58

1

Probably a change like

function addResultsRows(tbody, jsonResults) {
    for (var i in jsonResults) {
        (function (i) {
            var tableRow = tbody.insertRow(0);
            var currRow = jsonResults[i];
            tableRow.onclick = (function() { window.location = '/otherscript?cusnum=' + currRow['cusnum'] });
            var currCell = tableRow.insertCell(0);
            currCell.innerHTML = currRow['cusnum'];
        })(i);
    }
}

would work.

In your scripts, i retains the value from the last iteration in the loop and is the same for all the event handlers.

answered May 13 '15 at 06:58

myTerminal

1,596
1
14
31

The problem is with variable `currRow` (`i` isn't used inside the click handler) but otherwise this is the correct explanation. – JJJ May 13 '15 at 07:00
That does fix it, thankyou very much :) Just so I understand what is happening - the whole operation of adding a row is being turned into an anonymous function, and being autocalled on each iteration of the loop? – Blair May 13 '15 at 07:04
@Juhana - One of the things I tried was changing the currRow reference inside the anonymous function to jsonResults[i] to rule any problems related to that out - sadly that didn't work – Blair May 13 '15 at 07:07
The value of `i` is captured and passed into the closure. In simple terms, the inner `i` does not change when the `i` being iterated on changes. – myTerminal May 13 '15 at 07:07
Again, the closure fixes the problem because it turns `currRow` into a separate variable for each click handler. Passing `i` to the closure isn't even necessary. @Blair see the duplicate or http://stackoverflow.com/questions/111102/how-do-javascript-closures-work for an explanation of what's happening. – JJJ May 13 '15 at 07:11

Adding .onclick listeners in javascript loops

1 Answers1