6

I'm trying to have a counter increase gradually. The following works:

function _award(points){    
  var step = 1;
  while(points){
    var diff = Math.ceil(points / 10);
    setTimeout( "_change_score_by("+diff+");" /* sigh */,
                step * 25);
    points -= diff;
    step++;
  }
}

However, it uses an implicit eval. Evil! Let's use a closure instead, right?

function _award(points){    
  var step = 1;
  while(points){
    var diff = Math.ceil(points / 10);
    setTimeout( function(){ _change_score_by(diff); },
                step * 25);
    points -= diff;
    step++;
  }
}

Obviously, this doesn't work. All closures created catch the last value diff has had in the function -- 1. Hence, all anonymous functions will increase the counter by 1 and, for example, _award(100) will increase the score by 28 instead.

How can I do this properly?

badp
  • 11,409
  • 3
  • 61
  • 89

3 Answers3

10

This is a known problem. But you can easily create a closure on each loop iteration:

(function(current_diff) {
    setTimeout(function() {_change_score_by(current_diff);},
               step * 25);
})(diff);
Nikita Rybak
  • 67,365
  • 22
  • 157
  • 181
  • Using Functional.js you could create the function to pass to `setTimeout()` like this: `_change_score_by.saturate(diff)` and you wouldn't need the anonymous function. – Pointy Oct 17 '10 at 15:08
  • I guess you'd then have to also pass `step` as a parameter? – badp Oct 17 '10 at 15:10
  • 1
    @badp No. _setTimeout(param1, param2)_ is executed right this moment. Only 'inside' of function _param1_ isn't evaluated (it's executed later, as you noted in your post). – Nikita Rybak Oct 17 '10 at 15:15
  • 1
    I wouldn't really call this a "problem". It's just the way lexical scope is defined in Javascript. – Pointy Oct 17 '10 at 15:18
  • 1
    @Pointy Feature, then :) It's just that probably every js programmer makes this mistake once. (I did, at least) – Nikita Rybak Oct 17 '10 at 15:21
  • *"This is a known problem"* It's not a problem. It's a feature. Without it, JavaScript would be *dramatically* more limited. – T.J. Crowder Oct 17 '10 at 15:22
  • @Nikita - yes, it's certainly one of the more popular Javascript topics on Stackoverflow! – Pointy Oct 17 '10 at 16:12
  • Word! Just did this myself.. :S – myme Dec 18 '10 at 19:58
3

Solving the Closure Loop Problem gets less messy with ECMAScript Fifth Edition's Function#bind method:

setTimeout(_change_score_by.bind(window, diff), step*25);

You can hack this feature into browsers that don't support it yet to get the benefit now.

Community
  • 1
  • 1
bobince
  • 528,062
  • 107
  • 651
  • 834
2

Nikita's approach works (thanks!), but personally I prefer changing it to this more explicit way:

function _schedule_score_change(diff, step){
  setTimeout( function(){ _change_score_by(diff) },
              step*25 );
}

function _award(points){    
  var step = 1;
  while(points){
    var diff = Math.ceil(points / 10);
    _schedule_score_change(diff, step);
    points -= diff;
    step++;
  }
}
Community
  • 1
  • 1
badp
  • 11,409
  • 3
  • 61
  • 89