0

I found some questions that seem to relate to this, but at the extend of my understanding, mine is different, so here goes. I have greatly simplified my code and even replaced the one line with a comment, but the concept is the same...

function myFunction(options) {
    var button;
    options.widgets.forEach(function (w) {
        button = document.createElement("button");
        //add button to the DOM
        aListOnMyPage.addEventListener("selectionchanged", function (args) {
            var a = button;
        });
    });
}

So for each of the widgets I am creating a new object and then adding an event listener to a list on the page whose function needs to reference the DOM element (the button) that I created. The behavior I'm seeing is that my button is correctly set at the point where I create the event listener, but when the listener's event fires, button is always a reference to the last button I created. Perhaps it's because it's late, but I can't wrap my head around why it would act like this. Can anyone clarify this for me?

Jeremy Foster
  • 4,673
  • 3
  • 30
  • 49
  • Search for "javascript loop last value" or similar. –  Aug 25 '12 at 06:59
  • e.g. http://stackoverflow.com/questions/6425062/passing-functions-to-settimeout-in-a-loop-always-the-last-value , http://stackoverflow.com/questions/6599157/why-always-the-last-reference-to-the-object-is-used-in-loop etc are the same issue in a slightly different guise. –  Aug 25 '12 at 07:00

2 Answers2

3

Sure can.

Your problem here is actually scope. Let's take a look at the code you've written:

function myFunction(options) {
    var button;
    options.widgets.forEach(function (w) {
        button = document.createElement("button"); // UH-OH...
        //add button to the DOM
        aListOnMyPage.addEventListener("selectionchanged", function (args) {
            var a = button;
        });
    });
}

Notice the uh-oh? You've declared that button should equal document.createElement("button"), but since you aren't using the var keyword, you've declared one global variable, instead of multiple local ones. That means that every time your loop runs, that one global variable is being referenced by your listener, which causes your issue.

Make sure you use a local variable here, like so:

function myFunction(options) {
    options.widgets.forEach(function (w) {
        var button = document.createElement("button"); 
        //add button to the DOM
        aListOnMyPage.addEventListener("selectionchanged", function (args) {
            var a = button;  
        });
    });
}

This should greatly improve your situation. At the very least, I hope it helps you get a step further. Best of luck.

  • 1
    You're correct about the closure, but not about *button* being global. In the OP, it's local to the outer function. – RobG Aug 25 '12 at 08:26
  • Actually, in simplifying my code I removed the var button that was outside the loop, so it wasn't global, but this was exactly my problem. I think I was making it too difficult by expecting it to be a closure issue. Thanks, @lunchmeat317!! – Jeremy Foster Aug 25 '12 at 15:28
1

I think you are facing this problem because of closures, the button variable used throughout you’re myFunction is same, all changes are reference changes, so I think changing the declaration of the variable to inside the anonymous function should solve this problem.

function myFunction(options) {
    //var button;
    options.widgets.forEach(function (w) {
        var button = document.createElement("button");
        //add button to the DOM
        aListOnMyPage.addEventListener("selectionchanged", function (args) {
            var a = button;
        });
    });
}

this change provides the anonymous function in forEach with separate button variables.

Vignesh
  • 315
  • 4
  • 14
  • 1
    The is no need for the *a* variable, *button* in the listener has a closure to the one declared in the outer "each" function so can be used directly. – RobG Aug 25 '12 at 08:30
  • I know, I just tried to keep the code as smiler to his code as possible. – Vignesh Aug 30 '12 at 09:13