4. Beware of the ?: operator and enclose it in parentheses

Fragment taken from the Haiku project (inheritor of BeOS). The error is detected by the following PVS-Studio diagnostic: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '-' operator.

bool IsVisible(bool ancestorsVisible) const
{
  int16 showLevel = BView::Private(view).ShowLevel();
  return (showLevel - (ancestorsVisible) ? 0 : 1) <= 0;
}

Explanation

Let's check the C/C++ operation precedence. The ternary operator ?: has a very low precedence, lower than that of operations /, +, <, etc; it is also lower than the precedence of the minus operator. As a result, the program doesn't work in the way the programmer expected.

The programmer thinks that the operations will execute in the following order:

(showLevel - (ancestorsVisible ? 0 : 1) ) <= 0

But it will actually be like this:

((showLevel - ancestorsVisible) ? 0 : 1) <= 0

The error is made in very simple code. This illustrates how hazardous the ?: operator is. It's very easy to make a mistake when using it; the ternary operator in more complex conditions is pure damage to the code. It's not only that you are very likely to make and miss a mistake; such expressions are also very difficult to read.

Really, beware of the ?: operator. I've seen a lot of bugs where this operator was used.

Correct code

return showLevel - (ancestorsVisible ? 0 : 1) <= 0;

Recommendation

In previous articles we've already discussed the problem of a ternary operator, but since then I've become even more paranoid. The example given above shows how easy it is to make an error, even in a short and simple expression, that's why I'll modify my previous tips.

I don't suggest rejecting the ?: operator completely. It may be useful, and even necessary sometimes. Nevertheless, please do not overuse it, and if you have decided to use it, here is my recommendation:

ALWAYS enclose the ternary operator in parentheses.

Suppose you have an expression:

A = B ? 10 : 20;

Then you should write it like this:

A = (B ? 10 : 20);

Yes, the parentheses are excessive here...

But, it will protect your code later when you or your colleagues add an X variable to 10 or 20 while doing code refactoring:

A = X + (B ? 10 : 20);

Without the parentheses, you could forget that the ?: operator has low precedence, and accidentally break the program.

Of course, you can write "X+" inside the parentheses, but it will still lead to the same error, although it is additional protection that shouldn't be rejected.