4

Looking at node.js, and avoiding nesting callbacks. I found Async.js, can someone show me how can I rewrite this code using the async.waterfall method?

// db is a mongodb instance
db.open(function(err, client){
    client.createCollection("docs", function(err, col) {
         client.collection("docs", function(err, col) {
             for (var i = 0; i < 100; i++) {
                 col.insert({c:i}, function() {});
             }
             console.log('Insert done OK.');
         });
    });
});

What problems does this naive solution with inner functions have, over the async method?

db.open(openAndInsert);

function openAndInsert(err, client) {
    var myCollection = "docs";

    client.createCollection(myCollection, openCollection);

    function openCollection(err, col) {
        client.collection(myCollection, insertIntoCollection);
    }

    function insertIntoCollection(err, col) {
        for (var idx = 0; idx < 100; idx += 1) {
            col.insert({c:idx}, function () {});
        }
        console.log('Insert done OK.');
    }
}

The three levels of nesting in the original code seem like a code smell, but I'm not sure either my home rolled or the async solution are universally better. Would anyone prefer just leaving the nested functions as is? If so, how deep would the nesting have to be before going using some type of flow control?

  • I'm going to shamelessly direct you to qcnode--it helps you solve these problems. – JoshRagem Jun 21 '13 at 14:39
1

You asked several questions, I will respond tothe last two.

Callback nesting using anonymous functions is the opposite of code reuse and DRY. It makes code harder to read by splitting logic in one function by putting a callback right in middle.

Usually when you try to make more general functions to use for callbacks, you run into issues just trying to figure out what to do in different situations (like errors). You end up making a bunch of nested function calls and spaghetti code trying to account for everything.

Callbacks are there. That is the nature of nodejs.

Something like asyncjs or qcnode allows you to write code in a linear fashion that is more understandable and debuggable.

I feel that you should have little stub callbacks that handle errors and decide if you can go forward with what you trying to do, otherwise they should abort. I've been using qcnode (and it's ancestors) for years and I think this latest version is simple to use and very helpful.

Full disclosure: I am the publisher and main developer for qcnode.

  • Thanks Josh. I had a look at qcnode. Can you give me some reasons to use it over async or step? Are they all trying to solve the same problem? – Sean Jun 21 '13 at 16:23
  • 1
    There definitely is overlap with the three projects, but different goals, i think. Step is basic flow control, async provides a bunch of helper functions (like map), and qcnode adapts to your workflow. I've updated the README and started an example page in the wiki. Feel free to check it out; I think I'll put examples of using the others in qcnode. Just for fun! – JoshRagem Jun 22 '13 at 17:21
0

This does what I need it to using async.js:

db.open(function (err, client) {
    async.waterfall([
        function (callback) {
            client.createCollection("docs", callback);
        },
        function (collection, callback) {
            for (var i = 0; i < 100; i++) {
                collection.insert({c:i}, function() {});
            }
            callback(null, 'ok');
        }
    ],
    function (err, result) {
        console.log('result: ', result);
    });
});

By putting the async code inside the anonymous function passed to db.open the 'client' variable was visible to the first async function. Got that idea from this SO answer

  • I still like to have separate functions below the logic (hoisting makes this possible). – Tracker1 Jul 24 '13 at 23:05
0

I would suggest both the separate functions along with async... Also, bear in mind that Function.prototype.bind is incredibly powerful.

//open the connection first, because the client may need to be used for cleanup.
db.open(function(err, client){

    //run the tasks in series, passing the result of each to the next task
    async.waterfall([
        //bind collection method to run against "docs"
        //there is no need to create a wrapper function yourself for this
        client.collection.bind(client, "docs")

        //passes the resulting collection to addToDocs
        ,addToDocs 

    ], completed); //run completed function at the end

    //will add 100 docs into the database
    function addToDocs(docs /*collection*/, cb) {
        //list of items to be run (async/parallel)
        var todo = [];
        for (var i=0; i<100; i++) {
            //bind the insert method against a new object for i, so it can be run later
            todo.push(docs.insert.bind(col, {c:i}));
        }

        //do all the inserts, then run the original callback
        async.parallel(todo, cb);

        //alternatively, you could have used async.series(todo, cb) to run one at a time.
    }

    function completed(err, results) {
        //close client resources, return connection to pool etc

        //handle any errors
        if (err) {
            console.log(err);
        }

        //any other work to be done
    }
});

I happen to like this pattern, where you have your logic at the top, and reference methods below...

NOTE: only function declarations will be hoisted to work properly, var foo = function(){...} will not.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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