NodeJS Promises VS async library

Question

I am currently working on a NodeJS server, and am switching from callbacks to promises.

Here's an example of my code:

 var record = req.body.record;
var schoolId = req.body.schoolId;
var promise = BusController.addBus(record,schoolId);
promise.then(function(){
    return res.json({status:"success", message : "bus successfully created"});
}).catch(function(error){
        if(error){
            console.log(error);
            res.json({status:"error",message:"error while creating bus"});
            return;
        }

The bus controller has the following in it:

addBus : function(record,schoold){
    return new Promise(function(resolve,reject){
        var bus_number = record.bus_number;

        var newBus = new Bus({
            language: language ? language : "",
            bus_number: bus_number  ? bus_number : "",
            bus_driver: bus_driver  ? bus_driver : {},
            bus_details: bus_details ,
            trips: trips ? trips : [],
            school_id: schoold,
            location: location ? location : {}
        });
        newBus.save().then(function(bus){
            var busCreation = {};
            busCreation.name=  "باص رقم"+ ' ' +bus_number;
            busCreation.type= "3";
            busCreation.refid= bus._id;
            busCreation.email= record.email;
            busCreation.phone_number=record.phone_number;
            busCreation.school_id = schoold;
            console.log(busCreation);
            var promise = UserController.addUser(busCreation);
            promise.then(function(){
                resolve();
            })
        }).catch(function(error){
            if (error) {
                reject(error);
            }
        });
    });
}

This my current logic. Is this a good practice? Or should I switch to async library?

CR Drost · Accepted Answer · 2017-03-20 14:35:02Z

That bus controller is too complicated; in general you should not be using new Promise(fn) unless you are wrapping a callback, and what you're doing is better done by returning a promise from the .then(). (When you return a value from myPromise.then() it gets passed into Promise.resolve(p), which if p is not a promise creates a new one which immediately resolves with the value you created; or if p is a promise, just returns that promise.)

In addition I wouldn't mix camelCase and underscore_names; it leads to bigger confusions across the codebase. In addition the form a ? a : b is equivalent to the shorter a || b. I also would in general add a debugging string to any console.log so that while you're in development you can better drill down to exactly what you're looking for.

With those changes the bus controller simplifies to just:

addBus: function(record, schoold) {
    return new Bus({
        language: language || "",
        bus_number: record.bus_number || "",
        bus_driver: bus_driver  || {},
        bus_details: bus_details,
        trips: trips || [],
        school_id: schoold,
        location: location || {}
    }).save().then(function (bus) {
        var bus_creation_user = {
            name: "باص رقم" + ' ' + record.bus_number,
            type: "3",
            refid: bus._id,
            email: record.email,
            phone_number: record.phone_number,
            school_id: schoold
        };
        console.log("bus_creation_user", bus_creation_user);
        return UserController.addUser(bus_creation_user);
    });
});

The code for using it seems mostly fine but has some code smells. For example what is the if doing in your .catch(err => { if (err) { ... } })? Are you legitimately throwing non-Error objects like null and undefined and false? (In JS you can do this but it is widely considered to be a bad idea.) Instead you can just write,

BusController.addBus(record, schoolId).then(function(){
    res.json({status:"success", message: "bus successfully created"});
}).catch(function(error){
    console.log(error);
    res.json({status:"error", message: "error while creating bus"});
});

thank you very much, what you say makes sense and helped a lot — motchezz
– motchezz, Commented Mar 21, 2017 at 8:41
"async" and "await" are syntactic sugar for handling Promises. There is no difference, use async function and await if you have them. You might instead be asking whether Promises are better than callbacks, Node's general function types being ([Value], Either Error Value -> IO ()) -> IO() and promise-returning functions instead being [Value] -> IO (Either Error Value). They are, with the biggest reason being that it is easy to lose track of an error thrown in callback code, and a minor reason being that this Promise.resolve semantics de-nestifies weakly-interdependent code. — CR Drost
– CR Drost, Commented Mar 21, 2017 at 12:59
Code with strong interdependencies between promises can also be de-nested, but you need to hand around all of your state values as arguments because you can't capture them in the inner closure and you might not want the quasi-global null-by-default Smaaktics of putting them in an outer closure. — CR Drost
– CR Drost, Commented Mar 21, 2017 at 13:05

Stack Exchange Network

NodeJS Promises VS async library

1 Answer 1

You must log in to answer this question.

Hot Network Questions

NodeJS Promises VS async library

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions