Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Readibility of anonymous closures in nested loops

Tags:

javascript

A friend of mine got bitten by the all too famous 'anonymous functions in loop' javascript issues. (It's been explained to death on SO, and I'm actually expecting someone to my question as a duplicate, which would probably be fair game).

The issue amounts to what John Resig explained in this tutorial :

http://ejohn.org/apps/learn/#62

var count = 0; 
for ( var i = 0; i < 4; i++ ) { 
  setTimeout(function(){ 
    assert( i == count++, "Check the value of i." ); 
  }, i * 200); 
}

To a new user it should work, but indeed "i always have the same values", they say, hence crying and teeth gnashing.

I explained the issue with a lot of hand waving and some stuff about scopes, and pointed him to some solutions provided on SO or other sites (really, when you know it is such a common issue, google is your friend).

Of course the real answer is that in JS the scope is at the function level. So when the anonymous functions run, 'i' is not defined in the scope of any of them, but it is defined in the global scope, and it has the value of the end of the loop, 4.

Since we've all most likely be trained in languages that use block-level scope, it's yet-another-thing-that-js-does-a-little-bit-different-than-the-rest-of-the-world (meaning of "this", anyone ?)

What bogs me is that the common answer, actually even the one provided by John himself is the following :

var count = 0; 
for ( var i = 0; i < 4; i++ ) (function(i){ 
   setTimeout(function(){ 
     assert( i == count++, "Check the value of i." ); 
   }, i * 200); 
})(i);

Which obviously works and demonstrate mastery of the language and a taste for nested parenthesis that suspiciously makes you look like a LISP-er.

And yet I can't help thinking that other solutions would be much more readible and easier to explain.

This one is just pushing the anonymous closure a bit closure closer to the setTimeout (or closer to the addEventListener is 99.9% of the case where it bites someone) :

var count = 0;
for (var i = 0 ; i < 4 ; i++) {
    setTimeout((function (index) {
        return function () {
           assert( index == count++, "Check the value of i." ); 
        }
    })(i), i*200);
};

This one is about explaining what we're doing, with an explicit function factory :

var count = 0
function makeHandler(index) {
   return function() {
       assert(index == count ++);
   };
};
for (var i = 0 ; i < 4 ; i++) {
   setTimeout(makeHandler(i), i*200);
};

Finally, there is another solution that removes the problem altogether, and would seem even more natural to me (although I agree that it somehow sidesteps the problem 'by accident')

var count = 0;

function prepareTimeout(index) {
   setTimeout(function () {
      assert(index == count++);
   }, index * 200);
};

for (var k = 0; k < 4 ; k++) {
   prepareTimeout(k);
};

Are those solutions entirely equivalent, in terms of memory usage, number of scope created, possible leaking ?

Sorry if this is really a FAQ or subjective or whatever.

like image 930
phtrivier Avatar asked Jul 09 '11 11:07

phtrivier


1 Answers

Solution #1

for (var i = 0; i < 4; i++) (function (i) { 
    // scope #1  
    setTimeOut(function () { 
        // scope #2
    })(i), i*200);
})(i);

is actually quite nice as it reduces indentation and therefore perceived code complexity. On the other hand the for loop does not have its own block which is something jsLint (rightfully, in my opinion) would complain about.

Solution #2

for (var i = 0; i < 4; i++) {
    setTimeout((function (i) {
        // scope #1 
        return function () {
            // scope #2
        }
    })(i), i*200);
};

Is the way I would do it most of the time. The for loop has an actual block, which I find increases readability and falls more into place with what you'd expect from a conventional loop (as in "for x do y", opposed to the "for x create anonymous function and execute it right away" from #1). But that's in the eye of the beholder, really, the more experience you have, the more these approaches start to look the same for you.

Solution #3

function makeHandler(index) {
   // scope #1 
   return function() {
       // scope #2
   };
};
for (var i = 0 ; i < 4 ; i++) {
   setTimeout(makeHandler(i), i*200);
}; 

as you say makes things clearer in the for loop itself. The human mind can adapt easier to named blocks that do something predefined than to a bunch of nested anonymous functions.

function prepareTimeout(index) {
   // scope #1
   setTimeout(function () {
       // scope #2
   }, index * 200);
};

for (var k = 0; k < 4 ; k++) {
   prepareTimeout(k);
};

is the absolute same thing. I don't see any "issue sidestepping" here, it's just equivalent to #3.

As I see it, the approaches do not differ in the least bit, semantically - only syntactically. Sometimes there are reasons to prefer one over the other (the "function factory" approach is very re-usable, for example), but that doesn't apply to the standard situation you describe.

In any case, there are three concepts that a new user of JavaScript must get his head around:

  1. functions are objects and can be passed around like integers (and therefore they don't need a name)
  2. function scope and preservation of scope (how closures work)
  3. how asynchronicity works

Once these concepts have sunken in, there will be a point at which you no longer see such a big difference in those approaches. They are just different ways to "put it". Until then you simply choose the one you are most comfortable with.


EDIT: You could argue that that point is when you transition from a "you must do it like this" attitude to a "you could do it like this, or like this, or like this" attitude. This applies to programming languages, cooking and pretty much anything else.

To say something about the implicit question in the title: Readability is also in the eye of the beholder. Ask any Perl programmer. Or someone comfortable with regular expressions.

like image 116
Tomalak Avatar answered Nov 07 '22 01:11

Tomalak