5

When I only perform the next steps in my algorithm if various conditions are met I express it this way:

if (sc1 || sc2) {
  do();
  various();
  things();
}

When I only perform the next steps based on the fulfillment of a promise I can express it like this:

asyncCheck().then(ac1 => {
  if (ac1) {
    do();
    various();
    things();
  }
}

How can I express in idiomatic JavaScript when condition sc1 is a just a regular old synchronous expression but condition ac2 comes asynchronously via a promise?

Assume native ES6 promises and nontrivial code that gets executed if the conditions are met.

For instance, this "obvious" way seems ugly:

if (sc1) {
  do();
  various();
  things();
} else {
  asyncCheck().then(ac2 => {
    if (ac2) {
      do();
      various();
      things();
    }
  }
}

I could put the repeated code in a function that gets called either way, which is less ugly, but I feel like I could be missing something more idiomatic that other JavaScript programmers might be using.

I should add this observation too: Since in my case, there is a logical or, it should short circuit so it shouldn't bother with the slow deferred check if the simple check is already false.

hippietrail
  • 15,848
  • 18
  • 99
  • 158
  • 2
    Wrap the non-Promise in a Promise and use `Promise.all()`? *edit* oh wait you want `||` ... – Pointy Jun 19 '16 at 13:58
  • The repeated code is *already* in a function. Just move it out and give it a name. You can have it return the result of the condition, so `if (!my_func(sc1)) { asyncCheck().then(my_func) }` –  Jun 19 '16 at 14:01
  • @Pointy: I've been thinking of that but then I couldn't short circuit the `||` if the non-promise condition is false. I might add that to the question. – hippietrail Jun 19 '16 at 14:02
  • ...and of course you can use the logical OR too `my_func(sc1) || asyncCheck().then(my_func)` –  Jun 19 '16 at 14:09
  • 1
    Or if you have several cases of this, abstract out the `if` into a generic function that takes the "sync" condition and a callback. Then `maybeAsync(sc1, function() { do(); various(); things() })` –  Jun 19 '16 at 14:12
  • 1
    @squint: The last suggestion is one of the more promising kinds of solutions I've been pondering. (Pardon the pun) – hippietrail Jun 19 '16 at 14:22

4 Answers4

5

Pretty simple actually:

Promise.resolve(sc1 || asyncCheck()).then(cond => {
  if (cond) {
    do();
    various();
    things();
  }
});

Admittedly the truthiness of sc1 is possibly evaluated twice, but otherwise it does short-circuit. You can also use

(sc1 ? Promise.resolve(true) : asyncCheck()).then(cond => { … });
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Where is `ac2` coming from? –  Jun 19 '16 at 15:10
  • @squint: I guess OP used `asyncCheck()` for that. I'll edit. – Bergi Jun 19 '16 at 15:11
  • 1
    Well, at least this is a demonstration of a both clean and correct use of Promises for the given situation, so I have to give +1 for that. –  Jun 19 '16 at 15:17
  • OK now this is the kind of thing I was hoping for when I said "idiomatic"! Looks sweet so I'll try it out and come back to vote... – hippietrail Jun 19 '16 at 15:44
  • 1
    @hippietrail: Also have a look at [if with asynchronous case](http://stackoverflow.com/q/29802311/1048572) or [if-else flow in promise](http://stackoverflow.com/q/26599798/1048572) - the short-circuiting `||` operator is nothing but sugar for an if-else-statement. – Bergi Jun 19 '16 at 15:51
2

Thinking more and with everybody's help here, this is my current evolution:

function ifConditionsMet() {
  if (sc1) {
    return Promise.resolve(true);
  } else {
    return asyncCheck();
  }
}

ifConditionsMet().then(() => {
  do();
  various();
  things();
});

I this this should work fine with any exception handling too...

hippietrail
  • 15,848
  • 18
  • 99
  • 158
  • Yes it's partially pseudocode since sc1 might be something parametric or globally doable. The important bit is that it's synchronous. – hippietrail Jun 19 '16 at 14:38
  • Sorry for the downvote but this answer has some fundamental problems. Even if we ignore the fact you're not passing a function as a callback in the example - What happens here if `asyncCheck()` rejects with an error? – Benjamin Gruenbaum Jun 19 '16 at 14:39
  • You also probably want the function to return a promise so it can be continued later. – Benjamin Gruenbaum Jun 19 '16 at 14:40
  • In my current code it's a toy for personal use and for self education of newer js features. I would investigate any rejections as they come up. But it's a good point. Let me add a "promisified" version. The callback amidst promises seemed to be a code smell to me already... – hippietrail Jun 19 '16 at 14:46
  • This is what happens... people want to champion Promises as the solution to all problems, so they come up with imaginary issues as rationale for down-voting any simple, clean solution that doesn't use them. If `asyncCheck` is rejected with an error, then clearly the condition wasn't met... and it could be that he's handling the error in a generic way in that function. –  Jun 19 '16 at 14:51
  • 2
    Your second implementation should avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Jun 19 '16 at 15:17
  • 1
    @squint Honestly I find your comment rude. Maybe it's not that I _want_ to downvote, maybe I hate downvoting and have only done so so he can fix an issue? Maybe my knowledge isn't based on "what I liked" but on hundreds of answers and thousands of hours of investigating the subject as well as involvement in the spec and contributing to the two most promise libraries and node itself? Could it just be possible that I simply _disagree_ with you on whether or not this is a good solution? Did it occur to you I'm trying to _help_ OP reach a good solution rather than suffer with a bad one? – Benjamin Gruenbaum Jun 19 '16 at 15:24
  • Also, literally no one is championing promises as the solution for everything. The TC is working on various different primitives - like async iterators and observables for solving different use cases. – Benjamin Gruenbaum Jun 19 '16 at 15:25
  • @BenjaminGruenbaum: It's truthful. Happens *all the time* with people here. I don't care if people use Promises or not, but a bunch of folks jump on the inane *"if you're not doing it my way, you're doing it wrong"* bandwagon. Zealotry in programming is just so silly. –  Jun 19 '16 at 15:33
  • @hippietrail: Imagine that you didn't have Promises. What would you have done? You'd probably just pass a callback to `asyncCheck`. Then it's the same as your first solution but without all the problem of *"fixing the solution"*. Since this is a single-purpose function, it would close over the same `sc1` condition, and avoid the async call the same way. Or you could pass the condition in, and be able to reuse it this way in other places. I don't care what you use, but keep an open mind to simple solutions that don't follow the latest trends. –  Jun 19 '16 at 15:37
  • @squint I've been here monitoring the promises tag (and so has Bergi) for at least 3 years. I have literally never seen this happen. Shoving all async actions to promises is not just zealoty - it's patently wrong and shows a lack of fundamental understanding of promises and what they are. Promises shine as a DSL for composing asynchronous actions - they make your life much easier in complex flows and by all means _do_ consider learning them - but they don't replace anything nor do they simplify this case since there is only one async action here. – Benjamin Gruenbaum Jun 19 '16 at 15:38
  • I don't mind the downvote. I'm not new to Stack(Overflow|Exchange) and I'm learning quite a bit thanks to the feedback this question has attracted. – hippietrail Jun 19 '16 at 15:48
  • @BenjaminGruenbaum: The *argument from authority* fallacy needs to end. *"...hundreds of answers ...thousands of hours ...contributing to libraries and node ...involvement in the spec"*. None of these are pertinent. You say it's "bad" but your definition of "bad" is merely based on your personal bias. This is a clear demonstration of zealotry. I don't use Promises, because I find them profoundly unnecessary, but I sure don't downvote people just because they use them. –  Jun 19 '16 at 15:53
  • @hippietrail: It's not so much the vote; it's the pompous *"my way or the highway"* attitude, and if you call them out on it, suddenly, you're *"rude"*. Such arrogance. –  Jun 19 '16 at 15:55
  • While we're being honest - You don't use promises because you don't understand them even the slightest. I explained why I found your comment _rude_, not my answer - when you asked me to explain what's wrong with the other answer - I did. This is not zealotry at all. – Benjamin Gruenbaum Jun 19 '16 at 16:00
  • 1
    Actually in this particular case I didn't find any attitude though I have encountered such in many other occasions on SO and other fora. In fact I took it as a prod to keep working on my code to the point it would deserve an upvote. I do appreciate how in printed text connotations and attitudes are so subjective. – hippietrail Jun 19 '16 at 16:00
  • @hippietrail the downvote is a hint for you to fix your answer, not a sign of anything negative. – Benjamin Gruenbaum Jun 19 '16 at 16:01
  • @hippietrail watch out for the explicit construction antipattern Bergi mentioned. You can just `return` promises. See stackoverflow.com/questions/22539815/arent-promises-just-callbacks . – Benjamin Gruenbaum Jun 19 '16 at 16:05
  • @BenjaminGruenbaum: *"You don't use promises because you don't understand them even the slightest."* I do understand them. I just don't know all the minutia of their syntax, because I've never found a situation where it was worth learning them. You're not being honest; you're being presumptuous. This again suggests a mentality wherein your personal opinion becomes "truth". –  Jun 19 '16 at 16:06
  • @squint I'm going to disengage now. I don't think I'm being presumptuous and I do think you're being rude and that you don't understand promises but are looking for justification for your incompetence - you probably feel differently and this discussion is not suiting any of us or getting us anywhere. Let's agree to disagree and hopefully converse next time under more positive circumstance. – Benjamin Gruenbaum Jun 19 '16 at 16:09
  • 1
    @hippietrail your last solution looks valid, you might want to refactor it a little and get `const ifConditionsMet = (sc1, fn) => Promise.resolve(sc1 || fn());` and then do `ifConditionsMet(sc1, asyncCheck).then(...` for shorter code. The first solution is still wrong because if `asyncCheck` throws your code does not handle it - a promise rejecting and no one listening is like an unhandled exception. – Benjamin Gruenbaum Jun 19 '16 at 16:12
  • @BenjaminGruenbaum: The incompetence claim another silly attempt at establishing authority. I expressly admitted that I don't use Promises. Does that make one incompetent? If I was trying to instruct someone on how or when to use them, and was making errors while doing so, that would be different. I'm just saying use them if you want, but keep an open mind. The fact that you accuse me of being incompetent merely because I suggest alternate solutions, again demonstrates my point. –  Jun 19 '16 at 16:15
  • 1
    Indeed I have always struggled with chaining promises in my own code though it seemed to make sense in other people's code. Writing that version with the antipattern I felt there was some code smell and the help from both of you has helped me pinpoint that and understand it (-: I can now throw away the callback version and my promise version is only minor mutations/refactorings away from the submitted idiomatic solution I feel... – hippietrail Jun 19 '16 at 16:15
  • This is what I find amazing... `ifConditionMet(my_func)` == "bad", whereas `ifConditionMet().then(my_func)` == "good", apparently because of some ethereal notion of idiomatic superiority. I guess everyone needs to find religion somewhere... –  Jun 19 '16 at 16:36
1

Well no, promises just abstract values. While it is possible to express control flow structures with promises I don't think it's a good idea.

function or(v1, v2, ifPath, elsePath) { 
   v1 = Promise.resolve(v1);
   v2 = Promise.resolve(v2);
   return Promise.all([v1, v2])
          .then(([v1, v2]) => (v1 || v2) ? ifPath() : elsePath());
}

Which would let you do:

or(ac1, ac2, () => {
  do();
  various();
  things();
}, () => {});

But honestly that's a pretty bad idea in general. It's leaky (no short circuit) and doesn't compose with other abstractions. You can do a much more basic:

Promise.all([ac1, ac2]).then(([r1, r2]) => {
   if(r1 || r2) { 
      do();
      various();
      things();
   }
});

Which is easier and easy to compose with. In the next version of JavaScript (ES8, ES2017) you'll very likely have async/await which would let you do the following directly:

// the function containing this needs to be marked as `async`
if (sc1 || await sc2) {
  do();
  various();
  things();
}

Which will make things easier. This is still a little leaky but much much less so.

Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
  • I'm trying to understand how this would work. Doesn't the second condition come *from* the result of the async operation? It seems like you're using it before it's available, but then maybe I'm just not reading it correctly. –  Jun 19 '16 at 14:25
  • I've been dying to play with `async / await`! I like your thought process here. It's the most didactic response so far. – hippietrail Jun 19 '16 at 14:26
  • @squint note the array destructuring in the function argument You can do `Promise.all([v1, v2]).then(([p1, p2]) =>` which opens the array for you, if you name the `v`s and the `p`s both the same way it'll do what it does in the answer. I can create a fiddle if you'd like. – Benjamin Gruenbaum Jun 19 '16 at 14:28
  • @hippietrail well in bluebird we have `Promise.coroutine` which lets you do coroutines _today_ with generators. We hardly invented it - Q has it too and also a lot of other runners. In fact building a completely correct async await with generators is a 10 lines of code exercise :) If you like async await definitely consider it. I plan to write a transpiler that goes the other way for my own code (converting coroutines to async functions) and I've been using coroutines for a while. For frame of reference, async/await landed in Chakra (Edge) a year ago and in V8 (Chrome) 2 weeks ago. – Benjamin Gruenbaum Jun 19 '16 at 14:30
  • But I mean you're calling it with `or(ac1, ac2, () => {...`. In the question, he has `sc1` and `ac2` are the conditions. Are they instead functions in this case? I don't use Promises, so please excuse my inquiries. –  Jun 19 '16 at 14:31
  • @squint I just assumed ac1 and ac2 are the boolean values being checked or promises for boolean values. Promises are worth learning and I'm sure you'll love them by the way - they're hard to grasp initially but when you use them they make your life a ton easier - this is more true in Node than in the client where you have complex async workflows. – Benjamin Gruenbaum Jun 19 '16 at 14:38
  • But that's what I mean. In the original code, the `ac2` would not be available until after the async operation, so I don't see how it could be passed in. I have to admit that I have a strong bias against things like Promises. I find that the full expressiveness of the language makes for simpler and clearer code than the limitations of an abstraction. –  Jun 19 '16 at 14:39
  • @squint promises _are_ part of the language. They're just as much JavaScript as `Date` or `Array`. All new APIs like `fetch` return them and async functions use them. Any of these things (ac1 or ac2) can be promises - so you'd pass promises directly. I admit I don't really understand the naming convention. – Benjamin Gruenbaum Jun 19 '16 at 14:41
  • Yes `ac1` and `ac2` are boolean values but could be expressions or anything else boolean. In my real code I fetsch some JSON and resolve true or false after examining the returned object. – hippietrail Jun 19 '16 at 14:41
  • @hippietrail then it would work just fine in your example if they're either boolean or promises for booleans. – Benjamin Gruenbaum Jun 19 '16 at 14:41
  • @BenjaminGruenbaum: Promises are part of the library defined in the spec. They are an abstraction over the language-level constructs. But before we quibble over pedantries, the point is that they're unnecessary and limiting. People try to shoehorn everything into them, and end up with longer, less clear code than they would if they had just thought through the problem using the tools they have. The `await` syntax is a language-level construct, and makes much more sense as a solution. A library-level solution made sense (for those who wanted it) when the language couldn't handle it. –  Jun 19 '16 at 14:46
  • But if he has to create more promises to represent `ac2`, then it's just getting longer and more complex, and is demonstrating my point. His solution is much cleaner. –  Jun 19 '16 at 14:47
  • The `Promise.all` doesn't seem to short-circuit either – Bergi Jun 19 '16 at 15:07
  • Yes, I said it does not. @squint his solution doesn't work though. – Benjamin Gruenbaum Jun 19 '16 at 15:08
  • @BenjaminGruenbaum: Ah, the description of "*You can do a much more basic*" sounded as if that was short-circuiting. – Bergi Jun 19 '16 at 15:14
  • @squint will crash the NodeJS process in a future version most likely and will log the rejection to stderr in the next semver. Not to mention it is not dealing with the error. – Benjamin Gruenbaum Jun 19 '16 at 15:47
  • @BenjaminGruenbaum: Will supposedly and presumably crash vaporware, and your *assumption* that the error isn't already handled means it "doesn't work"? Work on your answer. It uses a condition that doesn't yet exist. –  Jun 19 '16 at 15:51
0
/**
 * Run conditions in short-circuit manner. If a condition resolves to true, run trueCallback. If all conditions resolve to false, run falseCallback.
 * @param trueCallback
 * @param falseCallback
 * @param reject Optional. If any condition is rejected, call this callback. Neither trueCallback or falseCallback will be called.
 * @param conditions
 */
function promisesOr(trueCallback, falseCallback, ...args) {
  let conditions;
  let reject;
  if (args.length === 0)
    throw new Error("Expected usage is promisesOr(trueCallback, falseCallback, conditions) or promisesOr(trueCallback, falseCallback, reject, conditions)");
  else if (args.length === 1) {
    conditions = args[0];
  } else {
    reject = args[0];
    conditions = args[1]
  }

  if (conditions.length === 0)
    falseCallback();
  else {
    conditions.shift()().then(r => {
      if (r)
        trueCallback();
      else
        promisesOr(trueCallback, falseCallback, reject, conditions);
    }).catch(reject);
  }
}

I wrote this function for my project. The usage is like the following.

If detectA resolves to true, promisesOr immediately runs trueCallback, without calling detectB or detectC.

detectA and detectB are async conditions and detectC is a synchronous condition but wrapped as async.

promisesOr(()=>document.write("A or B or C exists."),
            ()=> document.write("false"),
          [detectA, detectB, detectC]);

See the working code on https://jsfiddle.net/1rxudtzy/

Gqqnbig
  • 5,845
  • 10
  • 45
  • 86