Trying to define a promise.all

Question

I am trying to understand how promises work by creating the Promise.all method from an exercise of this book: https://eloquentjavascript.net/11_async.html#i_Ug+Dv9Mmsw

I tried looping through the whole array that's given as argument to the method itself, using .then for successful promises where the body of the handler pushes the result to a binding i defined previously outside of the loop, for rejected promises, i used .catch in a way that it takes the rejected value as a "reason" and rejects the main promise giving it an error

function Promise_all(promises) {
  return new Promise((resolve, reject) => {
    if(promises.length == 0) resolve(promises);
    let fullArray = [];

    for(let i=0; i<promises.length ; i++){
         promises[i]
        .then(x => fullArray.push(x))
        .catch(reason => reject(new Error(reason)));
    }
    resolve(fullArray);
  });
}

What i expected the function to do the following:

-Pick a promise from the "Promises" array.

-Solve the promise (if successful) by using the .then method on it with a handler function that just pushes the result to "fullArray".

-Solve the promise (if rejected) by using the .catch method on it with a handler function that simply calls the reject handler of the main promise that will be returned by "Promise_all".

-When the loop finishes, simply resolve the promise the "fullArray" of successful promises.

The code simply doesn't work the way i thought it would, using the test code of the book doesn't return the expected results:

Promise_all([]).then(array => {
  console.log("This should be []:", array);
});
function soon(val) {
  return new Promise(resolve => {
    setTimeout(() => resolve(val), Math.random() * 500);
  });
}
Promise_all([soon(1), soon(2), soon(3)]).then(array => {
  console.log("This should be [1, 2, 3]:", array);
});
Promise_all([soon(1), Promise.reject("X"), soon(3)])
  .then(array => {
    console.log("We should not get here");
  })
  .catch(error => {
    if (error != "X") {
      console.log("Unexpected failure:", error);
    }
  });

"*Solve the promise*" - sounds like you're not clear on what actually happens with a promise. You cannot "solve" or "force" it from outside. A promise resolves on its own, and all you can do is listen to it. The `then` and `catch` callbacks will always be called *asynchronously*, when the promise has actually settled. Your `fullArray` isn't actually filled when you call `resolve()` immediately after the loop that only attached handlers but didn't wait for anything. — Bergi, Aug 17 '19 at 09:51
So, maybe if i call "resolve()" inside the ".then" of the last promise of the "Promises" array, the loop won't be finished until "fullArray" is filled, right? — JDCH, Aug 17 '19 at 10:00
Yes, that's the right idea. But not the `then` callback of the *last promise* in the array, but rather the `then` callback on any promise that *happens last*. The promises in the array may fulfill at different times, and you want to wait until all of them are fulfilled. You need a counter for that. — Bergi, Aug 17 '19 at 10:06

score 0 · Accepted Answer · answered Aug 17 '19 at 10:47

The issue with the code, as @Bergi said, was that the ".then" and ".catch" callbacks are called asynchronously and as a consequence of this, "fullArray" wasn't even filled when "resolve()" was called even when it was outside of the loop. To solve this, i simply added "resolve()" inside of the promise that finishes last, to do this, i simply added a "counter" binding that has the value of the length of the "promises" array, and is reduced by 1 every time ".then" was called on a promise and when this "counter" equals 0, it calls "resolve()".

But that only solves the problem of filling the "fullArray" with the promises, not the issue of those promises being ordered correctly by the order they were called, to solve this i simply enumerated them in the array with the "i" binding of the loop, the end result was this:

function Promise_all(promises) {
  return new Promise((resolve, reject) => {
    if(promises.length == 0) resolve(promises);
    let fullArray = [],
        counter = promises.length;

    for(let i=0; i< promises.length ; i++){
         promises[i]
        .then(x => {
           fullArray[i] = x;
           counter--;
           if(counter == 0) resolve(fullArray)})
        .catch(reason => reject(new Error(reason)));
    }
  });
}

Also need to change `promises[i].then()` to `Promise.resolve(promises[i]).then()` in case a non-thenable is passed in the array (which is allowed). — jfriend00, Aug 17 '19 at 16:24
I also would recommend [using `.then(…, …)` instead of `.then(…).catch(…)`](https://stackoverflow.com/q/24662289/1048572), and resolving with the `fullArray` instead of the passed `promises` array in case of emptyness. — Bergi, Aug 18 '19 at 21:02

Trying to define a promise.all

1 Answers1