-1

This is probably an easy question but it's late at night and I can't get my head round this.

here's my code

$(document).ready(function () {

var items = getNumber();

for (var i = 0; i < items.length; i++) {

    var test = items[i].action;
    test();

}

});

function getNumber()
{

var items = [];

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

    var num = i * 10;

    items.push({ id: i, number: num, action: function () { alert(i) } });
}


return items
}

Could someone explain to me why the alert output is always 5? I want the alert parameter to be the index at the time it is added to the array. It seems like it is being dynamic.

If you could also post a solution how i could get this to work i would be extremely thankful

heymega
  • 9,215
  • 8
  • 42
  • 61
  • There is only *one* variable called `num` for each getNumber execution. So whenever the "action" callback function executes the alert will display the *last value* assigned to the *shared [among action callbacks] variable*. The standard solution is to use a "double closure" to bind/close over to a *new* variable. – user2246674 Sep 03 '13 at 22:13
  • See http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example?lq=1 , http://stackoverflow.com/questions/6425062/passing-functions-to-settimeout-in-a-loop-always-the-last-value *and* the related questions. – user2246674 Sep 03 '13 at 22:15
  • @user2246674 You mean `i`, not `num` (`num` is being copied into the object, `i` is being referenced). – bfavaretto Sep 03 '13 at 22:19
  • @bfavaretto You are correct, I did/do mean `i`. Whoops :> – user2246674 Sep 03 '13 at 22:29

1 Answers1

4

This is a common issue with JavaScript variable scoping: new variables are only introduced in a new execution context and thus, in the problematic code, i is shared across all the action callbacks.

Anyway, here is the corrected code following the standard idiom:

function getNumber()
{
  var items = [];
  for (var i = 0; i < 5; i++) {
    var num = i * 10;    
    items.push({
      id: i, number: num,
      // _i is a new variable for each callback
      action: (function (_i) {
        // use separate (one per callback) _i variable
        return function () { alert(_i) }
      })(i) // pass in current value for loop
    });
  }
  return items
}

Alternatively, if one doesn't like all the nesting, it's fine to use a "named" function to perform the same task. The key to point is that the closure is created (and returned from) a new execution context so that a different variable is closed-over:

function getNumber()
{
  function mkAction (i) {
      return function () { alert(i) }
  }
  var items = [];
  for (var i = 0; i < 5; i++) {
    var num = i * 10;    
    items.push({
      id: i, number: num,
      action: mkAction(i)
    });
  }
  return items
}

Another approach is to use Function.bind from ES5:

function getNumber()
{
  var items = [];
  for (var i = 0; i < 5; i++) {
    var num = i * 10;    
    items.push({
      id: i, number: num,
      action: (function (_i) { alert(_i) }).bind(null, i)
    });
  }
  return items
}

(Note that Function.bind can be emulated using a new execution context/closure even without native browser support. See the MDC documentation.)

See also:

Community
  • 1
  • 1
user2246674
  • 7,621
  • 25
  • 28