Optimize Node.js function

Question

I have the follow function:

Download.prototype.fundamentals = function(exchanges, cb){
 if (!(exchange = exchanges.pop())) return
 var that = this;
 this.login(function(err){
 if (err) return cb(true, exchange.code)
 var path = util.format(that.eoddata.fundamentals['path'], that.token, exchange.code)
 that.get(that.eoddata.fundamentals['host'], path, function(err, data){ 
 if (err) return cb(true, exchange.code) 
 that.xml(data, function(err, data){ 
 if (err || data['@'].Message != 'Success') return cb(true, exchange.code) 
 var stocks = data['FUNDAMENTALS']['FUNDAMENTAL']
 that.insert(stocks, exchange, function(){ 
 cb(false, exchange.code) 
 }) 
 that.fundamentals(exchanges, cb) 
 }) 
 })
 }) 
}

I think it easy to understand, I pop() an element to a list and then do other operations, and then recall the function to pop() another element.

I have a doubt regarding the writing of the function, now it works, but it does not seem very optimized. Example: I do not like var that = this.

How could I rewrite it better?

• sarnold: This might be a better fit on Code Review; I've flagged for moderator attention to migrate, we'll see if they agree you'll do better there. (You can flag too, if you want it higher up the moderator queue.)

Answer 1

Firstly, you are missing a var exchange; in your function.

Heavily nested callbacks are no fun for anyone. I would consider using the async library to try to simplify the callbacks.

https://github.com/caolan/async#waterfall

Something like this. The callbacks passed to waterfall are called in series, and any arguments passed to each callback function after the err argument, are passed as the first parameters of the next function.

var that = this is just something you kind of have to get used to. In some cases you can avoid it by using various methods to bind the value of 'this' to the proper value.

Download.prototype.fundamentals = function(exchanges, cb) {
 var that = this,
 exchange = exchanges.pop();
 if (!exchange) return;
 async.waterfall([
 function(callback) {
 that.login(callback);
 },
 function(callback) {
 var path = util.format(that.eoddata.fundamentals['path'], that.token, exchange.code)
 that.get(that.eoddata.fundamentals['host'], path, callback);
 },
 function(data, callback) {
 that.xml(data, function(err, xmldata) {
 err = err || xmldata['@'].Message != 'Success';
 callback(err, xmldata);
 });
 },
 function(xmldata, callback) {
 var stocks = xmldata['FUNDAMENTALS']['FUNDAMENTAL']
 that.insert(stocks, exchange, callback);
 that.fundamentals(exchanges, cb);
 }
 ], function(err) {
 cb(err, exchange.code);
 });
}