Sharing this because it had me confused for half an hour.

I was just working on the new file upload dialog for Zudio 1.1 (I’m replacing Plupload with a custom-written solution which works better with Azure Blob Storage on the server side). The new code is all AngularJS directives with some Angular-UI Bootstrap, including their spiffing progress directive. Except, when I uploaded multiple files, the only progress bar that moved was the last one in the list, and it jumped all over the place. My immediate suspicion, obviously, was “there’s something wrong with Angular-UI”, but then I remembered similar issues when I first started writing LINQ code in C# 3.0 and got caught by the “modified closure trap”.

If you don’t know what that is, consider this code, which is supposed to activate a whole list of things when a button is clicked:

  
[sourcecode language='csharp'  padlinenumbers='true']
foreach (var thing in things)  
{
    button.Click += (o,e) => thing.Activate();
}
[/sourcecode]

What actually happens here is that the Activate method on the last thing in the list will be called (repeatedly?), and the other things’ Activate methods won’t be called at all. That’s because the “thing” variable gets wrapped in a single closure object which is reused each time round the loop, and it ends up pointing to whatever the last thing was.

This was such a common cause of confusion, head-scratching and keyboard abuse that in C# 5 they changed the specification to say that a separate variable should be created each time around the loop, but before that the way to avoid this problem was to do that manually, like so:

  
[sourcecode language='csharp' ]
foreach (var thing in things)  
{
    var thisThing = thing;
    button.Click += (o,e) => thisThing.Activate();
}
[/sourcecode]

Now, when I write this kind of code, I always assign the loop variable to a separate variable, even in C# 5… and also in JavaScript:

  
[sourcecode language='javascript' ]
for (var i = 0; i < things.length; i++) {  
    var thing = things[i];
    button.addEventListener("click", function() {
        thing.activate();
    });
}
[/sourcecode]

Except it doesn’t work in JavaScript, because in JavaScript the var keyword won’t create a new variable within the same scope; var statements are hoisted to the top of the current scope by the runtime. That code above is semantically equivalent to:

  
[sourcecode language='javascript' ]
var i, thing;  
for (i = 0; i < things.length; i++) {  
    thing = things[i];
    button.addEventListener("click", function() {
        thing.activate();
    });
}
[/sourcecode]

In JavaScript, the way I work around this is to refactor the body of the loop into a separate function and pass the variable in; that creates a separate copy of the variable each time round the loop.

  
[sourcecode language='javascript' ]
function setupActivateOnClick(thing) {  
    button.addEventListener("click", function() {
        thing.activate();
    });
}
for (var i = 0; i < things.length; i++) {  
    setupActivateOnClick(things[i]);
}
[/sourcecode]

If you’re the kind of person who likes nested code that’s hard to read 6 days after you wrote it, let alone 6 months, you can do this as an Immediately-Invoked Function Expression:

  
[sourcecode language='javascript' ]
for (var i = 0; i < things.length; i++) {  
    (function (thing) {
        button.addEventListener("click", function() {
            thing.activate();
        });
    })(things[i]);
}
[/sourcecode]

which has the same effect, but with the added excitement of knowing that I’ll kill you if you do shit like that in any of my code. It’s not “functional programming”, it’s just bloody horrible*.

* Yes, IIFEs are a perfectly valid construct in JavaScript, and pretty much all code is and should be contained in them at the top level to prevent random declarations from hitting the global context, and it’s how JavaScript does modules, and all that, but that doesn’t mean you have to use them everywhere, you damn hipster.