Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: neutral (athena.apache.org: 194.196.100.162 is neither permitted
 nor denied by domain of mark.hindess@googlemail.com)
Message-Id: <200910262255.n9QMtTGE029775@d06av03.portsmouth.uk.ibm.com>
In-reply-to: <a43fbc6a0910261457r73912ed5k70e90e558908bb5e@mail.gmail.com>
References: <a43fbc6a0910261457r73912ed5k70e90e558908bb5e@mail.gmail.com>
Comments: In-reply-to Jesse Wilson <jessewilson@google.com>
   message dated "Mon, 26 Oct 2009 14:57:24 -0700."
From: Mark Hindess <mark.hindess@googlemail.com>
To: dev@harmony.apache.org
Subject: Re: Idiomatic Java: inverted conditions
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Mon, 26 Oct 2009 22:55:29 +0000


In message <a43fbc6a0910261457r73912ed5k70e90e558908bb5e@mail.gmail.com>,
Jesse Wilson writes:
>
> Harmony Team,
> 
> Continuing along with a theme, there's another C/C++ism in our Java
> code that frustrates me. Our Java code frequently inverts conditions
> from their natural language form. From HttpURLConnectionImpl:
> 
>             if (null == resHeader) {
> 
>                 resHeader = new Header();
> 
>             }

Even in C this is largely unnecessary since we compile most code with:

  -Wall -Werror

which should catch inadvertent assignments.

> ...or even more surprising, from HttpURLConnection:
> 
>         if (0 < chunkLength) {
> 
>             throw new IllegalStateException(Msg.getString("KA003"));
> 
>         }

This is definitely unnecessary.

> I find myself having to slow down to interpret what the code
> intends. I can't mentally parse "if 0 is less-than chunkLength" nearly
> as efficiently as the equivalent "if chunkLength is greater than 0"
> condition. From a quick survey of the code base, it looks like we use
> a mix of the two forms.

+1.  I agree it significantly impedes the reading of code.

> If anyone thinks I should avoid flipping of these conditionals back to
> their normal form, please let me know. In my logging patch, I flipped
> several "null == foo" checks and I found it made the code easier to
> read.

Again I'd rather separate patches but in this case it is trivial enough
that it is probably okay to make them in other patches.

-Mark.