2

Here, while using promise should i need to return the resolve and reject methods

the code executes smoothly but if there are multiple conditions statements then will reject and resolve auto-ends or we have to use return statements

 const getJobs = (filters, fieldASTs) => new Promise((resolve, reject) => {
        const AST = fieldASTs.fieldNodes[0].selectionSet.selections[0]
            .selectionSet.selections[0].selectionSet.selections;
        const FIELDS = _.map(AST, n => n.name.value);

        if (_.includes(FIELDS, 'employer')) {
            Job.find(filters, (err, d) => {
                if (err) return reject(err);

             // should i need to return or just use reject

                if (err === null && d === null) return reject(null);
             // return resolve(d) or only resolve()
                return resolve(d);
            });
        } else {
            Job.find(filters, (err, d) => {
              // here also
                if (err) return reject(err);
                // here too
                return resolve(d);
            });
        }
    });
terik_poe
  • 523
  • 4
  • 17
  • Why do you reject when `err === null`? Isn't that the success case? – jfriend00 Feb 06 '17 at 06:51
  • no, if a user enter's an 12 digit valid objectId but if that id isn't present in mine db, then it will neither produce an error which will be `null` nor a response which will be also `null` so.... – terik_poe Feb 06 '17 at 06:54
  • I get why you are checking `d === null`, but `err === null` SHOULD be the success case, not an error case. Remember, when `err` is NOT falsey is the error condition. When `err` is falsey (such as `null`) is the usualy success condition. If you're somehow using `err === null` as an error, that's just a really bad practice. – jfriend00 Feb 06 '17 at 07:01
  • Why don't you simply use `if`/`else` statements? – Bergi Feb 06 '17 at 07:02
  • @jfriend00 so i should simply check `d === null` then – terik_poe Feb 06 '17 at 07:05
  • Yes, that seems right. Just `if (d === null) ...` after your `if (err) ...` check. – jfriend00 Feb 06 '17 at 07:24
  • And, keep in mind, you can always just use `if/else` statements to and not do returns (just like in all Javascript programming). – jfriend00 Feb 06 '17 at 07:24

1 Answers1

2

Whether or not to use a return statement is entirely up the desired flow of control in your function. It actually has nothing to do with promises. If you don't want or need any more code in your function to execute and you aren't already completely isolated in a conditional, then use a return to exit the function. Same issue whether you are or aren't using promises.

Remember, all resolve() or reject() do is change the state of the promise (assuming it is in the pending state) and then, any .then() or .catch() handlers are scheduled for future execution after the current running Javascript returns control back to the system. They are just functions calls like other functions calls.

You do not have to use return statements after calling resolve() or reject().

So, whether or not a return statement is appropriate depends entirely upon your code. If you don't want to execute more code in that block, then return. If you don't want to waste time potentially calling resolve() or reject() elsewhere in the block (which will not actually do anything to the promise), then use a return. If your code is already within a conditional block and nothing else will execute that you don't want to execute, then there is no need for a return.

For example, in this part of your code:

if (_.includes(FIELDS, 'employer')) {
    Job.find(filters, (err, d) => {
        if (err) return reject(err);

        if (err === null && d === null) return reject(null);
        return resolve(d);
    });
 }

it is appropriate to use the return because there is no need to execute any more code in the current function. If you omitted the return there, your code would still function fine (in this particular case) because the extra code you would run would not actually do anything since calling reject() or resolve() after you've already rejected or resolved the promise does not change anything. But, I consider it a wasteful and a bit confusing practice to let code run that doesn't need to run. So, I would always use either a return or a conditional in this case.

Personally, I'd probably write this code like this:

if (_.includes(FIELDS, 'employer')) {
    Job.find(filters, (err, d) => {
        if (err) return reject(err);
        if (d === null) return reject(new Error("unexpected null result from Job.find()"));
        return resolve(d);
    });
}

Note: I removed the check for if (err === null) because that should be the success case.

Or, I would have promisified Job.find() more generically at a lower level so my logic flow would all be promises.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • so using `return reject() || resolve()` is better then only `reject() || resolve()` – terik_poe Feb 06 '17 at 06:52
  • 1
    @terik_poe - As my answer says, whether or not you should use `return` depends upon whether you want more code in that function to execute or not. Sometimes you do, sometimes you don't. It depends entirely upon the code in the function, not on the line of code that is resolving or rejecting. It's a regular control-flow thing, nothing specifically to do with promises. – jfriend00 Feb 06 '17 at 06:56
  • hey bro i am still confused can u come over chat please, to help me understand – terik_poe Feb 06 '17 at 07:22
  • 1
    @terik_poe - Yes, `resolve()` will execute, but it will not do anything because promises can't change their state once it is set. Please read my answer. `resolve()` and `reject()` are just normal function calls. They don't change the flow of execution one bit inside this function. They impact other flow of execution elsewhere in `.then()` or `.catch()` handlers, but not inside this function. Just normal function calls. – jfriend00 Feb 06 '17 at 07:22
  • 1
    so whether i use `return` in `reject` or `resolve` it will stop the execution after it, if i don't use `return` it create a promise state but will also execute the code after it. – terik_poe Feb 06 '17 at 07:23
  • 1
    @terik_poe - I don't understand your confusion. `return` is the same here as it is anywhere in Javascript. And, `resolve()` and `reject()` are just ordinary function calls. You appear to be thinking that `resolve()` or `reject()` would stop the function from executing. They don't. Code after them continues to execute unless you use conditionals or return from the function. It's normal Javascript flow of control, nothing different with promises at all. – jfriend00 Feb 06 '17 at 07:27
  • so in mine case there is no other statements after `if` condition, so using or not using return would be same, if there would statements after the `reject` or `resolve` if i wouldn't use `return` then it would create a promise state and even execute the statements after it – terik_poe Feb 06 '17 at 07:27
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/134946/discussion-between-terik-poe-and-jfriend00). – terik_poe Feb 06 '17 at 07:29