Thursday, February 25, 2010

Coding styles that make me twitch, part 7

Parentheses and precedence

This week, I will strive to be better at explaining the problem scope, while heroically and miraculously maintaining brevity.

I used to be annoyed at seeing extraneous parentheses with the ternary conditional operator pair ? :, but then I discovered that the PHP language designers decided to break precedence (see example #3). So I have stopped twitching as much as I used to when seeing parentheses galore in that context.

In Perl Best Practices, Damian Conway notes that even though the low-precedence logic operators and, not and or may seem nicer, they can confuse the reader regarding the intended meaning of the code. But he does not — as far as I can recall or find — address how these relates to parentheses and precedence (I suppose that should be "parentheses && precedence").

I start twitching when someone insists on using parentheses like this (imagine that the example was more convoluted):
if (expr1 ||
(expr2 && expr3) ||
expr4
...)
That does not help understanding. It confuses me, as the read, regarding your intentions. Is there a piece of code missing, perhaps a little bit of || expr2b or || expr3b?

Sometimes, I even see misguidedly mixed-in letter-literal operators:
if (expr1 or
(expr2 && expr3) or
(!expr4)
...)

I much prefer seeing
if (expr1 ||
expr2 && expr3 ||
expr4
...)
or
if (expr1
|| expr2 && expr3
|| expr4
...)
depending on what floats your boat in the most stylish way imaginable.

I would have thought that the logical and/or/not part of operator precedence would be easy to understand. It is essentially the same in most programming languages: Ruby, Python, Perl, Java, C, ... and not even PHP managed to mess this one up.

The chance that anyone is going to be confused by your code because of these parentheses not being there is far, far smaller than the chance that they are going to be confused by their presence.

I have heard the defense "but the code is so non-obvious that I had to add them!" Well, make your code obvious instead.

4 comments:

Matt said...

Of course, really that's unfair, since you never showed

if (expr1
. . || (expr2 && expr3)
. . || expr4) {

(excuse the .s needed to format this sanely)

which is just as easy on my eyes as your preferred version and far clearer.

bakkushan said...

May I suggest new lenses? :D

ijw said...

It's strange - I've always thought of overbracketing as a robust programming style. If I think it makes what I'm doing clearer for other people to read, I tend to do it.

Conversely, how do I know that the programmer who wrote the code

if(expr1 || expr2 && expr3 || expr4)

really knows his precedence rules (and that everyone else that reads it will)? And worse still

if(expr1 || expr2
&& expr3 || expr4)

bakkushan said...

No, over-bracketing is not robust - not even jokingly when referring to lisp.

Over-bracketing with parentheses add visual clutter, which unnecessarily complicates expressions that probably are complex enough in themselves.

If you think those who read your code will be so badly versed in the fundamentals of operator precedence, you have a huge problem: your audience does not consist of programmers.

You cannot expect them to understand other code that you write, either. How do you solve that?

The answer is: don't write for non-programmers. Write for programmers, but try to make it clear that you yourself knew what you were doing.

Adding extraneous parentheses does not show that you know what you are doing.