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 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.

Written by Andrey Karpov.

This error was found with PVS-Studio static analysis tool.

One thought on “Beware of the ?: operator and enclose it in parentheses

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s