0

Having a bit of trouble with Angular's asynchronization. Basically, I am looping through several cards. Cards of a certain two types require an API call, while cards not of those types don't require any API calls. After looping through all the cards, the array of finished cards is returned, but I am only getting back the ones that don't require any API calls.

This is a quick mockup I made of how it's working:

// If color of colorCard is blue, it needs 2 API calls
// If color of colorCard is red, it needs 1 API call
// If color of colorCard is yellow, it doesn't need an API call
// Pretend for this example that colorCards has one yellow card, one blue card, and two red cards

var buildCards = function() {
  var finishedArray = [];
  var promises = [];
  colorCards.forEach(function(e, i){
    if (e.color === 'blue') {
      promises.push(firstBlueApiCall); 
      var firstBlueIdx = 0;
      promises.push(secondBlueApiCall); 
      var secondBlueIdx = 1;
    } else if (e.color === 'red') {
      promises.push(redApiCall); 
      var redIdx = 0;
    }

    // If card is blue or red, complete API calls before pushing to finishedArray
    if (promises.length > 0) {
        $q.all(promises).then(function(response) {
          if (e.color === 'blue') {
            e.firstBlueData = response[firstBlueIdx];
            e.secondBlueData = response[secondBlueIdx];
          } else if (e.color === 'red') {
            e.redData = response[redIdx];
          }
          finishedArray.push(e);
          promises = [];
        });
    // If card is yellow, push to finishedArray without making any API calls
    } else {
      finishedArray.push(e);
      promises = [];
    }
  })
  return finishedArray;
}

In this example, the finishedArray that's returned only ever contains the one yellow card that didn't require an API call instead of all four cards. How can I get that 'return finishedArray' to wait until the red/blue cards have completed their API calls?

ClaytonAlanKinder
  • 313
  • 1
  • 3
  • 15
  • JavaScript is single-threaded. When a function completes it can only return data that is available or a *pending* promise that will be fulfilled in the *future*. – georgeawg Jan 17 '17 at 19:20

2 Answers2

1

The buildCards function can be simplified:

var buildCards = function(colorCards) {
  //var deferred = $q.defer();
  //var finishedArray = [];
  var promises = [];
  colorCards.forEach(function(card, i){
    promises.push(promiseFunction(card));
  });
  //$q.all(promises).then(function(finishedCards) {
  //  deferred.resolve(finishedCards)
  //})
  //return deferred.promise;
  return $q.all(promises);
}

There is no need to manufacture a promise with $q.defer as the $q.all method already returns a promise. In addition the manufactured promise doesn't handle rejection properly. If the any of $q.all promises has a rejection, the $q.defer promise will hang and never resolve.

This is known as a Deferred Anti-Pattern and should be avoided.

Similarly, the promiseFunction function can be modified to avoid a Deferred Anti-Pattern:

var promiseFunction = function(card){
  //var deferred = $q.defer();
  var localPromises = [];

  if (card.color === 'blue') {
    localPromises.push(blueApiCall1); var firstBlueIdx = promises.length - 1;
    localPromises.push(blueApiCall2); var secondBlueIdx = promises.length - 1;
  } else if (card.color === 'red') {
    localPromises.push(redApiCall); var redIdx = promises.length - 1;
  }

  var cardPromise;
  if (localPromises.length > 0) {
      //$q.all(promises).then(function(res) {
      cardPromise = $q.all(localPromises).then(function(res) {
        if (card.color === 'blue') {
          card.firstBlueData = res[firstBlueIdx];
          card.secondBlueData = res[secondBlueIdx];
        } else if (card.color === 'red') {
          card.redData = res[redIdx];
        }
        //deferred.resolve(card);
        //RETURN value to chain
        return card;
      });
  } else {
      //deferred.resolve(card);
      cardPromise = $q.when(card);
  }
  //return deferred.promise;
  return cardPromise;
}

The then method of a promise always returns a new promise that resolves to the value returned to the handler function. In addition if the original promise is rejected, the success handler will be skipped and the rejection will be carried down the chain to the new promise. This avoids erroneous hanging of a $q.defer.

Also notice that in the case where there are no promises to process with $q.all, the cardPromise can be created with $q.when.

Because calling the .then method of a promise returns a new derived promise, it is easily possible to create a chain of promises. It is possible to create chains of any length and since a promise can be resolved with another promise (which will defer its resolution further), it is possible to pause/defer resolution of the promises at any point in the chain. This makes it possible to implement powerful APIs

-- AngularJS $q Service API Reference - Chaining Promises

Always chain promises. Avoid using a Deferred Anti-Pattern.

Community
  • 1
  • 1
georgeawg
  • 48,608
  • 13
  • 72
  • 95
0

Here is how I ended up solving this:

var promiseFunction = function(card){
  var deferred = $q.defer();
  var localPromises = [];

  if (card.color === 'blue') {
    localPromises.push(blueApiCall1); var firstBlueIdx = promises.length - 1;
    localPromises.push(blueApiCall2); var secondBlueIdx = promises.length - 1;
  } else if (card.color === 'red') {
    localPromises.push(redApiCall); var redIdx = promises.length - 1;
  }

  if (localPromises.length > 0) {
      $q.all(promises).then(function(res) {
        if (card.color === 'blue') {
          card.firstBlueData = res[firstBlueIdx];
          card.secondBlueData = res[secondBlueIdx];
        } else if (card.color === 'red') {
          card.redData = res[redIdx];
        }
        deferred.resolve(card);
      });
  } else {
    deferred.resolve(card);
  }
  return deferred.promise;
}

var buildCards = function() {
  var deferred = $q.defer();
  var finishedArray = [];
  var promises = [];
  colorCards.forEach(function(card, i){
    promises.push(promiseFunction(card));
  });
  $q.all(promises).then(function(finishedCards) {
    deferred.resolve(finishedCards)
  })
  return deferred.promise;
}

I tried to make it so it's promises all the way down and it worked out well. Hopefully this helps people with similar issues in the future.

ClaytonAlanKinder
  • 313
  • 1
  • 3
  • 15