javascript - Look for Promise bluebird code review for node.js -

when , need use new promise(function<function resolve, function reject> resolver) -> promise

my sample code:

userinfo.js

var promise = require('bluebird'); var winston = require('winston'); var _ = require('lodash'); var request = promise.promisify(require("request"));  exports.getweather = function (data) {     var cityname = data.userprofile.city;     return request("http://0.0.0.0:3003/api/weather/bycity?city=" + cityname).spread(function (res, body) {         var result = json.parse(body).data;         return _.merge(data, result);     }); };  exports.getuserprofile = function (userid) {     return new promise(function (resolve, reject) {         request("http://0.0.0.0:3003/api/userprofile/getuserprofile?id=" + userid).spread(function (res, body) {             var result = json.parse(body).data;             resolve(result);         });     })  };  exports.getevents = function (data) {     var cityname = data.userprofile.city;     return request("http://0.0.0.0:3003/api/events/bycity?city=" + cityname).spread(function (res, body) {         var result = json.parse(body).data;         return _.merge(data, result);     }); };  exports.getfashion = function (data) {     var gender = data.userprofile.gender;     return request("http://0.0.0.0:3003/api/fashion/bygender?gender=" + gender).spread(function (res, body) {         var result = json.parse(body).data;         return _.merge(data, result);     }); };  exports.displaydetail = function (data) {     console.log(data); }; 

above code try call in 2 way in promise

getuserprofile.js

var userinfo = require('./userinfo');     module.exports = function(){        return userinfo.getuserprofile(3)                     .then(userinfo.getfashion)                     .then(userinfo.getevents)                     .then(userinfo.getweather)                     .then(userinfo.displaydetail)                     .catch(function (e) {                         console.log('error:');                         console.error(e.stack)                     })                     .finally(function () {                         console.log('done');                     });      } 

2nd way:

getuserinformation.js

var userinfo = require('./userinfo');      module.exports = function () {         return new promise(function (resolve, reject) {               resolve(3);         })             .then(userinfo.getuserprofile)                 .then(userinfo.getfashion)                 .then(userinfo.getevents)                 .then(userinfo.getweather)                 .then(userinfo.displaydetail)                 .catch(function (e) {                     console.log('error:');                     console.error(e.stack)                 })                 .finally(function () {                     console.log('done');                 });     }; 

getdetails.js

var userinfo = require('./getuserinformation');     userinfo()     .then(function(){             console.log('getdetails done')         })         .catch(function (e) {             console.log('error:');             console.error(e.stack)         })         .finally(function () {             console.log('done');         }); 

please let me know difference , there issues using these way?

exports.getuserprofile = function (userid) {     return new promise(function (resolve, reject) {         request("http://0.0.0.0:3003/api/userprofile/getuserprofile?id=" + userid).spread(function (res, body) {             var result = json.parse(body).data;             resolve(result);         });     }) }; 

please don't this. return callback, , return promise created then, have done in other 3 methods.

return userinfo.getuserprofile(3) .then(…) 

vs.

return new promise(function (resolve, reject) {     resolve(3); }) .then(userinfo.getuserprofile) .then(…) 

well, first 1 more readable , concise. they're pretty equivalent except case getuserprofile throw synchronously, shouldn't anyway. in first case getuserprofile invoked method on userinfo, while in second case it's callback function, this in calls different.

the second pattern can tremendously simplified though using promise.resolve instead of new promise constructor:

return promise.resolve(3) .then(userinfo.getuserprofile) .then(…) 

this totally fine, , aligns better rest of chain. speaking of which, …

.then(userinfo.getfashion) .then(userinfo.getevents) .then(userinfo.getweather) 

where each of functions returns promise resolves additional data merged argument

is not best way solve this. yes, ensures these 3 functions called after each other, , is acceptable pattern case. however, in case you're mixing request calls api argument-extraction , result-merging in same function; separation of concerns shouldn't. rather make functions pure

exports.… = function (arg) {     return request("http://0.0.0.0:3003/api/…?…=" + arg).spread(function (res, body) {         return json.parse(body).data;     }); }; 

and can combine them separately - , not in sequence, in parallel:

userinfo.getuserprofile(3) .then(function(data) {     var p = data.userprofile;     return promise.prop({          userprofile: 0,          fashion: userinfo.getfashion(p.gender), // `\          events: userinfo.getevents(p.city),     //   }=> execute requests in parallel          weather: userinfo.getweather(p.city)    // ./     }); }) .then(userinfo.displaydetail) .catch(function (e) {      console.error('error:', e.stack) });