JavaScript Gotcha: Closures and Loops

In this post I discuss a nasty gotcha with closures in JavaScript when working with loops. If you’re an experienced JavaScript developer, you’ve probably run into this before, but if, like me, you come from a C# background, you may be in for a nasty surprise.

In C#, it just works

So although this is a post about JavaScript, I’m going to start off with a C# code example. Suppose you have an array of strings, and want to create a button in your UI for each string. When the user clicks the button, they see a message that includes the text on the button. In C# with Windows Forms you could achieve this with the following code:

var f = new Form();
var options = new string[] { "Hello", "World" };
for(int n = 0; n < options.Length; n++)
{
    var message = options[n];
    var b = new Button();
    b.Location = new Point(10, n * 30);
    b.Text = message;
    b.Click += (s,a) => MessageBox.Show("you clicked on " + message);
    f.Controls.Add(b);
}
f.ShowDialog();

When you run this, you’ll see two buttons and the correct message for each button. The reason for this is that the message variable that the click event handler closes around is different for each time through the loop.

By the way, if you’d done something like this instead:

b.Click += (s,a) => MessageBox.Show("you clicked on " + options[n]);

Then you would have got an index out of range exception when you clicked the button, because there is only one instance of the n variable, and it contains the value of 2 by the time you get round to clicking either of the buttons. So I’m not saying that you can’t run into closure related issues with C#, but that declaring a variable inside your loop is a safe way to avoid them.

In JavaScript, it doesn’t work

So if we recreate our C# example in JavaScript with a bit of JQuery we get something like this:

var options = ["hello", "world"];
var menu = $("#menu");

for (var n = 0; n < options.length; n++) {
    var message = options[n];
    var b = $('<button/>');
    b.text(message);    
    b.click(function() {
        window.alert('you clicked ' + message);
    });
    
    menu.append(b);
}

If you understand closures in C#, you expect this to work. The click handler should close around the message variable. And it does. The trouble is, the message variable is not scoped to the inside of the for loop. It is scoped at the function level. So there is just one variable whose value gets updated each time you go through the loop. This means that all the buttons will report that you clicked on the last option.

So how can you fix this? Well it turns out that there are a few ways.

Option 1 – Use an IIFE

We can get our message variable inside a function scope by using the “immediately invoked function expression” pattern. That is, we put all the code inside our loop into a function, and then call that function immediately. Here’s what that looks like:

for (var n = 0; n < options.length; n++) {
    (function() {
        var message = options[n];
        var b = $('<button/>');
        b.text(message);    
        b.click(function() {
            window.alert('you clicked ' + message);
        });
        
        menu.append(b);
    })();
}

This is fine, but in my opinion, this looks a but ugly. We’ve introduced an extra level of indentation to work around the issue. So a preferable option would be to refactor the inner body of the loop out into its own function, and call that. This is actually a refactoring you should be very familiar with if you try to write “clean code”.

Option 2 – Extract inner body of loop to a function

var addMessageButton = function(message) {
   var b = $('<button/>');
    b.text(message);    
    b.click(function() {
        window.alert('you clicked ' + message);
    });    
    menu.append(b);
}
for (var n = 0; n < options.length; n++) {
    addMessageButton(options[n]); 
}

This makes the for loop much more readable, and reduces cyclomatic complexity, which is always a good thing. There’s one final enhancement we can make use of, and that’s to use the map method.

Option 3 – Use the array.map method

Having extracted the inner body of the loop to a function, we can clean up the code one last step by eliminating the loop altogether, and use the map method on an array to give us a much more functional approach:

options.map(addMessageButton);

The end result is cleaner, easier to read code, and avoids the original closure issue. I’m sure there are other ways of resolving this issue in JavaScript (I think another is to make use of the bind method), but I’m quite happy with this approach. Let me know in the comments if there’s an even better way.