0

I have the following code (dbclient is Redis client for Node.js):

dbclient.zrange("cache", -1000000000000000, +1000000000000000, function(err, replies) {
  logger.info("Replies: " + replies.length);
  logger.info(err);

  for (var i = 0; i < replies.length; i++) {
    logger.info("I: " + i)
    dbclient.hmget("doc:" + replies[i], "size", function(err, res) {
    cache.set(replies[i], parseInt(res[0]));
      logger.info(res[0]);
    });
  }
});

I notice a strange behavior:

The first output is: Replies: 195748, but in the for loop I notice that it always prints I: 195747 and the res[0] is always 198536. This run 195747 times.

It seems that it's stuck on the last index, and doesn't iterate over all items.

hexacyanide
  • 88,222
  • 31
  • 159
  • 162
MIDE11
  • 3,140
  • 7
  • 31
  • 53

1 Answers1

3

This is one of the most common errors in Javascript, in

function(err,res){
    cache.set(replies[i], parseInt(res[0]));
    logger.info(res[0])
}

you are using i, but that is always the one from the parent scope. Since the function is run as a callback asynchronously, it is always the value of the last loop iteration.

Change it to

(function(i) {
    return function(err,res){
        cache.set(replies[i], parseInt(res[0]));
        logger.info(res[0])
    };
})(i)

to bind i to the inner-most function's scope

further explanation: JavaScript closure inside loops – simple practical example

vvondra
  • 3,022
  • 1
  • 21
  • 34
  • There is no need to pass `err` and `res` to the closure – Oleg Jun 23 '15 at 08:45
  • You're right, I usually don't like to mix up the closures from which variables are though. – vvondra Jun 23 '15 at 08:47
  • 1
    Also `i` agrument should be taken from the outer function. So it should be: `(function(i) { return function(err, res) { /*...*/ } })(i);` – Oleg Jun 23 '15 at 08:51