Callback with additional parameters in loop

Question

I am writing Node.JS lambda function, where I need to invoke some API funciton with callback and pass there additional parameters.

The code looks like:

var s3 = ...;
for (var i = 0; i < data.foo.length; i++) {
    var v1 = data.foo[i].name;
    console.log("Loop: " + v1);
    var params = { ... };            
    foo.method1(params, function(err, data1) { 
         console.log("Method1: " + v1);
         s3.putObject(....);
    });
}

Here are two problems.

I do not understand why, but inside the callback passed to foo.method1... I have always the same value of v1 (I guess that it is the last one from the array).
The Code checker of Amazon console advises me that it is a bad practice to create a function within a loop:

Don't make functions within a loop.

I guess, that p.1 is somehow related to p.2 :-) That is why I've tried to create a named function and pass its reference to foo.method1. But it doesn't work as I can't pass additional parameters v1 and s3 there. I can only wrap its call like:

    foo.method1(params, function(err, data1) { 
          myCallback(err, data1, v1, s3);
    });

what doesn't make sense, as the result is the same.

Hint: foo.method1 is apparently asynchronous.

I doubt how to solve this issue?

`v1` is caught in the closure of each function. All the functions refer to the same, live copy of `v1`, which will be the last value by the time the functions are called. One solution is to use `let`, as described here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Cleaner_code_in_inner_functions — skirtle, Oct 19 '17 at 13:44

JLRishe · Accepted Answer · 2017-10-19T14:33:06.647

2

This is a common problem known as "closing over a loop variable". The problem is that v1 will continue to change while you are waiting for the callback to execute and the v1 that you see will be the last value it takes on.

One way to fix this: bind the v1 variable inside your callback:

var s3 = ...;
for (var i = 0; i < data.foo.length; i++) {
    var v1 = data.foo[i].name;
    console.log("Loop: " + v1);
    var params = { ... };         // v-- extra parameter   
    foo.method1(params, function(v1inner, err, data1) { 
         console.log("Method1: " + v1);
         s3.putObject(....);
    }.bind(null, v1));   // <-- bind
}

A better approach: use .forEach so that a new v1 variable is created for each iteration:

var s3 = ...;
data.foo.forEach(function (datai) {
    var v1 = datai.name;
    console.log("Loop: " + v1);
    var params = { ... };            
    foo.method1(params, function(err, data1) { 
         console.log("Method1: " + v1);
         s3.putObject(....);
    });
});

Edit: If you're looking for a more stress-free way to work with async code, I would suggest looking into Promises, which, as of now, are the future of async code in JavaScript. This would allow you to easily manage your async operations and, for example, execute some code when all of these operations had completed (if you wanted to do so).

edited Oct 19 '17 at 14:33

answered Oct 19 '17 at 13:43

JLRishe

99,490
19
131
169

The problem with `forEach` in this aproach is that you can't attach a callback when all the `items` in the `collection` are uploaded. – Alexandru Olaru Oct 19 '17 at 13:56
@AlexandruOlaru I don't see why it would be any less possible to attach such a callback than any other typical approach (binding, IIFE, function generator). Does the IIFE from your answer allow attaching such a callback in some way that `.forEach` doesn't? – JLRishe Oct 19 '17 at 14:00
So how can you attach an callback at the result of `.forEach` of asynchronous functions ? You need to implement a gate, or just use an existing tool as `async.each(data.foo, putOnS3, finalCallback)`. The IIFE is a response for the first question, `async.each` for the second question, there where 2 different questions. Agree that `bind` also works, for the scope problem. – Alexandru Olaru Oct 19 '17 at 14:05
@AlexandruOlaru Ok, but OP didn't say anything about wanting to execute a callback when all were completed. If they had, I would advise that they use promises. I answered the question they were asking, so I think it's pretty dumb for you to downvote me for not including a line at the end telling them to go use the `async` library. – JLRishe Oct 19 '17 at 14:34
Ok agree with you upvoted! – Alexandru Olaru Oct 19 '17 at 14:42

score 1 · Answer 2 · answered Oct 19 '17 at 13:46

I do not understand why, but inside callback passed to foo.method1... I have always the same value of v1 (I guess that it is last one).

You get the same v1 item cause you are iterating asynchronous functions, so internaly what it does, it is iterating your for, then will make x closures (data.foo.length) with the last i value`

So how can you fix that? You can wrap your i in an IIFE so this will not share the context.

(function(i) {
    var v1 = data.foo[i].name;
    console.log("Loop: " + v1);
    var params = { ... };            
    foo.method1(params, function(err, data1) { 
         console.log("Method1: " + v1);
         s3.putObject(....);
    });
})(i);

This code will create a special scope with the proper i value.

Second method is to use let i instead of var i this also will do the same trick as the above code. More info about let

The Code checker of Amazon console advises me that it is a bad practice to create a function within a loop

To solve this issue I recommend you to look at async npm module. You can find there each.

Callback with additional parameters in loop

2 Answers2