JS wait for callback to finish inside a loop

Question

I have in my node js app a for loop; inside this loop on each iteration a mysql query can be executed (not always, depends); the query is async and I get the result in the success callback; but I need that each iteration of the for loop waits for the callback to finish (if needed):

function calculate() {
    var resArray = [];

    //[...]

    for (i = 0; i < tables.length; i++) {
        if (id !== undefined) {
            var queryString = ...  //build query
            sequelize.query(queryString).success(function(result) {   //executes query
                id = result[0].id;

                //do stuff with id returned by the query
                //...
                //resArray.push( //push in resArray the result of stuff done above )
            });  
       }
       else {
           //do other kind of stuff and push result in resArray
       }
   }
   return resArray;
}

If id != undefined, the query is executed but the for loop doesn't wait for the success callback and the function returns an incomplete resArray.

How can I do (or how can I reorganize the code) to make this work? (I'm working with node js and I don't have the possibility to use jQuery).

Solution 1

Thanks to Amadan suggestions i got rid of this using a mixed method (with promises and a counter);

I'm really interested in your opinion about this and if you have suggestions to improve it;

First I've added to the project the q library (npm install q) for using promises. Thanks to q i've been able to wrap sequelize.query (callback-based) within a function that returns a promise; a counter for promises keeps in memory the number of promises still pending and the wrapping function executeQuery decremets it when each promise is resolved;

(the for loop in cycle() function executes 4 loops; it executes 2 times the query and 2 times a simple log; the goal is that the 'finished' log is sent to the screen only after for loop ends and all promises are resolved

var id = req.params.id;
var counter = 0;
var endCycle = false;

var queryString = 'SELECT * FROM offers WHERE id = ' + id;

function query () {
    var deferred = Q.defer();
    sequelize.query(queryString).success(function(result) {
        console.log('obtained result');
        deferred.resolve(result);
    });
    return deferred.promise;
}

function executeQuery() {
    query().then(function() {
        console.log('after');
        counter --;
        console.log(counter);
        finished();
    });
}

function finished() {
    if ((counter === 0) && (endCycle)) {
        console.log('finished');
        endCycle = false;
    }
}

function cycle() {
    var result;

    for (i = 0; i <= 3; i ++) {
        if (i > 1) {
            counter ++;
            executeQuery();
        }
        else {
            console.log('else');
        }
    }

    endCycle = true;
    finished();
}

cycle();

---

------------------ UPDATE ------------------------------------------------------------

Following hugomg suggestion I've updated the code with this that is more clean: I push every promise in an array and then use Q.all to wait for all of them to be resolved

var id = req.params.id;
var promisesArray = [];
var endCycle = false;

var queryString = 'SELECT * FROM offers WHERE id = ' + id;
function query () {
    var deferred = Q.defer();
    sequelize.query(queryString).success(function(result) {
        console.log('obtained result');
        deferred.resolve(result);
        finished();
    });
    promisesArray.push(deferred.promise);
}

function finished() {
    if (endCycle) {
        endCycle = false;
        Q.all(promisesArray).then(function() {
            console.log('finished');
        });            
    }
}

function cycle() {
    var result;

    for (i = 0; i <= 3; i ++) {
        if (i > 1) {
            query();
        }
        else {
            console.log('else');
        }
    }
    endCycle = true;
    finished();
}

cycle();

Solution 2

You need to change your way of thinking. If calculate calls an asynchronous method, the only way you're getting its results is not by return, but by calling another callback. In node.js, you mustn't think of a function as a black box that takes an input and returns an output; you need to think of it as a step in a process that has a certain future. The question you need to ask yourself is - "I got these results; what then?"

Just like with sequelize.query, where you tell it "do this query, and then do this with the results", you need to be able to call calculate and tell it "go do those queries, and then do this with their results". You're not getting a return from sequelize.query - and you shouldn't be trying to return something from calculate, either.

Promises would work great, but without relying on them, you can just count. There are four changes in code below:

// HERE: take a callback parameter
function calculate(callback) {
    var outstandingRequests = 0;
    var resArray = [];

    //[...]

    for (i = 0; i < tables.length; i++) {
        if (id !== undefined) {
            var queryString = ...  //build query

            // HERE: count how many responses you are expecting
            outstandingRequests++;

            sequelize.query(queryString).success(function(result) {   //executes query
                id = result[0].id;

                //do stuff with id returned by the query
                //...
                //resArray.push( //push in resArray the result of stuff done above )

                // HERE: check if all requests are done
                // if so, the array is as full as it will ever be
                // and we can pass the results on
            }).done(function() {
                if (!(--outstandingRequests)) {
                    callback(resArray);
                }
            });  
       }
       else {
           //do other kind of stuff and push result in resArray
       }
   }
   // HERE: return is useless in asynchronous code
}

and then you can replace the hypothetic and non-functional

var resArray = calculate();
console.log("Here's your results:", resArray);

with

calculate(function(resArray) {
    console.log("Here's your results:", resArray);
});

EDIT: put the countdown into done handler to account for possible errors.

Comments

[Rohan Desai]

You could use Q.all instead of keeping track of that counter yourself. Also, what you have done so far isn't really taking advantage of promises. Your code is very similar to an equivalent callback version.

[Cereal Killer]

Can you provide an example of using Q.all in this case? I'm trying to understand how to do that but without success...

[Rohan Desai]

Create an array with the promises returned by your query function. Pass that awway of promises to Q.all and it will return a promise that resolves when all queries are resolved.

[Cereal Killer]

Ok I've updated the answer using Q.all; you said also that I'm not really taking advantage of promises; can you explain more? Even reading a lot of doc about promises I'm still a bit confused... Wich one is the proper way of using them?