What is the best practice concerning C# short-circuit evaluation?

Question

An answer and subsequent debate in the comments in another thread prompted me to ask:

In C# || and && are the short-circuited versions of the logical operators | and & respectively.

Example usage:

if (String.IsNullOrEmpty(text1) | String.IsNullOrEmpty(text2) | String.IsNullOrEmpty(text3))
{
    //...
}

versus:

if (String.IsNullOrEmpty(text1) || String.IsNullOrEmpty(text2) || String.IsNullOrEmpty(text3))
{
    //...
}

In terms of coding practice which is the better to use and why?

Note: I do realize this question is similar to this question but I believe it warrants a language specific discussion.

Konrad Rudolph · Accepted Answer · 2008-12-12T00:03:21.303

45

In terms of coding practice which is the better to use and why?

Simple answer: always use the short-circuited versions. There’s simply no reason not to. Additionally, you make your code clearer because you express your intent: logical evaluation. Using the bitwise (logical) operations implies that you want just that: bit operations, not logical evaluation (even though the MSDN calls them “logical operators” as well, when applied to boolean values).

Additionally, since short-circuiting only evaluates what needs evaluating, it’s often faster, and it allows to write such code as

bool nullorempty = str == null || str.Length == 0;

(Notice that to solve this particular problem a better function already exists, namely string.IsNullOrEmpty which you also used in your question.) This code wouldn’t be possible with the bitwise logical operations because even if str were null, the second expression would get evaluated, resulting in a NullReferenceException.

EDIT: If you want side-effects to occur in a logical context please still don’t use bitwise operations. This is a typical example of being too clever. The next maintainer of the code (or even yourself, after a few weeks) wo sees this code will think “hmm, this code can be cleared up to use conditional operators,” thus inadvertedly breaking the code. I pity whoever is in charge of fixing this bug.

Instead, if you have to rely on side, effects, make them explicit:

bool hasBuzzed = checkMakeBuzz();
bool isFrobbed = checkMakeFrob();
bool result = hasBuzzed || isFrobbed;

Granted, three lines instead of one. But a much clearer code as a result.

edited Dec 12 '08 at 00:03

answered Dec 11 '08 at 21:43

Konrad Rudolph

530,221
131
937
1,214

1

this ia a nice answer, but can you change "bitwise" operators to "logical" and "logical" to "conditional"? This is the terminology MSDN uses. technically both are "logical" but the doubled versions also short-circuit – Jimmy Dec 11 '08 at 22:03
"The conditional-AND operator (&&) performs a logical-AND of its bool operands, but only evaluates its second operand if necessary." "http://msdn.microsoft.com/en-us/library/2a723cdk(VS.71).aspx" – Jimmy Dec 11 '08 at 22:05
4

Perfect answer. +1 for side effect discussion -- agreed, for maintenance, always be explicit. 3 LOCs vs. maintenance nightmare, 3 LOCs wins hands-down. – John Rudy Dec 11 '08 at 22:38
@Jimmy: Your link more or less contradicts your terminology claim. Actually, I see that you're right to some extend: the MSDN calls both operators `logical`, when applied to bools. I'll reflect that in my answer. – Konrad Rudolph Dec 11 '08 at 23:59
2

Also, even if you DON'T happen to think that SC evaluation is the way to go, do it anyway because people have been writing C/C++ this way for 30 years. It's pretty much Common Law now. – Dave Markle Dec 12 '08 at 00:07
But you are not writing C++. A C++ compiler goes through [pretty heroic measures](http://stackoverflow.com/questions/26124620/why-does-msvc-emit-a-useless-movsx-before-performing-this-bit-test) to avoid the cost of [branch prediction fail](http://stackoverflow.com/a/11227902/17034). In C# you always get a branch. And if the left-hand operand expression is poorly predicted then you do pay heavily, makes the || or && operator as much a *five* times slower on a modern processor. Always measure. – Hans Passant Jun 10 '16 at 14:43
@Hans I’m pretty strongly of the opinion that this requires fixing in the compiler, not user code. Because fixing it in user code would result in (a) weakening the code’s guarantees, and (b) performing impossible to predict and sometimes-impossible to benchmark “optimisations”, thus (c) premature optimsations, which will often turn out to be (d) pessimisations. In short, this isn’t an area where user code optimisation is a good strategy. This is work for an optimising compiler. – Konrad Rudolph Jun 10 '16 at 15:53
Hmm, I've never once seen a program optimized by a strong opinion. It is in fact the worst way to do it. – Hans Passant Jun 10 '16 at 16:02
@Hans Hard as though it may be to believe, this is exactly what my previous comment was saying. This is a terrible area in which to trust the user to properly optimise, because collecting reliable data here is very hard. As a consequence, you’ll get opinion-based optimsations. And worse, it involves a painful trade-off against weakened semantics. So I’d be very wary of doing this. – Konrad Rudolph Jun 10 '16 at 16:20

score 7 · Answer 2 · answered Dec 11 '08 at 22:36

7

I am going to answer this question in reverse: when is the only time I use the logic operators?

I will sometimes use the logical comparisons when I have a series of (low-cost) conditionals that must all be met. For example:

bool isPasswordValid = true;

isPasswordValid &= isEightCharacters(password);
isPasswordValid &= containsNumeric(password);
isPasswordValid &= containsBothUppercaseAndLowercase(password);

return isPasswordValid;

In my opinion, the above is more readable than:

return (isEightCharacters(password) &&
        containsNumberic(password)  &&
        containsBothUppercaseAndLowercase(password));

The downside is that it's a bit more esoteric.

answered Dec 11 '08 at 22:36

tsimon

8,362
2
30
41

Ah, interesting use of the bitwise assignment operators. Thanks! – Michael Whatcott Feb 15 '13 at 23:37
2

Strictly in your example, if the password is 7 characters, it is not valid. So whether it contains Numeric and/or Upper/Lowercase is irrelevant and changes nothing. Short-Circuiting should indeed be applied. If readability of a single return statement is an issue, you could change the code to three lines that say: `if (!RequirementCheck(input)) return false;` then the last line should just say `return true;`. However, if you wanted each req. checked to return a specific message, then combine the messages, then this example no longer applies to this question and we have more irrelevance. – Suamere Dec 30 '13 at 16:31
I sometimes use bitwise assignment operators in this way inside a loop, eg. `int sum = 0;` `bool anyOdd = false;` `foreach (int number in GetNumbers())` `{` `sum += number;` `anyOdd |= number % 2 == 1;` `}` But in this answer I agree with @Suamere that short-circuiting is cleaner. – hypehuman Sep 08 '15 at 15:20

score 2 · Answer 3 · answered Dec 11 '08 at 21:48

Use && and || when you only care about the result and want to know that result as soon as possible and none of your expressions have side effects that must occur even if the boolean condition is not met. That is to say, pretty much always.

Use & and | when every expression must be evaluated (for example, if you have side effects from your expressions). But since you should never have side effects that your program depends on that must occur even if the boolean condition is not met, you should probably not be using & and |.

For example, this would probably be exceptionally silly:

if (false & somethingThatUpdatesTheDatabase()) { /* ... */ }

What is the best practice concerning C# short-circuit evaluation?

3 Answers3

Linked