CODEWITHSUNDEEP
javascriptcallbackpromisees6-promiseasynccallback
robe007
robe007

Reputation: 3927

Improved way to deal with callbacks inside promises

I have the following code that uses callbacks inside promises:

const clue = 'someValue';

const myFunction = (someParam, callback) => {
    someAsyncOperation(someParam) // this function returns an array
    .then((array) => {
        if (array.includes(clue)){
            callback(null, array); // Callback with 'Success'
        }
        else{
            callback(`The array does not includes: ${clue}`); // Callback with Error
        }
    })
    .catch((err) => {
        // handle error
        callback(`Some error inside the promise chain: ${err}`) // Callback with Error
    })
}

and call it like this:

myFunction (someParam, (error, response) => {
    if(error) {
        console.log(error);
    }
    else {
        // do something with the 'response'
    }    
})

Reading some documentation, I found that there is some improved way to do this: 

const myFunction = (someParam, callback) => {
    someAsyncOperation(someParam) // this function returns an array
    .then((array) => {
        if (array.includes(clue)){
            callback(array);
        }
        else{
            callback(`The array does not includes: ${clue}`);
        }
    }, (e) => {
        callback(`Some error happened inside the promise chain: ${e}`);
    })
    .catch((err) => {
        // handle error
        callback(`Some error happened with callbacks: ${err}`)
    })
}

My question:

In the sense of performance or best practices, it's okay to call the 'callback' function inside the promise as the two ways show or I'm doing something wrong, I mean some promise anti-pattern way ?

Upvotes: 1

Views: 1399

Answers (2)

jfriend00
jfriend00

Reputation: 707158

Do not use callbacks from inside of promises, that is an anti-pattern. Once you already have promises, just use them. Don't "unpromisify" to turn them into callbacks - that's moving backwards in code structure. Instead, just return the promise and you can then use .then() handlers to set what you want the resolved value to be or throw an error to set what you want the rejected reason to be:

const clue = 'someValue';

const myFunction = (someParam) => {
    return someAsyncOperation(someParam).then(array => {
        if (!array.includes(clue)){
            // reject promise
            throw new Error(`The array does not include: ${clue}`);
        }
        return array;
    });
}

Then, the caller would just do this:

myFunction(someData).then(array => {
    // success
    console.log(array);
}).catch(err => {
    // handle error here which could be either your custom error
    // or an error from someAsyncOperation()
    console.log(err);
});

This gives you the advantage that the caller can use all the power of promises to synchronize this async operation with any others, to easily propagate errors all to one error handler, to use await with it, etc...

Upvotes: 1

charlietfl
charlietfl

Reputation: 171679

This seems really backwards and takes away from the benefits of promises managing errors and passing them down the chain

Return the asynchronous promise from the function and don't interrupt it with callbacks. Then add a catch at the end of the chain

const myFunction = (someParam) => {
  // return the promise
  return someAsyncOperation(someParam) // this function returns an array
    .then((array) => {
      return array.includes(clue) ? array : [];
    });
}

myFunction(someParam).then(res=>{
  if(res.length){
     // do something with array
  }else{
     // no results
  }
}).catch(err=>console.log('Something went wrong in chain above this'))

Upvotes: 1

Related Questions