Search code examples
javascriptclosurescode-readability

What are the alternatives to creating a closure here?


Suppose we have a list of simple objects:

var things = [
  { id: 1, name: 'one' },
  { id: 2, name: 'two' },
  { id: 3, name: 'three' }
];

And we need to iterate through these objects and register them as parameters for some later event. The naive approach shares the same object reference between all callbacks, so each fires against the last item:

for (var i = 0; i < things.length; i++) {
  var o = things[i];
  setTimeout(function() { doSomethingWith(o); }, i * 1000);
}

A typical solution is to create a closure, limiting the scope of our object reference:

for (var i = 0; i < things.length; i++) {
  (function() {
    var o = things[i];
    setTimeout(function() { doSomethingWith(o); }, i * 1000);
  })();
}

If we're not targeting IE<9, we could depend on .forEach():

things.forEach(function(o, i) {
  var o = things[i];
  setTimeout(function() { doSomethingWith(o); }, i * 1000);
});

But, we've ultimately created a sort of closure in this case anyway in passing an anonymous function to forEach().

Is there a way to accomplish this without a closure or function reference?

The underlying problem is three-fold:

  1. Less importantly: The function reference passed into setTimeout() (or whatever it may be) makes you (me) feel like you're creating a closure. So, I have a tendency to forget the outer closure.
  2. More importantly: The additional function/closure declarations encourage "arrow code." For complex operations, code readability for complex operations pretty quickly deteriorates as code migrates off the screen ... This is potentially addressed by .forEach() in IE>9, unless the application or organizational style guide dictates a newline + indent for the closure.
  3. Most importantly: I was pretty sure there was a simple way to handle this. And I feel silly for not being able to think of it right now.

Maybe the better way to ask it is this: What the devil did we all do before we all started compulsively creating closures?


Solution

  • I don't think there's anything wrong with using closures here. They are the natural tool in javascript, and are rather necessary for asynchronous callbacks with local states - as we want to avoid global state.

    If you care that much about indentation, you can put the scope-providing IEFE on the same line as the loop:

    for (var i = 0; i < things.length; i++) (function() {
      var o = things[i];
      setTimeout(function() { doSomethingWith(o); }, i * 1000);
    }());
    

    Otherwise, you've already used forEach perfectly fine. Notice that you don't need to care about IE<=8 in your code, as forEach is trivially shimmable if you want to support it.

    And of course, ES6 will give us let statements that solve this very common problem with a new syntax - you'll need to use a 6to5-transpiler though:

    for (let i = 0; i < things.length; i++) {
    //   ^^^
      var o = things[i];
      setTimeout(function() { doSomethingWith(o); }, i * 1000);
    }
    

    If you want a very clear code organisation, make the closure explicit:

    function makeDoer(thing) {
        return function() { doSomethingWith(thing); };
    }
    for (var i = 0; i < things.length; i++) {
        setTimeout(makeDoer(things[i]), i*1000);
    }
    

    What the devil did we all do before we all started compulsively creating closures?

    We used global state, and solved our problems differently. For example, your case would tackled by a semi-recursive function much better:

    var i = 0;
    function next() {
        if (i < things.length) {
            doSomethingWith(things[i++]);
            setTimeout(next, 1000);
        }
    }
    next();