In message <a43fbc6a0910261457r73912ed5k70e90e558908b...@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.
