Which code is more readable?

Question

Suppose I have two methods bool Foo() and bool Bar(). Which of the following is more readable?

if(Foo())
{
    SomeProperty = Bar();
}
else
{
    SomeProperty = false;
}

or

SomeProperty = Foo() && Bar();

On the one hand, I consider the short-circuiting && to be a useful feature and the second code sample is much shorter. On the other hand, I'm not sure people are generally accustomed to seeing && outside a conditional statement, so I wonder if that would introduce some cognitive dissonance that makes the first sample the better option.

What do you think? Are there other factors that affect the decision? Like, if the && expression is longer than one line that can fit on the screen, should I prefer the former?

---

Post-answer clarifications:

A few things that I should have included in the initial question that the answers brought up.

• Bar() may be more expensive to execute than Foo(), but neither method should have side effects.
• The methods are both named more appropriately, not like in this example. Foo() boils down to something like CurrentUserAllowedToDoX() and Bar() is more like, XCanBeDone()

Answer 1

I agree with the general consensus that the Foo() && Bar() form is reasonable unless it is the case that Bar() is useful for its side effects as well as its value.

If it is the case that Bar() is useful for its side effects as well as it's value, my first choice would be to redesign Bar() so that production of its side effects and computation of its value were separate methods.

If for some reason that was impossible, then I would greatly prefer the original version. To me the original version more clearly emphasizes that the call to Bar() is part of a statement that is useful for its side effects. The latter form to me emphasizes that Bar() is useful for its value.

For example, given the choice between

if (NetworkAvailable())
  success = LogUserOn();
else
  success = false;

and

success = NetworkAvailable() && LogUserOn();

I would take the former; to me, it is too easy to overlook the important side effect in the latter.

However, if it were a choice between

if (NetworkAvailable())
  tryWritingToNetworkStorage = UserHasAvailableDiskQuota();
else
  tryWritingToNetworkStorage = false;

and

tryWritingToNetworkStorage = NetworkAvailable() && UserHasAvailableDiskQuota();

I'd choose the latter.

Answer 2

I like this shorthand notation assuming your language permits it:

SomeProperty = Foo() ? Bar() : false;

Answer 3

SomeProperty = Foo() && Bar();

This is way more readable. I don't see how anyone could choose the other one frankly. If the && expression is longer then 1 line, split it into 2 lines...

SomeProperty = 
   Foo() && Bar();

      -or-

SomeProperty = Foo() && 
               Bar();

That barely hurts readability and understanding IMHO.

Answer 4

It depends on what Foo and Bar do.

For example, IsUserLoggedIn() && IsUserAdmin() would definitely be better as an &&, but there are some other combinations (I can't think of any offhand) where the ifs would be better.

Overall, I would recommend &&.

Answer 5

Neither. I'd start with renaming SomeProperty, Foo and Bar.

What I mean is, you should structure your code as to convey your intentions clearly. With different functions, I might use different forms. As it stands, however, either form is fine. Consider:

IsFather = IsParent() && IsMale();

and

if (FPUAvailable()) {
    SomeProperty = LengthyFPUOperation();
} else {
    SomeProperty = false;
}

Here, the first form stresses the logical-and relationship. The second one stresses the short-circuit. I would never write the first example in the second form. I would probably prefer the second form for the second example, especially if I was aiming for clarity.

Point is, it's hard to answer this question with SomeProperty and Foo() and Bar(). There are some good, generic answers defending && for the usual cases, but I would never completely rule out the second form.

Answer 6

In what way do you think people might misinterpret the second one? Do you think they'll forget that && shortcircuits, and worry about what will happen if the second condition is called when the first is false? If so, I wouldn't worry about that - they'd be equally confused by:

if (x != null && x.StartsWith("foo"))

which I don't think many people would rewrite as the first form.

Basically, I'd go with the second. With a suitably descriptive variable name (and conditions) it should be absolutely fine.

Answer 7

In the case where the conditional expressions are short and succinct, as is this case, I find the second to be much more readable. It's very clear at a glance what you are trying to do here. The first example though took me 2 passes to understand what was happening.

Answer 8

I would go for the second, but would probably write it like the first time, and then I would change it :)

Answer 9

When I read the first one, it wasn't immediately obvious SomeProperty was a boolean property, nor that Bar() returned a boolean value until you read the else part of the statement.

My personal view is that this approach should be a best practice: Every line of code should be as interpretable as it can with reference to as little other code as possible.

Because statement one requires me to reference the else part of the statement to interpolate that both SomeProperty and Bar() are boolean in nature, I would use the second.

In the second, it is immediately obvious in a single line of code all of the following facts:

• SomeProperty is boolean.
• Foo() and Bar() both return values that can be interpreted as boolean.
• SomeProperty should be set to false unless both Foo() and Bar() are interpreted as true.