1

In the code below I'm adding a callback function essentially to a list in a loop.

every Item in the list that I call the call back it will log >>item30

can you please tell me why? Is there a way create a new function() as the call back so that it can log 

item0
item1
item2
item3
item4
and so on .......

......

  for (var j = 0 ; j < 30 ; j += 1) {

      addThisThingtoList(function () {
          console.log( "item" +j );
      });

  }
Hello-World
  • 9,277
  • 23
  • 88
  • 154
  • possible duplicate of [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – 1983 Mar 12 '14 at 18:38

3 Answers3

2

This only happens if your function addThisThingtoList() is using asynchronous behavior (like Ajax calls) and thus its callback is called some time later after the for loop has completely run its course and thus it's index value is at the ending value.

You can fix that with a closure that will freeze the loop value separately for each call to addThisThingtoList() like this:

  for (var j = 0 ; j < 30 ; j += 1) {
      (function(index) {
          addThisThingtoList(function () {
              console.log( "item" + index);
          });
      })(j);
  }

Working demo: http://jsfiddle.net/jfriend00/A5cJG/

By way of explanation, this is an IIFE (immediately invoked function expression). The variable j is passed to the IIFE and it becomes a named argument to the function that I named index. Then, inside that function, you can refer to index as the value of j that is frozen uniquely and separately for each call to addThisThingtoList(). I could have named the argument index to also be j in which case it would have just overriden the higher scoped j, but I prefer to use a separate variable name to be more clear about what is what.

Here's a good reference on the IIFE concept if you want to read more about it:

http://benalman.com/news/2010/11/immediately-invoked-function-expression/

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • @Smeegs - I'm not sure what your jsFiddle was trying to illustrate. Here's a jsFiddle that illustrates the concept by calling the callback sometime later: http://jsfiddle.net/jfriend00/A5cJG/ – jfriend00 Mar 12 '14 at 18:36
  • @jsfriend00 My fiddle illustrates the same thing, jsfiddle runs all the js at load. The `setTimeOut` I added at the end only executes after the load process is complete. ie. the iteration is complete and all the callback functions have been added to the list. Your example uses an arbitrary `sometime` later, while mine is called as soon as the load process is complete. Both essentially doing the same thing, but you chose an arbitrary length of time to wait before calling it. – Smeegs Mar 12 '14 at 18:49
  • @Smeegs - OK, I was confused by your implementation. You were saving an array of function pointers and then calling one of them later. I was calling them all later. I'm curious why you're using `setTimeout()` with no time value passed to it? – jfriend00 Mar 12 '14 at 18:59
  • `setTimeout()` waits for whatever process it's created in to finish before executing. Here's a quick example: http://jsfiddle.net/J9wZ6/ You can see that I write to the log in two 7 item increments. The first one straight up, the second with `setTimeout()` wrapped around the fourth call. As you can see, it get's printed last. Because it waiting for the load process to complete before writing it's part to the console. – Smeegs Mar 12 '14 at 19:12
  • So, in my original fiddle, I use `setTimeout()` to wait for the array to be completely built and the iteration to complete before executing the callback. The use of the `setTimeout()` ensures that all tasks associated with the process have completed. – Smeegs Mar 12 '14 at 19:12
  • @Smeegs - you are calling `setTimeout()` illegally. The delay option (2nd argument to `setTimeout()`) is not documented as optional. I know what it does and how it works, but you are supposed to pass a delay value as the second argument. You can pass `0` if you want the shortest possible delay after the current thread of JS finishes executing and other waiting events are serviced. – jfriend00 Mar 12 '14 at 19:13
  • @jsfriend00 actually, it's not documented as required. There is a lot of debate on whether or not it's actually required. I've yet to find a browser that will break with my implementation. Most browsers will default to their own minimum time. I'm not saying not to add the 0 though. It probably considered better practice to do so. I've just found it completely unnecessary. – Smeegs Mar 12 '14 at 19:19
  • @Smeegs - I stand corrected. In the [latest HTML5 revision](http://www.whatwg.org/specs/web-apps/current-work/multipage/timers.html#timers), the delay time is documented as optional. I personally think that passing `0` makes more readable code (which is what I tend to do when I want it to run as soon as possible after the current JS thread finishes), but apparently the spec says you don't have to pass anything. The code example in the HTML5 spec, passes `0` when they want to run it as soon as possible. – jfriend00 Mar 12 '14 at 19:33
  • @jsfriend00, thanks for the info. I do agree that it makes the code more readable. – Smeegs Mar 12 '14 at 19:37
1

This is what closure is all about.

When you call the function, it pulls the value of j at the time it's called, not at the time when it was added to the list.

So by the time you start calling the functions, j is already at 30.

Smeegs
  • 9,151
  • 5
  • 42
  • 78
0

It's a classic javascript scope bug. You need to pass in the j variable to your function:

addThisThingToList(function(j) {
    console.log("item" + j);
});

As @Smeegs says, "this is what closure is all about."

gstroup
  • 1,064
  • 8
  • 16
  • 1
    You're in the right direction, but this particular implementation won't solve it. The `addThingtoList()` function doesn't know anything about `j` so it can't pass it to the callback. – jfriend00 Mar 12 '14 at 18:23