CODEWITHSUNDEEP
javascriptangularjsfor-looppromiseangular-promise
Leon Gaban
Leon Gaban

Reputation: 39018

How to return single promise after for loop (which produces a promise on every iteration) is complete?

I have a problem with my promise return code, I have a function getTagQuotes which contains a for loop which can make multiple calls an API to return data into an array.

How my code for this begins below:

// If there are tags, then wait for promise here:
if (tags.length > 0) {

    // Setting promise var to getTagQuotes:
    var promise = getTagQuotes(tags).then(function() {
        console.log('promise =',promise);

        // This array should contain 1-3 tags:
        console.log('tweetArrayObjsContainer =',tweetArrayObjsContainer);

        // Loop through to push array objects into chartObj:
        for (var i=0; i<tweetArrayObjsContainer.length; i++) {
            chartObj.chartData.push(tweetArrayObjsContainer[i]);
        }

        // Finally draw the chart:
        chartDirective = ScopeFactory.getScope('chart');
        chartDirective.nvd3.drawChart(chartObj.chartData);
    });
}

My getTagQuotes function with the promise return:

function getTagQuotes(tags) {
    var deferred = $q.defer(); // setting the defer
    var url      = 'app/api/social/twitter/volume/';

    // My for loop, which only returns ONCE, even if there are 3 tags
    for (var i=0; i<tags.length; i++) {
        var loopStep = i;
        rawTagData   = [];

        // The return statement
        return GetTweetVolFactory.returnTweetVol(url+tags[i].term_id)
            .success(function(data, status, headers, config) {
                rawTagData.push(data);

                // One the last loop, call formatTagData
                // which fills the tweetArrayObjsContainer Array
                if (loopStep === (rawTagData.length - 1)) {
                    formatTagData(rawTagData);
                    deferred.resolve();
                    return deferred.promise;
                }
            });
    }

    function formatTagData(rawData) {

        for (var i=0; i<rawData.length; i++) {
            var data_array = [];
            var loopNum = i;

            for (var j=0; j<rawData[loopNum].frequency_counts.length; j++) {
                var data_obj = {};
                data_obj.x = rawData[loopNum].frequency_counts[j].start_epoch;
                data_obj.y = rawData[loopNum].frequency_counts[j].tweets;
                data_array.push(data_obj);
            }

            var tweetArrayObj = {
                "key" : "Quantity"+(loopNum+1), "type" : "area", "yAxis" : 1, "values" : data_array
            };

            tweetArrayObjsContainer.push(tweetArrayObj);
        }
    }
}

Take notice of this line 

return GetTweetVolFactory.returnTweetVol(url+tags[i].term_id)

it's inside my for loop:

for (var i=0; i<tags.length; i++)

Everything works great if I only have to loop through once. However as soon as there is another tag (up to 3) it still only returns the first loop/data. It does not wait till the for loop is done. Then return the promise. So my tweetArrayObjsContainer always only has the first tag.

Upvotes: 6

Views: 7380

Answers (6)

Roamer-1888
Roamer-1888

Reputation: 19288

With reference to this question and the earlier question :

  • the code in general will be a lot cleaner with array.map() in lieu of for loops, in several places.
  • getTagQuotes() will be made cleaner by building an array of promises, submitting it to $q.all() and returning an aggregate promise.
  • formatTagData(), and its relationship with its caller, will be made cleaner by returning the transformed rawData.

With a few assumptions, the code should simplify to something like this :

getTagQuotes(tags).then(function(tweetArrayObjsContainer) {
    chartObj.chartData = chartObj.chartData.concat(tweetArrayObjsContainer); // concat() ...
    // chartObj.chartData = tweetArrayObjsContainer;                         // ... or simply assign??
    chartDirective = ScopeFactory.getScope('chart');
    chartDirective.nvd3.drawChart(chartObj.chartData);
});

function getTagQuotes(tags) {
    var url = 'app/api/social/twitter/volume/';
    var promises = tags.map(function(tag) {
        var deferred = $q.defer();
        GetTweetVolFactory.returnTweetVol(url + tag.term_id)
        .success(function(data, status, headers, config) {
            deferred.resolve(data);
        })
        .error(function(data, status) {
            console.log(tag.term_id + ': error in returning tweet data');
            deferred.resolve(null); // resolve() here effectively catches the error
        });
        return deferred.promise;
    });
    return $q.all(promises).then(formatTagData); //this is much much cleaner than building an explicit data array and resolving an outer deferred.

    function formatTagData(rawData) {
        return rawData.filter(function(data) {
            return data || false; // filter out any nulls
        }).map(function(item, i) {
            return {
                'key': 'Quantity' + (i+1),
                'type': 'area',
                'yAxis': 1,
                'color': tagColorArray[i],
                'values': item.frequency_counts.reverse().map(function(c) {
                    return {
                        x: addZeroes(c.start_epoch),
                        y: c.tweets,
                    };
                })
            };
        });
    }
}

Upvotes: 0

guramidev
guramidev

Reputation: 2238

You should return an array of promises here, which means that you should change getTagsQuotes like this:

function getTagQuotes(tags) {

    var url      = 'app/api/social/twitter/volume/',
        promises = [];

    for (var i=0; i<tags.length; i++) {

       promises.push( GetTweetVolFactory.returnTweetVol( url+tags[i].term_id ) );

    }

    return promises;
}

And then loop through this promises like this:

if (tags.length > 0) {

    var promises = getTagQuotes(tags);

    promises.map( function( promise ) {

         promise.then( function( data ) { 

            //Manipulate data here

         });

    });
}

Edit: In case you want all promises to be finished as outlined in the comment you should do this:

if (tags.length > 0) {

    Promise.all( getTagQuotes(tags) ).then( function( data ) { 

        //Manipulate data here

    });
}

Edit: Full data manipulation:

Promise.all( getTagQuotes(tags) ).then( function( allData ) {

allData.map( function( data, dataIndex ){

    var rawData = data.data,
        dataLength = rawData.frequency_counts.length,
        j = 0,
        tweetArrayObj = {
            // "key"    : "Quantity"+(i+1),
            // "color"  : tagColorArray[i],
            "key"    : "Quantity",
            "type"   : "area",
            "yAxis"  : 1,
            "values" : []
        };

    for ( j; j < dataLength; j++ ) {

        rawData.frequency_counts[j].start_epoch = addZeroes( rawData.frequency_counts[j].start_epoch );

        tweetArrayObj.values.push( { x:rawData.frequency_counts[j].start_epoch, y:rawData.frequency_counts[j].tweets  } );

    }

    tweetArrayObjsContainer.push( tweetArrayObj );

});

for ( var i= 0,length = tweetArrayObjsContainer.length; i < length; i++ ) {

    chartObj.chartData.push( tweetArrayObjsContainer[ i ] );

}

chartDirective = ScopeFactory.getScope('chart');
chartDirective.nvd3.drawChart(chartObj.chartData);

});

Upvotes: 1

Fernando Pinheiro
Fernando Pinheiro

Reputation: 1816

Put every promise in an array then do:

$q.all(arrayOfPromises).then(function(){
  // this runs when every promise is resolved.
});

Upvotes: 1

Tiago Fael Matos
Tiago Fael Matos

Reputation: 2077

return deferred.promise; should be the return value of your function, not the GetTweetVolFactory.returnTweetVol(), because that's what you intend to promisify.

Your problem is that you are calling several GetTweetVolFactory.returnTweetVol(), and then you need to merge all those async calls to resolve your promise. In order to do that, you should promisify just one GetTweetVolFactory.returnTweetVol() call:

function promisifiedTweetVol(rawTagData, urlStuff) {
    var deferred = $q.defer(); // setting the defer

    GetTweetVolFactory.returnTweetVol(urlStuff)
        .success(function(data, status, headers, config) {
            rawTagData.push(data);

            // One the last loop, call formatTagData
            // which fills the tweetArrayObjsContainer Array
            if (loopStep === (rawTagData.length - 1)) {
                formatTagData(rawTagData);
                deferred.resolve();
            }
        });

    return deferred.promise;
}

And then call each promise in a loop and return the promise that resolves when all promises are completed:

function getTagQuotes(tags) {
    var url      = 'app/api/social/twitter/volume/';
    var promises = [];

    // My for loop, which only returns ONCE, even if there are 3 tags
    for (var i=0; i<tags.length; i++) {
        var loopStep = if;
        rawTagData   = [];

        promises.push( promisifiedTweetVol(rawTagData, url+tags[i].term_id) );
    }

    // ...

    return $.when(promises);
}

There are a few more issues with your code, but you should be able to get this working with my tip.

Upvotes: 2

Ben Wilde
Ben Wilde

Reputation: 5672

Three issues:

  1. you didn't return the deferred promise from the getTagQuotes method.
  2. you were looking at i to see if you were through the loop, and the for loop is already completed (i == (tags.length - 1)) before the first success is even called.
  3. you called return in the first iteration of the loop so that you didn't even get to the 2nd item.

Here's corrected code (didn't test it yet)

function getTagQuotes(tags) {
    var deferred = $q.defer(); // setting the defer
    var url      = 'app/api/social/twitter/volume/';
    var tagsComplete = 0;

    for (var i=0; i<tags.length; i++) {
        rawTagData   = [];
        GetTweetVolFactory.returnTweetVol(url+tags[i].term_id)
            .success(function(data, status, headers, config) {
                rawTagData.push(data);
                tagsComplete++;

                if (tagsComplete === tags.length) {
                    formatTagData(rawTagData);
                    deferred.resolve();
                }
            });
    }

    return deferred.promise;
}

Upvotes: 2

Chris Jaynes
Chris Jaynes

Reputation: 2907

Using deferreds is widely considered to be an anti-pattern. If your promise library supports a promise contstructor, that's an easier way to create your own promises.

Rather than trying to resolve all of the promises in one, I usually use a promise implementation that has an all function. Then I create one function that returns a promise for a thing, and then another function that returns a promise for all the things.

Using a map() function is also usually a lot cleaner than using a for loop.

Here's a generic recipe. Assuming your promise implementation has some flavor of an all function:

var fetchOne = function(oneData){
 //Use a library that returns a promise
 return ajax.get("http://someurl.com/" + oneData);
};

var fetchAll = function(allData){
  //map the data onto the promise-returning function to get an
  //array of promises. You could also use `_.map` if you're a 
  //lodash or underscore user.
  var allPromises = myData.map(fetchOne);
  return Promise.all(allPromises);
};

var allData = ["a", "b", "c"];
var promiseForAll = fetchAll(allData);

//Handle the results for all of the promises.
promiseForAll.then(function(results){
  console.log("All done.", results);
});

Upvotes: 0

Related Questions