Find the bug · Medium

Fix the Async Total (forEach)

Medium Find the bug Most asked

totalPrice(ids) looks up the price of each product with the async fetchPrice(id) and returns the sum. (Here fetchPrice is simulated: it waits 1 ms and returns id * 10.)

It always returns 0. Find the bug and fix it. Keep fetchPrice as it is.

Example 1

Input: ids = [1, 2, 3]
Output: 60
Explanation: The buggy version returns 0.

The buggy code

async function fetchPrice(id) {
  await new Promise((resolve) => setTimeout(resolve, 1)); // simulated network call
  return id * 10;
}

async function totalPrice(ids) {
  let total = 0;
  ids.forEach(async (id) => {
    total += await fetchPrice(id);
  });
  return total;
}

async function fetchPrice(id: number): Promise<number> {
  await new Promise((resolve) => setTimeout(resolve, 1)); // simulated network call
  return id * 10;
}

async function totalPrice(ids: number[]): Promise<number> {
  let total = 0;
  ids.forEach(async (id) => {
    total += await fetchPrice(id);
  });
  return total;
}

Hints

Hint 1

What does forEach do with the promise your async callback returns?

Hint 2

When does return total run relative to the awaits?

Find and fix the bug

Loading the editor… (JavaScript is required to run code in your browser.)

Solution and explanation

Show the bug and the fix

Time O(n) · Space O(n)

The bug: forEach ignores the promises returned by an async callback. It starts all the callbacks, does not wait for any of them, and return total runs immediately, while total is still 0. The additions happen later, after the function has returned.

The fix: use a construct that waits. for (const id of ids) total += await fetchPrice(id) runs the lookups one after another; const prices = await Promise.all(ids.map(fetchPrice)) runs them concurrently, which is usually what you want for independent I/O. Mention a concurrency limit when ids can be large.

Fixed version

async function fetchPrice(id) {
  await new Promise((resolve) => setTimeout(resolve, 1)); // simulated network call
  return id * 10;
}

async function totalPrice(ids) {
  const prices = await Promise.all(ids.map(fetchPrice)); // concurrent, and actually awaited
  return prices.reduce((sum, p) => sum + p, 0);
}

async function fetchPrice(id: number): Promise<number> {
  await new Promise((resolve) => setTimeout(resolve, 1)); // simulated network call
  return id * 10;
}

async function totalPrice(ids: number[]): Promise<number> {
  const prices = await Promise.all(ids.map(fetchPrice)); // concurrent, and actually awaited
  return prices.reduce((sum, p) => sum + p, 0);
}

Common mistakes

ids.map(async ...) without Promise.all: you get an array of promises, not numbers.
reduce with an async callback: the accumulator becomes a promise.

Fix the Binary Search · Easy
Fix the Two Sum Hash Map · Easy
Fix the Longest Substring Without Repeats · Medium
Fix Merge Intervals · Medium
Fix the Mutable Default Argument · Easy
Fix the Substring Check · Easy