exports.create = function(req, res) {
var company_id = req.company_id;
var client = new Client(req.body);
Company.findOne({_id: company_id}, function(err, company) {
if(err) {
response = {
status: 'error',
error: err
}
return res.json(response);
} else if(!company) {
response = {
status: 'error',
error: 'Invalid company_id'
}
return res.json(response);
} else {
client.save(function(err) {
if(err) {
response = {
status: 'error',
error: err
}
} else {
response = {
status: 'ok',
client: client
}
}
return res.json(response);
});
}
});
}
That's my code (using Express
, if it matters). I'm trying to learn more about promises, specifically with Q
. I feel that this is the perfect kind of logic that can be implemented with promises to avoid this hairy conditional nest. But I'm not sure how to start?
But I'm not sure how to start?
Start by removing the callbacks and using the Promise methods instead. Then put the error handling in a dedicated error callback, instead of using that condition. Also, you can put the code for building that response
object in the very end (removing the duplication), and only pass/throw the err
down to it.
exports.create = function(req, res) {
var client = new Client(req.body);
Q.ninvoke(Company, "findOne", {_id: req.company_id}).then(function(company) {
if(!company)
throw 'Invalid company_id';
else
return company;
}).then(function(company) {
return Q.ninvoke(client, "save");
}).then(function(saveResult) {
return {
status: 'ok',
client: client
};
}, function(err) {
return {
status: 'error',
error: err
};
}).done(function(response) {
res.json(response);
});
};
This is not really the use case for Promises. Promises are a method for eliminating deeply nested callbacks in node.js, not complex logic dealing with a result, like you have in your code. Think of the case where the outcome of your .findOne
query would determine the next query you need to make - this quickly leads to a nested callback situation, which can be made less nested and more natural to read, evaluate, re-factor, and build upon if you use Promises.
You do have one nested callback for the outcome of your .save
call. So, you might successfully re-factor this using Promises, like so
new Promise(function() {
Company.findOne()
...
.then(function(company) {
company.update()
...
}, function(err) {
...
})
The starting point is to wrap your first query so that it returns a promise instead of dealing with callbacks within callbacks.