Scoping issue when attempting to pass variable into object's method constructor within a foreach loop

Question

I understand what's happening in the following block of code, but unsure of how to fix it:

for (var i = 0; i < songs.length; i++){
    var listItem = $('<li/>').appendTo(songList);

    var song = songs[i];

    var link = $('<a/>', {
        id: song.id,
        href: '#' + song.id,
        text: song.name,
        contextmenu: function(e){
            var contextMenu = new ContextMenu(song);
            contextMenu.show(e.pageY, e.pageX);

            //Prevent default context menu display.
            return false;
        }
    }).appendTo(listItem);
}

In this scenario, song, when accessed inside of the contextmenu method, will always be the last item iterated over in the foreach loop. I'm wondering how to make it so that there is a 1:1 relationship between links and their defining songs.

I tried assigning the song object as a property of the link object (accessing via this.song), but it comes up as undefined.

possible duplicate of [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Florian Margaine, Aug 22 '12 at 17:58
Ahh. You are correct -- this is a duplicate. I'm having a bit of trouble extending the logic to my application, though. I'll play around with it and if I figure it out in a minute I'll close it up! — Sean Anderson, Aug 22 '12 at 18:00

score 2 · Accepted Answer · edited May 23 '17 at 10:24

for (var i = 0; i < songs.length; i++){
    // note this
    (function(i) {
        var listItem = $('<li/>').appendTo(songList);

        var song = songs[i];

        var link = $('<a/>', {
            id: song.id,
            href: '#' + song.id,
            text: song.name,
            contextmenu: function(e){
                var contextMenu = new ContextMenu(song);
                contextMenu.show(e.pageY, e.pageX);

                //Prevent default context menu display.
                return false;
            }
        }).appendTo(listItem);
    // and this
    }(i));
}

See this duplicate for an explanation.

Thanks for that. Closures are still a bit weird to me, apparently. :) Cheers. — Sean Anderson, Aug 22 '12 at 18:03

Elliot B. · Answer 2 · 2012-08-22T18:18:26.853

The other answer is the better approach--declaring the object out of the scope of the for loop. But here's how to tackle the problem if declaring the object must be declared in the for loop:

song is declared up one level of scope from your contextMenu function. So whenever contextMenu executes, it will look up the scope chain until it finds the variable, by which point it will always find the last instantiated song. You need to get a copy of song available to the contextMenu, which is also tricky because objects are passed by reference in JavaScript. You can use a self-invoked function in combination with $.extend to make a copy of song (renamed newSong in my example) and solve this problem:

for (var i = 0; i < songs.length; i++){
    var listItem = $('<li/>').appendTo(songList);

    var song = songs[i];

    var link = $('<a/>', {
        id: song.id,
        href: '#' + song.id,
        text: song.name,
        contextmenu: (function(song){
            var newSong = $.extend(true,{},song);
            return function(e) {
            var contextMenu = new ContextMenu(newSong);
            contextMenu.show(e.pageY, e.pageX);

            //Prevent default context menu display.
            return false;
          };
        })(song);
    }).appendTo(listItem);
}

Your answer seems just as valid, but I think I prefer wrapping in a closure as opposed to creating a copy in this way. Just personal preference, I suppose. — Sean Anderson, Aug 22 '12 at 18:12
Our answers both use closures, but I just felt like showing a method where you can solve this problem if referencing an out of scope object is a requirement (which requires `$.extend`). To be sure though, his answer is the better approach in your case. — Elliot B., Aug 22 '12 at 18:13

Scoping issue when attempting to pass variable into object's method constructor within a foreach loop

2 Answers2