4

I am new to ES6 Promises and have been doing research. I have some code executing in NodeJS that does some async work, but I have a few pre-conditions that must be checked first. I'm seeking the idiomatic best-practice for dealing with this (if such a thing exists) along with some reasoning. I hope to gain an understanding, as I already have working code.

Consider the following fictional snippet:

function doStuff(data, cb) {
     if (!data) {
         return cb(new Error("Don't be an idiot"));
     }

     externalLibrary.doSomethingCallbackAsync(data, cb);
}

Were I to translate this into promise-land, I see two options.


Option 1, I can include the pre-condition in the promise.

function doStuff(data){
    return new Promise((resolve, reject) => {
        if (!data) {
            return reject(new Error("Don't be an idiot"));
        }

        externalLibrary.doSomethingCallbackAsync(data, function(err, newData) {
            if (err) {
                return reject(err);
            }
            return resolve(newData);
        });
    });
}

Option 2, I can run the pre-condition prior to the promise. I'm not certain I understand the intent of Promise.reject(), but it seems to fit the bill here by allowing me to return an immediately rejected promise.

function doStuff(data){
    if (!data) {
        return Promise.reject(new Error("Don't be an idiot"));
    }

    return new Promise((resolve, reject) => {
        externalLibrary.doSomethingCallbackAsync(data, function(err, newData) {
            if (err) {
                return reject(err);
            }
            return resolve(newData);
        });
    });
}

I prefer Option 2 for readability reasons, but I don't fully understand Promise.reject() and I am concerned that Option 2 is misusing it. Just to reiterate, I am looking for a best-practice solution.

Kevin Burdett
  • 2,892
  • 1
  • 12
  • 19
  • I'm sure that this is just an example, but don't forget that it's better to use error objects instead of a plain string for these types of error messages if this was a real function. – Qantas 94 Heavy Mar 07 '16 at 23:30
  • Thanks for the advice @Qantas94Heavy. It is merely an example, I typed it freehand and got lazy :) Update to avoid any confusion. – Kevin Burdett Mar 07 '16 at 23:33

1 Answers1

3

I'm not certain I understand the intent of Promise.reject(), but it seems to fit the bill here

You've understood it correctly. It's explicitly made for use cases such as this. Go for it, it is the best practice.

The advantage of Option 2 is that it works much cleaner with APIs that already do return promises (think return externalLibrary.doSomethingAsync(data)), and has less chance to fall for the Promise constructor antipattern.

Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • 1
    See also [Is there a way to return early in deferred promise?](http://stackoverflow.com/a/31934181/1048572) and [if-else flow in promise](http://stackoverflow.com/q/26599798/1048572) for more best-practice code examples. – Bergi Mar 07 '16 at 23:34
  • Thanks for the succinct answer and the extra links @Bergi! – Kevin Burdett Mar 08 '16 at 01:11