Preserving Execution order in nested for loop and callbacks in NodeJS MongoDB

Question

Scenario: There are users and users has many posts. For a particular group of users, I need to fetch 10 recent posts per user and send them in response.
Here is what I have come up with:

users is array having user info.

var allPosts = [];

for(var i=0; i<users.length; i++){
    (function(i){                                               //Level-1 
        db.collection('posts', function(err, postCollection){
            (function(i){                                       //Level-2
                postCollection.find({"user_id": users[i]['user_id']}).sort({"created": -1}).limit(10).toArray(function(err, post) {
                    (function(i){                               //Level-3
                        for(var j =0; j< post.length; j++){
                            (function(j){
                                post[j]['created'] = ObjectId(post[j]['_id'].toString()).getTimestamp();
                                allPosts.push(post[j]);
                                if(j === post.length-1){
                                    res.send(allPosts);
                                }
                            })(j);
                        }    
                    })(i);
                });
            })(i);
        });    
    })(i);                        
}

Now, the execution order is preserved up to Level-2, but when it enters Level-3, all things just go wrong: I have two users in array and one user has 3 posts and another has 10 posts, sometimes response is only 3 posts and sometimes all the 13 posts. I think its because of MongoDB. I am even taking care of execution order by using immediately invoked function expression(IIFE), but it just does not seem to work here. Any help is appreciated. Thanks

Catalin MUNTEANU · Accepted Answer · 2014-05-19T09:47:23.757

First of all you should beautify your code. Using anonymous functions inside loops inside callbacks of other loops is not really easy to maintain or read.

The problem with your code was that in the last loop ( the j loop ) you get to j == users.length - 1 before queries for others users were finished so the response is sent with the number of post queries finished until that moment.

One other big mistake you made was that you request the post collection inside a loop. That's wrong! You should cache both the database and the collection.

Try this code:

var allPosts = [];
var post_collection = null;

var get_user = function(i, callback) {
    post_collection
        .find({"user_id": users[i]['user_id']})
        .sort({"created": -1})
        .limit(10)
        .toArray(function(err, post) {

            // Do something when you get error
            // Always call the callback function if there is one
            if(err) {
                callback();
                return;
            }

            for(var j=0; j<post.length; ++j) {
                post[j]['created'] = ObjectId(post[j]['_id'].toString()).getTimestamp();
                allPosts.push(post[j]);
            }

            callback();
        });
};

var fetch_users = function() {
    var count = users.length;

    for(var i=0; i<users.length; ++i) {
        get_user(i, function() {
            // Each time a query for one user is done we decrement the counter
            count--;

            // When the counter is 0 we know that all queries have been done
            if(count === 0) {
                res.send(allPosts);
            }
        });
    };    
};

// Get the collection, check for errors and then cache it!
db.collection('posts', function(err, postCollection) {

    // Always check for database errors
    if(err) {
        console.log(err);
        return;
    }

    post_collection = postCollection;
    fetch_users();
});

You should know that this code is not tested. I might have missed a semicolon or some braces but you should figure that out easily.

Thanks @Catalin, it works perfectly fine. A little modification: need to check before calling callback in get_user if(j === post.length-1){callback();} . Also, thanks for your advice and for helping me out in such short time — Avdhesh Parashar, May 19 '14 at 09:16
Yes you are right but you don't need the if statement. Actually that callback should be after the loop. I've edited the answer. You should also change it in your code. — Catalin MUNTEANU, May 19 '14 at 09:46

Preserving Execution order in nested for loop and callbacks in NodeJS MongoDB

1 Answers1