harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: Idiomatic Java: inverted conditions
Date Tue, 27 Oct 2009 00:16:01 GMT
On 26/10/2009, Alexey Petrenko <alexey.a.petrenko@gmail.com> wrote:
> The construction which kills me is:
>
>  <if "some string".equals(str)> instead of <if str.equals("some string")>
>
>  But unfortunately the first form is better then second one :)

Surely it's only better if the "str" variable can be null?

i.e.

    if ("some string".equals(str)) // works even if str == null

is simpler than

   if (str != null && str.equals("some string"))


However, if "str" is known to be non-null, then use

   if (str.equals("some string"))

In the first case, I suggest including the comment to avoid problems
if someone tries to swap the variable and string.

>  Alexey
>
>  2009/10/27 Jesse Wilson <jessewilson@google.com>:
>
> > 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();
>  >
>  >            }
>  >
>  >
>  > ...or even more surprising, from HttpURLConnection:
>  >
>  >        if (0 < chunkLength) {
>  >
>  >            throw new IllegalStateException(Msg.getString("KA003"));
>  >
>  >        }
>  >
>  >
>  > 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.
>  >
>  > 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.
>  >
>  > Thanks,
>  > Jesse
>  >
>

Mime
View raw message