CODEWITHSUNDEEP
cif-statementconditional-statementsreadabilityshort-circuiting
legends2k
legends2k

Reputation: 32904

Short-circuiting and readability

In this line

if ((last_search == NULL) || (last_search != NULL && total_results != 0))

I know that C's short-circuit evaluation rules say that only if last_search is not null will it try and evaluate the right-hand side of ||, hence it's equivalent to writing

if ((last_search == NULL) || (total_results != 0))

and I was advised to use the later by someone, but still isn't the former more readable? Also won't the compiler optimize out the redundant last_search != NULL?

Upvotes: 2

Views: 186

Answers (3)

Jens Gustedt
Jens Gustedt

Reputation: 78923

It is not only the short circuit evaluation that makes it unreadable but also the fact of using superfluous comparisons.

if ( !last_search || total_results)

is much easier to read than anything that you proposed.

Upvotes: 1

Jon
Jon

Reputation: 437386

This is subjective, but no, the first variant is not more readable because there's more to read. The most readable code (and the least buggy!) is that which does not exist. Just think what would happen if you wanted to check 3 or 4 conditions at once.

And here's another counterexample: would you write code like this?

if (number < 0) {
}
else if(number >= 0) {
    // why not just "else"?
}

As for performance: the compiler would probably optimize the redundant call away, but undoing the performance degradation does not help with the readability degradation.

Upvotes: 10

Luchian Grigore
Luchian Grigore

Reputation: 258618

No it's not. The second one is a lot more readable. Why would you keep the redundant check in there, regardless of whether the compiler will optimize it away?

The first version tells you that last_search is not NULL because it got there, but if you can't tell that from the first condition (last_search == NULL failed), you've probably got bigger issues than readability.

Upvotes: 5

Related Questions