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.

Answer 1

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.