CODEWITHSUNDEEP
javascriptnode.jsasynchronouspromise
Sujeet
Sujeet

Reputation: 3810

map() with async vs promise.all()

If I have an array of elements and I want to do parallel operations on them.

I would use promise.all().

I knew promise.all() accepts array of promises. Correct me if I am wrong, I don't think so.

Here, it clearly says.

The Promise.all() method returns a single Promise that fulfills when all of the promises passed as an iterable have been fulfilled or when the iterable contains no promises or when the iterable contains promises that have been fulfilled and non-promises that have been returned. It rejects with the reason of the first promise that rejects, or with the error caught by the first argument if that argument has caught an error inside it using try/catch/throw blocks.

So, yes we can pass simple functions to promise.all(), and it resolves if they return and rejects if they throw an error.

Now look at the below code. 

const promises = todayAssignedJobs.map(async todayAssigned => {
  const [leaderboard, created] = await Leaderboard.findOrCreate({...});

  if (!created) {
    const rating = (todayAssigned.rating + leaderboard.rating * leaderboard.jobs_completed) / (leaderboard.jobs_completed + 1);
    const commission = todayAssigned.commission + leaderboard.commission;
    const jobsCompleted = leaderboard.jobs_completed + 1;

    await Leaderboard.update({
      rating,
      commission,
      jobs_completed: jobsCompleted,
      updated_by: 'system',
    }, {
      where: {
        id: leaderboard.id,
      },
    });
  }

  await AssignedJob.update({
    is_leaderboard_generated: true,
  }, {
    where: {
      id: todayAssigned.id,
    },
  });
});

await Promise.all(promises);

Here, I have a doubt.

We are iterating on each element of the array and doing operation on them asyncrhonously. They don't return anything explicitly.

So, I think map is doing parallel operations here too.

Why shuold then use promise.all() here?

Upvotes: 5

Views: 10569

Answers (3)

CertainPerformance
CertainPerformance

Reputation: 370819

You only need Promise.all if you want to do something when all operations are complete. For example:

const promises = todayAssignedJobs.map(async todayAssigned => {
  // lots of async stuff
});

await Promise.all(promises);

// now, all Promises have resolved
// alert the user that the leaderboard is completely updated

If you don't need anything to occur once you've determined that all Promises have completed, then there's no point to the Promise.all - you may as well just create the Promises in a loop and leave them as-is. In that case, since you wouldn't be using the resulting array of Promises, it would be more appropriate to use forEach (which is the array method to use for side-effects).

There's one issue, though - you aren't handling errors, but errors should be handled, otherwise they'll give warnings or exit the Node process:

const processJob = async (todayAssigned) => {
  const [leaderboard, created] = await Leaderboard.findOrCreate({...});

  if (!created) {
    // ...

// ...
todayAssignedJobs.forEach(async todayAssigned => {
  try {
    await processJob(todayAssigned);
  } catch(e) {
    // handle errors
  }
});

Upvotes: 4

Paul
Paul

Reputation: 141827

The purpose of Promise.all here is to be able to await all the promises before continuing.

If you added a console.log immediately before the await Promise.all(promises); it would run synchronously before any promises have resolved, whereas a console.log immediately after that line will only appear after all the promises have resolved.

Upvotes: 3

jfriend00
jfriend00

Reputation: 707466

.map() is not promise-aware. So, when you pass it an async callback like you are, it does not pay any attention to that returned promise. So, it just runs the loop one after another, not waiting for any of the returning promises. Thus, all the asynchronous operations started in the .map() loop will be in-flight at the same time.

If that's what you want and you want to collect all the returned promises so you can later see when they are all done with Promise.all(), then this pattern works well:

Promise.all(someArray.map(callbackThatReturnsAPromiseHere))

And, this is a common design pattern for it. In fact, the Bluebird promise library has a special function that combined these two called Promise.map(). It also offered another nice feature that let you control how many concurrent async operations could run at once (because it's map() operation is promise-aware).

It sounds like you're trying to figure out if you should only use .map() and not use Promise.all(). If you do that, you will run your asynchronous operations in parallel, but you will not have any idea when they are all done or any ability to collect all the results. You would use Promise.all() on the array of returned promises to know when they are all done and/or to collect their resolved results.

FYI, .map() is JUST a plain loop. It doesn't have any special asynchronous features or any special run-in-parallel features. You can do the same thing with a for loop if you want. It does NOT pause for your async callback to wait for it to be done so a side effect of running it is that you launch a bunch of parallel asynchronous operations.

Upvotes: 8

Related Questions