Home > Software engineering >  What is the problem with my implementation of Promise.all()?
What is the problem with my implementation of Promise.all()?

Time:10-16

I am trying to write my Promise.all() function. It should repeat the functionality of the native Promise.all() method.

If there are only promises in the array, the code works correctly, probably the problem appears if the array contains something other than a promise.

Here is an example of execution:

const p1 = Promise.resolve(3);

const p2 = 1337;

const p3 = new Promise((resolve, reject) => {
  setTimeout(resolve, 10000, 'foo');
});

promiseAll([p1, p3, p3]).then(values => console.log(values)); // [3, 1337, "foo"]

Here is my code:

const promiseAll = promises => new Promise((resolve, reject) => {
  if (promises.length === 0) { resolve([]); }
  const results = [];

  let resolved = 0;

  promises.forEach(promise => {
    if (!(promise instanceof Promise)) {
      Promise.resolve(promise).then(data => results.push(data));
    }
    promise.then(result => {
      results.push(result);
      resolved  = 1;
      if (resolved === promises.length) {
        resolve(results);
      }
    }, error => reject(error));
  });
});

What is the problem?

CodePudding user response:

You are creating a new promise with Promise.resolve(promise) but you don't change the original promise var. Do

if (!(promise instanceof Promise)) {
  promise = Promise.resolve(promise);
}

CodePudding user response:

Two main problems with your implementation:

  1. Inside the forEach() method, you have a check: promise instanceof Promise. If this check is false, you call Promise.resolve() BUT then you also class promise.then(...) - you probably meant to call then() method inside an else block

  2. Promise.all() maintains order - your implementation doesn't. This is because you just push the promise result in to the results array in the order in which the promises are resolved

You can change your implementation as shown below to fix the above mentioned problems:

const promiseAll = promises => {
    return new Promise((resolve, reject) => {
        if (promises.length === 0) {
          resolve([]);
          return;
        }

        const results = [];
        let resolved = 0;

        function collectResult(result, index) {
          results[index] = result;
          resolved  = 1;
          if (resolved === promises.length) {
            resolve(results);
          }
        }

        promises.forEach((value, index) => {
          if (
            typeof value === 'object' &&
            'then' in value &&
            typeof value.then === 'function'
          ) {
            value.then(res => collectResult(res, index)).catch(reject);
          } else {
            Promise.resolve(value).then(res => collectResult(res, index));
          }
        });
    });
};
  • Related