1

I have this code:

var queue = [];
var allParserd = [];

_.each(webs, function (web) {
    queue.push(function () {
        WebsitesUtils.parseWebsite(web, function (err, parsed) {
            allParserd.push(parsed);
        });
    });
});

Promise.all(queue).then(function (data) {
    console.log(allParserd);
});

Basically I need to fetch all my webs and be sure to give the result after that every parsing is done. the function parseWebsite return the correct data, but in this way is not called and allParsed return just as an empty array. I'm sure that I miss some things, I've started to use the promises just from some days.

If you need some more information just tell me.

P.s. I want that all the functions to start at the same time; I don't want to wait for each one response for going forward.

Max
  • 2,763
  • 3
  • 16
  • 20
  • 2
    Your parseWebsite function should really be refactored to return a promise, mixing callbacks and Promises can be confusing. Once you've done that you can take advantage of the Promise.map/Promise.each functions etc that the likes of Bluebird provide. – Stefano Sep 29 '14 at 14:04
  • What `Promise` library are you using? – Bergi Sep 29 '14 at 14:35
  • Bluebird, i will add a tag – Max Sep 29 '14 at 14:40

3 Answers3

2

Tagged with Bluebird so let's use it:

First, let's convert your callback API to promises:

  Promise.promisifyAll(WebsitesUtils);

Now, let's use .map to map every item in webs to it being parsed parseWebsite:

Promise.map(webs, function(item){
    return WebsitesUtils.parseWebsiteAsync(item); // note the suffix
}).then(function(results){
    // all the results are here.
}).catch(function(err){
    // handle any errors
});

As you can see - this is trivial to do with Bluebird.

Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
  • Thank for your reply, this solution works too, i've marked the other one as solution just because there are also a general example, other than only Bluebird. Thanks again – Max Sep 30 '14 at 10:54
  • @DannoEterno if you want to learn more about how promisification works see http://stackoverflow.com/questions/22519784/how-do-i-convert-an-existing-callback-api-to-promises :) – Benjamin Gruenbaum Sep 30 '14 at 11:04
  • Also, if Bergi's solution solves your problem by all means do upvote it. The Promise tag is relatively small and the faster Bergi (and other tag regs like jfriend and esailija) gets to a gold badge the better it is for us since he gets more moderation privileges :) – Benjamin Gruenbaum Sep 30 '14 at 11:05
  • Yeah, i know, but i don't have reputation on stackoverflow to give the upvote that he deserve. I've always lurked without the possibility to upvote every solution that i've found on SO :( – Max Sep 30 '14 at 20:47
1

Promise.all doesn't take a queue of functions to execute. It expects an array of promises which represent the results of the many concurrently running (still pending) requests.

The first step is to have a function that actually returns a promise, instead of only executing a callback. We can use

function parseWebsite(web) {
    return new Promise(function(fulfill, reject) {
        WebsitesUtils.parseWebsite(web, function (err, parsed) {
            if (err)
                reject(err);
            else
                fulfill(parsed);
        });
    });
}

or simply use promisification that does this generically:

var parseWebsite = Promise.promisify(WebsitesUtils.parseWebsite, WebsitesUtils);

Now we can go to construct our array of promises by calling that function for each site:

var promises = [];
_.each(webs, function (web) {
    promises.push(parseWebsite(web));
});

or just

var promises = _.map(webs, parseWebsite);

so that in the end we can use Promise.all, and get back our allParsed array (which even is in the same order as webs was!):

Promise.all(promises).then(function(allParsed) {
    console.log(allParsed);
});

Bluebird even provides a shortcut function so you don't need promises:

Promise.map(webs, parseWebsite).then(function(allParsed) {
    console.log(allParsed);
});
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Thank you for your useful explanation, i was missing the part that i needed an array of promises and not of functions. Thank you! – Max Sep 30 '14 at 10:53
0

Here's how might do it with async:

var async = require('async');

var webs = ...

async.map(webs, function(web, callback) {
  WebsitesUtils.parseWebsite(web, callback);
}, function(err, results) {
  if (err) throw err; // TODO: handle errors better

  // `results` contains all parsed results
});

and if parseWebsite() isn't a prototype method dependent on WebsitesUtils then you could simplify it further:

async.map(webs, WebsitesUtils.parseWebsite, function(err, results) {
  if (err) throw err; // TODO: handle errors better

  // `results` contains all parsed results
});
mscdex
  • 104,356
  • 15
  • 192
  • 153
  • Hi mscdex, thanks for your reply. The first solution seems don't work, WebsitesUtils is never called (and it don't return either an error) and i cannot use the second one because is dependent from WebsitesUtils. – Max Sep 29 '14 at 14:32