Flock Dawson
Flock Dawson

Reputation: 1878

Use of promises in loop (node)

I'm working on a NodeJS backend service, and use promises to load any data. Now, I want to make a combination (array) of items coming from different sources. I have the following solution, but I don't think this is the right way to get around something like this.

var results = [];
loop(items, index, data, results).then(function() {
    console.log(results);
});

function loop(items, index, data, results) {
    return utils.getPromise(function(resolve, reject) {
        if (index === items.length) {
            // Stop
            resolve();
        } else {
            doAction(items[index], data).then(function(result) {
                if (result) {
                    mergeResults(results, result)
                } else {
                    loop(items, index + 1, data, results).then(resolve, reject);
                }
            }, reject);
        }
    });
}

function doAction(item, data) {
    return utils.getPromise(function(resolve, reject) {
        item.doIt(data).then(resolve, reject);
    });
}

I think the right way would be to return a promise immediately, and add results on the fly, but I don't know exactly how to do this. Any suggestions?

Upvotes: 0

Views: 1013

Answers (2)

Keith
Keith

Reputation: 24181

function randTimeout(t) {
  return new Promise(function (resolve) {
    setTimeout(function () {
      console.log('time done:' + t);
      resolve();
    }, t);
  });
}

var timeouts = [];
for (var l = 0; l < 20; l ++) {
  timeouts.push(Math.random() * 1000 | 0);
}

console.log('start');
Promise.map(timeouts, function (t) {
  return randTimeout(t);
}, {concurrency: 3}); 
<script src="https://cdnjs.cloudflare.com/ajax/libs/bluebird/3.4.6/bluebird.min.js"></script>

Like @Tobi says, you can use Promise.All..

But Promise.all has one gotcha!!.

If the array your processing has lots of items, sending lots of items to go off and do some async work at the same time might be sub-optimal. And in some cases impossible, eg. opening too many files at once in node will get you a too many handles error.

The Bluebird promise library has a useful function called map, and has an option called concurrency.. This allows you to process X amounts of promises at a time. Personally I think the map function with a concurrency option should be part of the Promise spec, as it's so useful. Without using the concurrency option it would have the same effect as doing promise.all, without having to make an array first.

In the above example I created 20 random timeouts, of max of 1 second. Without using map the maximum time doing this would be 1 second, as all promises would be executed at once. But using the concurrency option of 3, it's doing 3 at a time, and as you can see takes longer than 1 second to complete.

Upvotes: 1

Tobi
Tobi

Reputation: 524

You could use Promise.all to gather all the results from the promises. The .all promise itself is then resolved with an array containing all the single results from each promise.

I think the right way would be to return a promise immediately

Like you suggested you would just return the promise and create an array from these promises. After the loop you give this array into Promise.all.

Might look like this:

var promiseArray = [];
for(var i=0;i<array.length;i++){
    promiseArray.push(doSomethingAsync(array[i]));
}
Promise.all(promiseArray).then(function(responseArray){
    //do something with the results
});

Upvotes: 6

Related Questions