Skip to content

Instantly share code, notes, and snippets.

@dewwwald
Last active April 20, 2017 07:31
Show Gist options
  • Save dewwwald/bdf26de5ace5240af53a462e8bd2e709 to your computer and use it in GitHub Desktop.
Save dewwwald/bdf26de5ace5240af53a462e8bd2e709 to your computer and use it in GitHub Desktop.
Node JS Musing - Callbacks / Promise / Async.waterfall

Reason

I created this gist because of a recent question I asked in Z.A. Tech's slack channel.

I said: Hey node people. I have a question about Promise vs Async waterfall. What do you think is more readable? option 1:

    function (done) {
      DataModel.find({ owner: user._id }, function (err, response) {
        done(err, response);
      });
    },
    function(response, done) {
      DataModel2.find({ owner: user._id, complete: true })
        .select('key')
        .exec(function (err, responseTwo) {
          done(err, response, responseTwo);
        });
    }
]);

option 2:

let getSecondModel = response => new Promise((resolve, reject) {
  DataModel2.find({ owner: user._id, complete: true })
    .select('key')
    .exec(function (err, responseTwo) {
      if (err) {
        reject(err);
        return;
      }
      // .. do more stuff
      resolve(response, responseTwo);
    });
});

let getFirstModelPromise = new Promise(function (resolve, reject) {
  DataModel.find({ owner: user._id }, function (err, response) {
    if (error) {
       reject(error);
    }
    resolve(response);
  });
});

getFirstModelPromise
  .then(response => getSecondModel(response))
  .catch(err => console.log(error));

other_ben said: So

You should prefer promises wherever possible for a few reasons

The big ones being

  1. Interop with other api's and the language itself
  2. Composability
  3. Concurrency

I'll start with 3.

I said: My argument for using promise here is purely for having function blocks that are named.

In theory, this will promote readability and comprehension of the code.

other_ben: Maintainability and readability are big motivators for compositon (edited)

The big problem I find with traditional callbacks is that the interface is not enforced

most code uses the (err, args) => {} signature

also: this will fail

    setTimeout(() => {
        done();
    }, 1000)
}
try {
    IHazCallbacks(() => {

        throw new Error('Wut?');
    });
} catch (e) {
    console.log(e);
}

ryan.kotzen said: @dewwwald to your original question on Promise vs Async waterfall, we don't use promises on our express api, pure callbacks + async. Then on the client side (react-native) pretty much use promises exclusively. In my experience it comes down to not fighting the underlying framework. Express is built on the concepts of middleware and callbacks and trying to change that causes a headache. Many client side libraries have gone the promise route, so trying to force that to use callbacks causes a different headache

Personally, I prefer callbacks + async, but we always name our function blocks to reveal intent, so you end up with code like this:

module.exports = function initialise(callback) {
    async.waterfall([
        createApp,
        initialiseSwagger,
        addRoutes,
        addCommonSwaggerItems,
        generateSwaggerJson,
        mongo.connect,
        rateLimit.initialise,
        authentication.initialise,
        permissions.initialise
    ], callback);
};

to make the functions more reusable we tend to follow the pattern of function nameOfFunction(context, callback){ rather than having function nameOfFunction(arg1,arg2,arg3, callback){

that allows you to reorder and reuse the functions at any point in the app, as long as the context object has the prerequisites. The downside is that you cant really tell what the "true" method signature is without looking at the code

but in practice it's rarely a problem as the functions are short by convention

function addRoutes(app, callback) {
    app.use(routes);
    app.use(error.notFound);
    app.use(error.errorHandler);
    app.use(error.boomErrorHandler);
    return callback(null, app);
}

ryan said: @ryan.kotzen has the right idea about named functions and also defining your functions within an inline async.waterfall call does not make them re-usable its quite nice to see how you can compose them to add/remove features

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment