0

How can I optimise the following (i.e. avoid nested promises)? It works but seems like I'm going to continue nesting promises

The code first authenticates and returns a service, then it feeds that service into a function that calls the api asynchronously to gets items, then I'll do something with the items after, probably calling another function containing an async call:

new Promise(function(resolve, reject) {
    auth.authenticate(resolve);
}).then(function(service) {
    console.log('service', service);
    new Promise(function(resolve, reject) {
        lineItems.getLineItems(service, resolve, reject);
    }).then(function(items) {
        console.log('returned line items');
        console.log(items);
    }).catch(function(err){
        console.log('error!', err);
    });
});
darkace
  • 838
  • 1
  • 15
  • 27
  • 1
    This code is error-prone, because you don't [return](http://stackoverflow.com/questions/37081508/resolving-an-array-of-promises-from-within-a-parent-promise/37084467#37084467) the second promise. – jib Oct 21 '16 at 14:53

2 Answers2

1

Just return the new promise from then:

new Promise(function(resolve, reject) {
    auth.authenticate(resolve);
}).then(function(service) {
    console.log('service', service);
    return new Promise(function(resolve, reject) {
        lineItems.getLineItems(service, resolve, reject);
    });
}).then(function(items) {
    console.log('returned line items');
    console.log(items);
}).catch(function(err){
    console.log('error!', err);
});

Also, if you can adjust lineItems.getLineItems to return a promise, it looks more concise:

new Promise(function(resolve, reject) {
    auth.authenticate(resolve);
}).then(function(service) {
    console.log('service', service);
    return lineItems.getLineItems(service);
}).then(function(items) {
    console.log('returned line items');
    console.log(items);
}).catch(function(err){
    console.log('error!', err);
});
Sergey Lapin
  • 2,633
  • 2
  • 18
  • 20
  • So is it good practice to be creating promises in this fashion and passing through resolve/reject to functions? Or is it better for the function that I'm calling to be a promise in the first place? – darkace Oct 21 '16 at 10:03
  • 2
    If `lineItems.getLineItems` is designed by you, IMO this is redundant to pass `resolve` and `reject` there. You'd better make it to take only `service` as an argument and return a promise. So you can just `return lineItems.getLineItems(service);` instead of `return new Promise(function(resolve, ...` – Sergey Lapin Oct 21 '16 at 10:09
  • If you can add that to your answer I'll accept it :) – darkace Oct 24 '16 at 18:59
1

I see a couple of issues primarily on the way functions are defined. This actually comes from non-standard signatures of async functions defined in code.

If auth.authenticate and lineItems.getLineItems are written by you, update these functions to return a proper Promise. Then the composition would be:

auth.authenticate()
    .then((service) => lineItems.getLineItems(service))
    .then((items)   => console.info('Items:',items))
    .catch((err)    => console.error(err));

If auth.authenticate and/or lineItems.getLineItems are external, and follow the standard nodejs callbak/errback style, you can wrap these function to return a promise:

const authenticate = Promise.promisify(auth.authenticate, {context:auth});
const getLineItems = Promise.promisify(lineItems.getLineItems,{context:lineItems});

authenticate()
    .then(getLineItems)
    .then((items)   => console.info('Items:',items))
    .catch((err)    => console.error(err));
S.D.
  • 29,290
  • 3
  • 79
  • 130