Implementing Promise.all

Question

One of the exercises in Chapter 17 of the book Eloquent Javascript is to implement the Promise.all() method, I came up with this implementation (which doesn't work):

function all(promises) {
  return new Promise(function(success, fail) {
    var successArr = new Array(promises.length);
    if (promises.length == 0)
      success(successArr);
    var pending = promises.length;
    for (var i = 0; i < promises.length; i++) {
      promises[i].then(function(result) {
        successArr[i] = result;
        pending -= 1;
        if (pending == 0)
          success(successArr);
      }, function(error) {
        fail(error);
      });
    }
  });
}


// Testing
function soon(val) {
  return new Promise(function(success) {
    setTimeout(function() { success(val); },
               Math.random() * 500);
  });
}
all([soon(1), soon(2), soon(3)]).then(function(array) {
  console.log("This should be [1, 2, 3]:", array);
});
// => [undefined, undefined, undefined, 3]

Interestingly, the author's solution is similar apart from the use of forEach to iterate over the promises array instead of the for loop in my case:

function all(promises) {
  return new Promise(function(success, fail) {
    var successArr = new Array(promises.length);
    if (promises.length == 0)
      success(successArr);
    var pending = promises.length;
    promises.forEach(function(promise, i) {
      promise.then(function(result) {
        successArr[i] = result;
        pending -= 1;
        if (pending == 0)
          success(successArr);
      }, function(error) {
        fail(error);
      });
    });
  });
}

// Testing
function soon(val) {
  return new Promise(function(success) {
    setTimeout(function() { success(val); },
               Math.random() * 500);
  });
}
all([soon(1), soon(2), soon(3)]).then(function(array) {
  console.log("This should be [1, 2, 3]:", array);
});
// => [1, 2, 3]

Why does the use forEach makes all the difference here?, I'm guessing it's something related to the scope created by the anonymous function passed to forEach, but I can't quite figure how that works.

@Bergi this question is not a duplicate, at least not for the original question you marked as being duplciated — Alexander Mills, Jun 01 '16 at 21:21
@AlexMills: How so? The problem is the value of the `i` variable in the asynchronous callback, which is exactly what the canonical dupe target describes. Even the self-answer by the OP makes the same point. What would you have answered differently? — Bergi, Jun 01 '16 at 23:12

Edmeral · Accepted Answer · 2016-04-10T14:24:23.583

8

I just figured it out, it's the i variable, because we're on a for loop, it changes before the promise get a chance to resolve but on the forEach version the i variable is properly scoped so each iteration has it's own i variable

edited Apr 10 '16 at 14:24

answered Apr 10 '16 at 14:21

Edmeral

185
1
7

Yes exactly! Well spotted. This is called a closure - i don't think your book explains it, but you will come to know them very well. See [this question](http://stackoverflow.com/questions/111102/how-do-javascript-closures-work) for an explanation. – Rhumborl Apr 10 '16 at 14:23
Thanks for the link ;) – Edmeral Apr 10 '16 at 14:55

Implementing Promise.all

1 Answers1

Linked