I'm still somewhat new to asynchronous programming and I've been wondering if there's a better way to accomplish what I'm trying to do.
exports.content = function(req, res){
OPID = req.params.id;
titles.findOne({postID: OPID}, function (err, post) { //function #1
if (err) throw(err);
readComment(OPID, function(comment){ //function #2
branchFilter.getBranches(function(branches){ //function #3
res.render('content', {title: post.title, content: post.body, OPID: post.postID, comments: comment, branches: branches});
})
});
});
};
In this example I have three nested function which use callbacks to retrieve data from other modules. I need to have all this data in the res.render statement. I imagine if I continue with this approach I'm going to need even more nested functions. Is there a better way of doing this?
For you example, have a function that just gets the data
exports.content = function(req, res){
getData(req, function(err, data) {
// trivial error handling
if (err) {
console.dir('error getting your data', err);
return res.redirect('/');
}
res.render('content', data)
}
// cb is a callback function
function getData(req, cb) {
OPID = req.params.id;
titles.findOne({postID: OPID}, function (err, post) { //function #1
if (err) { return cb(err); }
readComment(OPID, function(err, comment){ //function #2 (with error handling)
if (err) { return cb(err); }
branchFilter.getBranches(function(err, branches){ //function #3
if (err) { return cb(err); }
var output = {
title: post.title,
content: post.body,
OPID: post.postID,
comments: comment, branches: branches
}
return cb(null, output);
});
});
});
}
Take a look at http://callbackhell.com/ gives a very good overview on how write clean code using callbacks. The following is copied from that site
Here is some (messy) browser javascript that uses browser-request to make an AJAX request to a server:
var form = document.querySelector('form')
form.onsubmit = function(submitEvent) {
var name = document.querySelector('input').value
request({
uri: "http://example.com/upload",
body: name,
method: "POST"
}, function(err, response, body) {
var statusMessage = document.querySelector('.status')
if (err) return statusMessage.value = err
statusMessage.value = body
})
}
This code has two anonymous functions. Let's give em names!
var form = document.querySelector('form')
form.onsubmit = function formSubmit(submitEvent) {
var name = document.querySelector('input').value
request({
uri: "http://example.com/upload",
body: name,
method: "POST"
}, function postResponse(err, response, body) {
var statusMessage = document.querySelector('.status')
if (err) return statusMessage.value = err
statusMessage.value = body
})
}
As you can see naming functions is super easy and does some nice things to your code:
Building on the last example, let's go a bit further and get rid of the triple level nesting that is going on in the code:
function formSubmit(submitEvent) {
var name = document.querySelector('input').value
request({
uri: "http://example.com/upload",
body: name,
method: "POST"
}, postResponse)
}
function postResponse(err, response, body) {
var statusMessage = document.querySelector('.status')
if (err) return statusMessage.value = err
statusMessage.value = body
}
document.querySelector('form').onsubmit = formSubmit
Code like this is less scary to look at and is easier to edit, refactor and hack on later.
This is the most important part: Anyone is capable of creating modules (AKA libraries). To quote Isaac Schlueter (of the node.js project): "Write small modules that each do one thing, and assemble them into other modules that do a bigger thing. You can't get into callback hell if you don't go there."
Let's take out the boilerplate code from above and turn it into a module by splitting it up into a couple of files. Since I write JavaScript in both the browser and on the server, I'll show a method that works in both but is still nice and simple.
Here is a new file called formuploader.js that contains our two functions from before:
function formSubmit(submitEvent) {
var name = document.querySelector('input').value
request({
uri: "http://example.com/upload",
body: name,
method: "POST"
}, postResponse)
}
function postResponse(err, response, body) {
var statusMessage = document.querySelector('.status')
if (err) return statusMessage.value = err
statusMessage.value = body
}
exports.submit = formSubmit
As JohnnyHK suggested, a good async library will help you out with this. I would also suggest Caolan's async. You can get around pretty much any asynchronous problem with a combination of auto and forEach, although if runtime is an issue async may not be the best choice. I would rewrite your code as follows:
exports.content = function(req, res){
OPID = req.params.id;
async.auto({
post: function(next) {
titles.findOne({postID: OPID}, next);
},
comment: function(next) {
readComment(OPID, function(comment){ next(null, comment); });
},
branches: function(next) {
branchFilter.getBranches(function(branches){ next(null, branches); });
}
}, function(err, results) {
if(err) throw(err);
res.render('content', {title: results.post.title, content: results.post.body, OPID: results.post.postID, comments: results.comment, branches: results.branches});
});
};
What auto does is takes a dictionary of functions (with optional dependencies on other functions in the dictionary - see the async readme for more) and passes the results as a dictionary to the second argument, a "final" function.
If any of the functions passes a non-null err to its callback (the first argument), it stops calling other functions and immediately passes err and a null results to the "final" function.
Note how I had to pass anonymous functions in the branches and comment functions. This is because auto, like the other functions in this async library, expects the first argument to its callback to be the error value and the second to be the results.