current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Join the Stack Overflow Community
Stack Overflow is a community of 6.6 million programmers, just like you, helping each other.
Join them; it only takes a minute:
Sign up
up vote 4 down vote favorite
1

I would like to ask this question, because I'm not sure if I got the Node.js logic right

I have a set of id's that I need to query using redis' get method. And after checking a certain value(let's say that im checking whether the object I get with given "key" has a null name), I add them to a list. Here is my example code;

var finalList = [];
var list = [];
redisClient.smembers("student_list", function(err,result){
            list = result; //id's of students
            console.log(result);

            var possibleStudents = [];


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


                redisClient.get(list[i], function(err, result){
                    if(err)
                        console.log("Error: "+err);
                    else{
                        tempObject = JSON.parse(result);
                        if(tempObject.name != null){
                            finalList.push(tempObject);
                        }
                    }
                });     
            }

    });
   console.log("Goes here after checking every single object");

But as expected due to the async nature of Node, without checking every single id in the list it executes the "Goes here...". My need is to apply the rest of procedures after every id is checked(mapping in redis db and checking the name). But I do not know how to do it. Maybe if I can attach callback to the for loop and assure the rest of my functions start to run after loop is finished(i know it's impossible but just to give an idea)?

share|improve this question
up vote 4 down vote accepted

I would go the route you suggest in your question and attach a custom callback to your fetching function:

function getStudentsData(callback) {
    var setList = [];
    var dataList = [];

    redisClient.smembers("student_setList", function(err,result) {
        setList = result; //id's of students

        for(var i = 0; i < setList.length; i++) {
            redisClient.get(setList[i], function(err, result) {
                if(err) {
                    console.log("Error: "+err);
                } else {
                    tempObject = JSON.parse(result);
                    if(tempObject.name != null) {
                        dataList.push(tempObject);
                    }
                }
            });     
        }

        if(dataList.length == setList.length) {
            if(typeof callback == "function") {
                callback(dataList);
            }
            console.log("getStudentsData: done");
        } else {
            console.log("getStudentsData: length mistmach");
        }

    });
}

getStudentsData(function(dataList) {
    console.log("Goes here after checking every single object");
    console.log(dataList.length);
    //More code here
});

That's probably the most efficient method; alternatively you could rely on an old school while loop until the data is ready:

var finalList = [];
var list = [0];

redisClient.smembers("student_list", function(err,result) {
    list = result; //id's of students
    var possibleStudents = [];

    for(var i = 0; i < list.length; i++) {
        redisClient.get(list[i], function(err, result) {
            if(err) {
                console.log("Error: "+err);
            } else {
                tempObject = JSON.parse(result);
                if(tempObject.name != null) {
                    finalList.push(tempObject);
                }
            }
        });     
    }
});


process.nextTick(function() {
    if(finalList.length == list.length) {
        //Done
        console.log("Goes here after checking every single object");
        console.log(dataList.length);
        //More code here
    } else {
        //Not done, keep looping
        process.nextTick(arguments.callee);
    }
});

We use process.nextTick instead of an actual while to make sure other requests are not blocked in the meantime; due to the single threaded nature of Javascript this is the preferred way. I'm throwing this in for the sake of completeness, but the former method is more efficient and fits better with node.js, so go for it unless a major rewrite is involved.

It's worth nothing that both cases rely on async callbacks, which means any code outside it can still potentially run before others are done. E.g., using our first snippet:

function getStudentsData(callback) {
    //[...]
}

getStudentsData(function(dataList) {
    //[...]
});

console.log("hello world");

That last console.log is almost guaranteed to run before our callback passed to getStudentsData gets fired. Workaround? Design for it, it's just how node.js works. In our case above it's easy, we just would call console.log only in our callback passed to getStudentsData and not outside it. Other scenarios require solutions that depart a bit more from traditional procedural coding, but once you get your head around it you'll find being event-driven and non-blocking is actually a pretty powerful feature.

share|improve this answer
    
First example doesn't seem to work for me :(. See this, the redis call is asynchronous. The console log of my results would be shown first since it's the exact same logic as in your example, hmmm. – NiCk Newman Jan 25 '16 at 1:48
up vote 3 down vote

Try finish module. I created this module to deal with this problem. It is easier to use than Async, and also yields better performance. Here's an example:

var finish = require("finish");
finish(function(async) { 
  // Any asynchronous calls within this function will be captured
  // Just wrap each asynchronous call with function 'async'
  ['file1', 'file2', 'file3'].forEach(function(file) {
    async(function(done) { 
      // Your async function should use 'done' as callback, or call 'done' in its callback
      fs.readFile(file, done); 
    });
  });
}, function(err, results) {
  // fired after all asynchronous calls finish or as soon as an error occurs
  console.log(results[0]);console.log(results[1]);console.log(results[2]);
});
share|improve this answer
2  
Should post sample of the code as well as the link. That way if the link becomes dead the post is not useless. – Matthew Jan 29 '13 at 18:04
    
Thanks. Code sample added. – Chaoran Mar 27 '13 at 3:02
1  
WARN: finish currently chokes if the array you forEach is empty, and exits with "TypeError: cannot read property 'kickoff' of undefined". Took me a while to track that down; hope it saves someone else some pain! See issue on Github. – Ollie Ford Aug 13 '15 at 10:38
    
@OllieFord Issue is fixed. Thanks for reporting this. – Chaoran Aug 17 '15 at 22:34
    
Awesome, just saw :) thanks! – Ollie Ford Aug 17 '15 at 22:54
up vote 1 down vote

Try async module for node.js. That module has asynchronous forEach.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.