flow control solutions in Node.js - use async.waterfall?

Question

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?

Answer 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.

Answer 2

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

Answer 3

pipe of async functions can be done in a really clean way:

npm install js-awe

This is how the solution of your problem looks like:

import {plan, repeat} from 'js-awe'

const createCollection = plan().build([
  () => db.open(),
  (client) => client.createCollection("docs"),
  (collection) => Promise.all(
    repeat(100).times(i => collection.insert({c:i}))
  )
])

createCollection()
  .then(result => console.log('result: ', result))
  .catch(error => console.log('Error: ', error))

Answer 4

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.