Closing over the loop variable is just as harmful in JavaScript as it is in C#, and more cumbersome to fix


Prerequisite reading: Closing over the loop variable considered harmful.

JavaScript has the same problem. Consider:

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  document.getElementById("myButton" + i)
   .addEventListener("click",
         function() { alert(i); });
 }
}

The most common case where you encounter this is when you are hooking up event handlers in a loop, so that's the case I used as an example.

No matter which button you click, they all alert 4, rather than the respective button number.

The reason for this is given in the prerequisite reading: You closed over the loop variable, so by the time the function actually executed, the variable i had the value 4, since that's the leftover value after the loop is complete.

The cumbersome part is fixing the problem. In C#, you can just copy the value to a scoped local and capture the local, but that doesn't work in JavaScript:

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  var j = i;
  document.getElementById("myButton" + i)
   .addEventListener("click",
         function() { alert(j); });
 }
}

Now the buttons all alert 3 instead of 4. The reason is that JavaScript variables have function scope, not block scope. Even though you declared var j inside a block, the variable's scope is still the entire function. In other words, it's as if you had written

function hookupevents() {
 var j;
 for (var i = 0; i < 4; i++) {
  j = i;
  document.getElementById("myButton" + i)
   .addEventListener("click",
         function() { alert(j); });
 }
}

Here's a function which emphasizes this "variable declaration hoisting" behavior:

function strange() {
 k = 42;
 for (i = 0; i < 4; i++) {
  var k;
  alert(k);
 }
}

The function alerts 42 four times because the variable k refers to the same variable k throughout the entire function, even before it has been declared.

That's right. JavaScript lets you use a variable before declaring it.

The scope of JavaScript variables is the function, so if you want to create a variable in a new scope, you have to put it in a new function, since functions define scope.

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  var handlerCreator = function(index) {
   var localIndex = index;
   return function() { alert(localIndex); };
  };
  var handler = handlerCreator(i);
  document.getElementById("myButton" + i)
   .addEventListener("click", handler);
 }
}

Okay, now things get weird. We need to put the variable into its own function, so we do that by declaring a helper function handler­Creator which creates event handlers. Since we now have a function, we can create a new local variable which is distinct from the variables in the parent function. We'll call that local variable local­Index. The handler creator function saves its parameter in the local­Index and then creates and returns the actual handler function, which uses local­Index rather than i so that it uses the captured value rather than the original variable.

Now that each handler gets a separate copy of local­Index, you can see that each one alerts the expected value.

Now, I wrote out the above code the long way for expository purposes. In real life, it's shrunk down quite a bit.

For example, the index parameter itself can be used instead of the local­Index variable, since parameters can be viewed as merely conveniently-initialized local variables.

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  var handlerCreator = function(index) {
   return function() { alert(index); };
  };
  var handler = handlerCreator(i);
  document.getElementById("myButton" + i)
   .addEventListener("click", handler);
 }
}

And then the handler­Creator variable can be inlined:

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  var handler = (function(index) {
   return function() { alert(index); })(i);
  document.getElementById("myButton" + i)
   .addEventListener("click", handler);
 }
}

And then the handler itself can be inlined:

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  document.getElementById("myButton" + i)
   .addEventListener("click",
       (function(index) {
         return function() { alert(index); })(i));
 }
}

The pattern (function (x) { ... })(y) is misleadingly called the self-invoking function. It's misleading because the function doesn't invoke itself; the outer code is invoking the function. A better name for it would be the immediately-invoked function since the function is invoked immediately upon definition.

The next step is to change then the name of the helper index variable to simply i so that the connection between the outer variable and the inner variable can be made more obvious (and more confusing to the uninitiated):

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  document.getElementById("myButton" + i)
   .addEventListener("click",
       (function(i) {
         return function() { alert(i); })(i));
 }
}

The pattern (function (x) { ... })(x) is an idiom that means "For the enclosed block of code, capture x by value." And since functions can have more than one parameter, you can extend the pattern to (function (x, y, z) { ... })(x, y, z) to capture multiple variables by value.

It is common to move the entire loop body into the pattern, since you usually refer to the loop variable multiple times, so you may as well capture it just once and reuse the captured value.

function hookupevents() {
 for (var i = 0; i < 4; i++) {
  (function(i) {
   document.getElementById("myButton" + i)
    .addEventListener("click", function() { alert(i); });
  })(i);
 }
}

Maybe it's a good thing that the fix is more cumbersome in JavaScript. The fix for C# is easier to type, but it is also rather subtle. The JavaScript version is quite explicit.

Exercise: The pattern doesn't work!

var o = { a: 1, b: 2 };
document.getElementById("myButton")
 .addEventListener("click",
   (function(o) { alert(o.a); })(o));
o.a = 42;

This code alerts 42 instead of 1, even though I captured o by value. Explain.

Bonus reading: C# and ECMAScript approach solving this problem in two different ways. In C# 5, the loop variable of a foreach loop is now considered scoped to the loop. ECMAScript code name Harmony proposes a new let keyword.

Comments (38)
  1. Joshua says:

    OK C# is weird. Expected block scope but function lifetime but then again natural methods of compilation choke on returning closures.

  2. Matt says:

    Because Javascript values are references, and hence the "value" of o is the reference to the object, whose "a" field you changed to 42.

  3. Mark says:

    This is why jQuery.each is such a godsend.

  4. David says:

    Actually, there's a significantly shorter version:

    function hookupevents() {

    for (var i = 0; i < 4; i++) {

      document.getElementById("myButton" + i)

       .addEventListener("click", function(i) { alert(i); }.bind(this, i));

     }

    }

    }

    (This also, incidentally, removes the need for the 'var self = this' hack and lets you refer to this inside the closure just fine.) In the future, you'll be able to do the same fix as C# by declaring your variables with let rather than var. If you iterate with Array.prototype.forEach rather than a for loop, you also avoid this problem.

    [Ah, bind. I wish more people knew about bind. -Raymond]
  5. alegr1 says:

    JavaScript: FORTRAN of XXI century.

  6. Tim says:

    I can deal with a lot of JavaScript's faults, and I think they're exaggerated in many cases, but the function scoping and variable hoisting are just awful design decisions and cause problems almost every time I write JavaScript.

  7. Dan Bugglin says:

    And my previous post was too long so I split it.

    Exercise: If you had overwritten the ENTIRE o object, you'd get 1, since you'd be clobbering the o object with a new value and the old o would still exist as the variable you passed into the function.  But instead you are changing the object's property.  Javascript arguments are always passed by reference; even primitives:

    var a = 1;

    (function(a) {

     setTimeout(function() {

       console.log(a.b);

     });

    })(a);

    a.b = 2;

    This outputs "2".  If numbers were passed by value, you'd get "undefined".

  8. Dan Bugglin says:

    @David Best part is it's easy enough to write a shim for legacy IE browsers if you need to do that.

    jQuery even has a shim built in: $.proxy.

    @Tim Easy enough to avoid though… just don't use closure.  There are usually "proper" ways to pass stuff around unless you're using some of the behaviors of closure to do stuff like simulate static private variables for a class.

    But then debugging tools can't see those variables, since they are out of scope.  Would be nice to have proper JS support for classes… but then again I have to code for IE7/8 so it's not like I'd be using them any time this decade…

  9. Blank says:

    The real way to do this in JS is:

    function hookupevents() {

    for (var i = 0; i < 4; i++) {

     let localIndex = i;

     document.getElementById("myButton" + localIndex)

      .addEventListener("click", function() { alert(localIndex); });

    }

    }

    Of course, if you can shuffle things around a bit, you can do it like this:

    function hookupevents() {

     for (let button of document.getElementsByClass("ourbuttons"))

       button.addEventListener("click", () => alert(button.getAttribute("index"));

    }

    but then you wouldn't be having the issue at hand anyway. (You would, instead, be having the issue of your script not working on anything that's not Firefox, which you could alleviate somewhat by using Array.prototype.forEach.call(), but that's just nasty.)

  10. Kevin says:

    Python has this problem too.  Variables are just names, so like with Javascript, you can't just create a "new" variable.  The most common technique is to exploit another gotcha to cancel the first gotcha out.  Namely, function argument default values are only evaluated once, when the function is declared.  Normally, this causes problems for people who want to use a mutable value as the default for an argument.  But in this case, it means we can force eager evaluation of the loop variable:

    l = []

    for i in range(10):

       l.append(lambda x=i: x)

  11. prprcupofcoffee says:

    Amazing. Javascript has the same variable declaration semantics as VB6 – a variable declared anywhere in a function can be used anywhere in the function.

  12. John Doe says:

    @The MAZZTer, @Mike Caron, you're both right, kind of.

    Old JScript (and that means IE in <= 8 mode) will clobber the enclosing block or the global open. It clobs the name of a named function expression when executed.

    But even from JavaScript 1.5 / ECMAScript 3rd Edition, it shouldn't.  It's a bug you'll have to deal with when supporting old IE.

  13. Ancient_Hacker says:

    All this internal variables to a closure makes my brain ache.

    How about we just revert back to plain unassailable TEXT so we are SURE that a constant is being used, something like:

    for (var i = 0; i < 4; i++) {

      var func = eval(      "function() { alert(" + i + "); }"    );

      document.getElementById( "myButton" + i ).addEventListener( "click", func );  

    }

  14. Christian says:

    The example in the exercise alerts 42 because the function gets the same instance "o" and the last line changes a field of the *same* instance. Instead of using another instance.

  15. Damien says:

    You captured the object *reference* by value, not the object itself.

  16. Dan Bugglin says:

    Ooh I write JS for a living, this stuff is easy for me.

    JavaScript closure (allowing you to embed functions inside functions and use variables from higher up) is powerful but as you showed can be confusing if you're not aware of the intricacies.

    Another example, let's say you have the following code:

    $(".jqueryrocks").each(function() {

     open = $(this).attr("open");

     if (open) {

       $(this).removeAttr("open");

       // do something with the value

     }

    });

    Since open is undeclared, the JS engine looks through closure for a declared variable.  When it can't find one, it uses the property on the global object (window), creating it if it didn't exist.

    But it already exists.  Congrats, your undeclared variable has just blown away the built-in function window.open.  This is a fun bug if your code uses a window property, especially if the code that clobbers it or the code that uses is only called under specific conditions, and especially if the property value is simple enough that you can't immediately recognise where it came from.  I have seen a lot of window.i in my day, who knows where those are getting set from.

    A more confusing variant, though it doesn't have to do with closure:

    Let's say you have this code:

    SomeClass.prototype.open = function() {

     /* … */

    }

    Now, let's say someone else gets their hand on this code, and their fancy text editor has a feature that allows you to view all function names in the file… but this function has no name and the editor is not smart enough to figure out it is "open" so it does not show up properly.

    So they figure, "I'll just do this!"

    SomeClass.prototype.open = function open() {

     /* … */

    }

    Again… you've clobbered window.open.  As a bonus, on legacy versions of it, it looks like IE creates two instances of your function for both places!  I worked on a project where this was standard syntax for DreamWeaver support (though everyone stopped using it long ago).  Fun.

    As for the stuff in this post… I have a different method of solving this problem:

    function hookupevents() {

    for (var i = 0; i < 4; i++) {

     var element = document.getElementById("myButton" + i);

     element.index = i;

     element.addEventListener("click", function() { alert(this.index); });

    }

    }

    Though if I could I'd use jQuery, DOM work is so much easier:

    function hookupevents() {

    for (var i = 0; i < 4; i++) {

     $("#myButton" + i).data("index", i).click(function() { alert($(this).data("index")); });

    }

    }

    Beautiful.  To be fair I'm not sure what .data() does that makes it's verboseness worth it… however when you clone nodes you lose custom properties, and I assume based on how I know .data() works you end up duplicating the data on node clone.

    As long as I'm here:

    function hookupevents() {

     // Assuming we can just add a css class onto all buttons.  Look, we can add as many buttons as we want now!

     $(".myButton").click(function() { alert($(this).data("index")); }).each(function(index) {

       $(this).data("index", index);

     });

    }

  17. Kevin says:

    @Ancient_Hacker:

    Sometimes, the variable you want to close over can't be trivially converted to text and back again.

  18. Mike Caron says:

    @The MAZZter

    > SomeClass.prototype.open = function open() {

    >  /* … */

    > }

    >

    > Again… you've clobbered window.open.

    That doesn't do what you claim it does. It allows the function to invoke itself by name (open()) rather than through the class you were assigning to (someClassInst.open()). It doesn't touch window.open, although it does hide it if you don't qualify which open you are referring to.

  19. SomeGuyOnTheInternet says:

    VB.NET 2012 Winforms example… (VB.NET gives a compiler warning on the MsgBox line about it, yay VB.NET!)

    Public Class Form1

     Sub New()

       ' This call is required by the designer.

       InitializeComponent()

       ' Add any initialization after the InitializeComponent() call.

       For i As Integer = 1 To 4

         Dim btn As Button = DirectCast(Me.Controls.Find("Button" & i.ToString, True)(0), Button)

         Dim ce = Sub(sender As Object, e As EventArgs)

                    MsgBox(i.ToString)

                  End Sub

         AddHandler btn.Click, ce

       Next i

     End Sub

    End Class

  20. cheong00 says:

    @The MAZZTer: For the same reason you had better not to create methods with the same name as document/window members. If you have to you should always attach a prefix to it.

  21. dsn says:

    @ancient_hacker

    What if we were iterating over strings, and someone put in a string like "); doBadStuff("?

  22. whatever says:

    There's one more option that I like even more and that's a partially applied function for these cases.

    Some JavaScript libraries (Prototype.js?) have a function for creating them (in JavaScript) already, and it's not that complicated to roll your own anyway.

  23. John says:

    Is C++ the only language where you get to choose if you want to capture each variable by reference or value?

    Avoids all these problems. (And probably introduces a load of new ones :P )

  24. Neil says:

    The Java version… well, using inner classes, since I don't know whether Java has closures yet, avoids the problem by requiring captured variables to be final. This pretty much requires you to make a local copy inside the loop.

    And here's an ugly JavaScript version for completeness:

    function Alerter(i) {

     this.i = i;

    }

    Alerter.prototype.handleEvent = function(e) {

     alert(this.i);

    };

    function hookupevents() {

     for (var i = 0; i < 4; i++) {

       document.getElementById("myButton" + i)

               .addEventListener("click", new Alerter(i));

     }

    }

  25. DWalker says:

    John:  "Is C++ the only language where you get to choose if you want to capture each variable by reference or value?"  In VB.Net, functions and subroutines have a ByRef and ByVal keyword for each variable on the parameter definition.  Likely in C# too.

  26. Gabe says:

    DWalker: Variables captured by a closure are not formal parameters to the function. C# does not allow you to specify that a captured variable is copied at the time of closure creation, and I suspect VB does not either.

  27. voo says:

    @The MAZZter: None of the examples given have really anything to do with the "intricacies" of closures. Closures are a perfectly simple concept. It's just that JavaScript's idea of scoping is particularly stupid and don't even start talking about implicit globals. God be thanked the latest standard fixes some of the most egregious mistakes.

    On a side note: Poor Brendan Eich. He's a clever guy and considering the circumstances he actually did a really good job (you come up with a language in a week and implement it). Still in the end most people will remember him for creating one of the worst languages ever specified.

  28. 640k says:

    Is the .net language powershell it's possible to explict call GetNewClosure() whenever you want to close a scriptblock over variables.

    Unfortunately the closure is assosiated with the scope of the module, not with the code, a design fail which makes closures useless.

  29. John Doe says:

    @voo, that, the 640k enough for everyone and the reality distortion field.  What do they have in common?

  30. Azarien says:

    The breaking change in C# was evil.

    You don't change language semantics like that. Now nothing is certain. The next version may change the meaning of int.

  31. voo says:

    @Azarien: Wait you wrote honest-to-goodness code that *relied* on the changed behavior? Seriously?

    Please post it, I have to see that.

    @John Doe: I guess the first two are both generally taken out of context these days, but since the "10 days to implement the first version" is a quote from Brendan Eich himself and I haven't see any proof that he invented this to look better in retrospective, I'm really not sure what your point is.

  32. Azarien says:

    @voo: no I didn't, at least in production. Once I wrote a simple program with lambda in a loop to see what would happen. Something happened, and I remembered this (original) behavior. Later I found that they broke what I've learned in a new release ;-)

  33. n says:

    function hookupevents() {

    for (var i = 0; i < 4; i++) {

     document.getElementById("button" + i)

      .addEventListener("click",alert.bind(window,i));

    }

    }

  34. Eamon Nerbonne says:

    There's a syntax error in many of or immediately evaluated functions expressions; you're missing the "};" closing token for the returned handler.

    e.g.

      function hookupevents() {

        for (var i = 0; i < 4; i++) {

          document.getElementById("myButton" + i)

            .addEventListener("click",

              (function(index) {

                return function() { alert(index); })(i));

        }

      }

    should be

      function hookupevents() {

        for (var i = 0; i < 4; i++) {

          document.getElementById("myButton" + i)

            .addEventListener("click",

              (function(index) {

                return function() { alert(index); };})(i));

        }

      }

    or something similar.

  35. Adam says:

    Eric, you said that for this bit of code:

    function strange() {

    k = 42;

    for (i = 0; i < 4; i++) {

     var k;

     alert(k);

    }

    }

    You're using the variable k before declaring it, but really what you're doing is just declaring k in global scope (not function scope because you omitted the var keyword), then changing its scope to the function by putting var k in the loop.

    You can test for whether or not k is in global scope with this snippet:

      console.log(Object.keys(window));

    With this, you can see that when you write "k = 42" followed by "var k", k is not in global scope. But if you wrote just "k = 42", k is suddenly in global scope.

    [You yourself confirmed that in the function "strange", there is no k in global scope, so I'm not sure what you're trying to say. -Raymond]
  36. John Doe says:

    @voo, he prototyped it in 10 days with lots of experience.  Let's disseminate this:

    – Prototype.  It was slow, it didn't do closures (inner functions were only valid for the duration of the outer function), it didn't have Unicode support, time and date support sucked, etc.  And as far as we can all tell, we (the general public) have never seend this first prototype, so I'm basically talking about JavaScript 1.0, which was definitely not done in 10 days; just think about GC.

    – Experience.  I remember Eich saying he used to implement languages for fun.  And professionally, he'd done a couple of times before, so parsing, byte-compiling, interpreting, etc. was not really a quest for him.  I admit that 10 days is still quite a feat, but if you add it up, it's two full weeks, and it was a prototype.  He admittedly borrowed lots of ideas from other languages: Java (most of the syntax, he says he was forced to do it), Self (only the idea of object prototyping) and Scheme (only the idea of first-class functions).

  37. hagenp says:

    Can you use an expression to force the current value of 'i' being used?

    Like this:

    function hookupevents() {

    for (var i = 0; i < 4; i++) {

     document.getElementById("myButton" + i)

      .addEventListener("click",

            function() { alert(i + 0); });

    }

    }

    [Try it and see. (The expression will be evaluated when the function is invoked, i.e., when the click occurs. It will then calculate i + 0 based on the value of i at the time, which will probably be 4.) -Raymond]
  38. imma says:

    @hagenp No, not like that. You've just used the value of i at time of the listener function execution … plus 0

    Do this instead, like suggested further up :

    function hookupevents() {

     for (var i = 0; i < 4; i++)

       function(index){

         document.getElementById("myButton" + index).addEventListener(

           "click",

           function() {

             alert(index);

           });

       }(i);

     }

    }

Comments are closed.