1

I am attempting to use short circuiting between two function calls. Take for example this code:

private void BubbleDown(Graph.Vertex item)
{
    BubbleDownLeft(item) || BubbleDownRight(item);
}

Where BubbleDownLeft and BubbleDownRight both return a bool. This throws the following error:

Error CS0201 - Only assignment, call, increment, decrement, and new object expressions can be used as a statement

I know I can easily rewrite the function to use an if statement (but that's not as pretty) or even just assign it to a value I don't use:

private void BubbleDown(Graph.Vertex item)
{
    bool ignoreMe = BubbleDownLeft(item) || BubbleDownRight(item);
}

So why is my first coding example an error rather than valid or perhaps just a warning?

jgawrych
  • 3,322
  • 1
  • 28
  • 38
  • 1
    FWIW, this is not allowed in Java either. – John Hascall Feb 29 '16 at 22:15
  • 1
    Just speaking for me, but if I'm doing a code review, I'm not going to like the original (illegal) code or even the valid solution. A traditional "if" is going to be much more obvious as to both the intent and utility of the method. It somewhat pains me that you would consider the obvious code "not as pretty." – Anthony Pegram Feb 29 '16 at 22:16
  • 1
    What about: `a() || b() || c() || d() || e()`? -- that looks a fair bit more grotesque as a "traditional if". – John Hascall Feb 29 '16 at 22:23
  • 1
    I'll agree the extraneous ignoreMe is worse than an if, but the original is easily read. It clearly says that bubbleDown is the result of either the left or the right. Going with the a, b, c, d, e, example, you would have to put `if (!(a() || b() || c() || d())) e()` which would look like e() is the goal, but really it's a 5-branching path where the first path to succeed is taken. – jgawrych Feb 29 '16 at 22:29
  • 1
    In other words, you are using the boolean short circuit logic to execute functions until one of them succeeds. – holroy Feb 29 '16 at 22:32

2 Answers2

2

The suggested code doesn't make sense for a computer. When you ask it to BubbleDown() you tell it to either BubbleDownLeft() or BubbleDownRight() but also to ignore the result. The compiler has no incentive to use short-circuit evaluation since the result is ignored anyway. This short circuit-evaluation is what kicks in if this was an if statement.

In a comment you state:

I'll agree the extraneous ignoreMe is worse than an if, but the original is easily read. It clearly says that bubbleDown is the result of either the left or the right. Going with the a, b, c, d, e, example, you would have to put if (!(a() || b() || c() || d())) e()) which would look like e() is the goal, but really it's a 5-branching path where the first path to succeed is taken.

This clearly indicates that you want the short-circuit evaluation, and your options for getting this are either to change return type and do:

private bool BubbleDown(Graph.Vertex item)
{
    return BubbleDownLeft(item) || BubbleDownRight(item);
}

Or you could opt for an if statment with an empty block. Note the ; after the if statement:

private void BubbleDown(Graph.Vertex item)
{
   if (BubbleDownLeft(item) || BubbleDownRight(item)) ;
}

I think I would prefer the first, and then simply ignore the return value at the point where your calling BubbleDown(), or actually verify that it actually has executed any of the options truthfully.

holroy
  • 3,047
  • 25
  • 41
  • 2
    The code makes perfect sense: `if (!BubbleDownLeft(item)) BubbleDownRight(item);` And, the original syntax `a() || b();` is perfectly legal in C. – John Hascall Feb 29 '16 at 22:26
  • @JohnHascall, Then using the first option, would probably be the best bet. But it is a somewhat strange case of boolean shortcutting. – holroy Feb 29 '16 at 22:27
  • 2
    @JohnHascall Saying something is perfectly legal in C doesn't necessarily mean it makes sense. You can pretty much do whatever you want in C, the compiler won't stop you. C# stops you because it's *likely* whatever you've written is different from what you were meaning to write. In this case it appears that `BubbleDown` *should* return a bool, as both the other `BubbleDownX` appear to be able to fail, so the parent call should also return a failure notification as well. – Rob Feb 29 '16 at 22:47
  • @Rob I agree that C# is nannying you here, but not for the reason you suggest. The statement would still be illegal even if BubbleDown was bool and not void. – John Hascall Mar 01 '16 at 02:02
  • @JohnHascall Sorry, I didn't mean to say that the error was because of the return type of `BubbleDown`. I meant that it's an error because it's likely that throwing away the result is *not* what is intended. A solution for using the result is to return it, but in general, running the operator `||` and never using the result is likely to be incorrect (even if there are legitimate cases). – Rob Mar 01 '16 at 02:06
  • 1
    @Rob, that is why I suggest using the `return` pattern, which allows for testing on the return value, instead of blatantly ignoring it as originally suggested by OP (and which is allowed in C). – holroy Mar 01 '16 at 02:08
1

With C# 6 you can be more elegant:

private bool BubbleDown(Graph.Vertex item)
    => BubbleDownLeft(item) || BubbleDownRight(item);

But, anyway, in this way you would change method return type.

If you, strictly, wants to code like this, without changing BubleDown signature, you could do some trick with C# 6 expression bodied functions:

private void BubbleDown(Graph.Vertex item)
    => BubbleDownImplementation(item);

private bool BubbleDownImplementation(Graph.Vertex item)
    => BubbleDownLeft(item) || BubbleDownRight(item);

In addition, take a look at this Eric Lippert answer.

Community
  • 1
  • 1
  • 1
    Given that the whole point of `a() || b()` is its instantly understood succinctness, this giant encircling blob of pointless syntax seems decidedly unappealing. – John Hascall Mar 01 '16 at 11:48