2007-08-12

Java Blooper #1: Ternary Insanity

From time to time, we all write code that could be clearer. Sometimes in the rush of solving a problem, we don't pay attention to the micro details of the code flowing from our fingertips. Other times, we refactor some existing code, and don't necessarily take the opportunity to clean up as much as we could.

I find it useful sometimes when reading code to think about whether it could be rewritten in a more straightforward way, and if so whether any lessons can be learned about writing tight and expressive, and most importantly, readable code.

Over the next few weeks, I'm going to blog weekly examples of some Java code bloopers that I've seen. All the examples are real and have been observed "in the wild". However some have been simplified somewhat to make a point. I'll post each blooper on a Monday, and follow up with a rewritten version the following Friday.

Most of the examples are pretty simple, and easily cleaned up - feel free to post observations as comments to the entries, and don't be shy to smack me down on the solutions if there's a better way to express the same thing.

The first installment is called Ternary Insanity. 

class Thing {
  private boolean _visible;

  boolean isVisible() {
    return _visible ? true : false;
  }
}

Posted by Brian Duff at 23:30

Labels: , , ,

AddThis Social Bookmark Button

9 comments:

  1. or how about this...

    if (_visible) {
      return new Boolean(true);
    } else {
      return new Boolean(false);
    }
    ReplyDelete
  2. I have seen:

    if (_visible == true) {
    return true;
    } else {
    return false;
    }
    ReplyDelete
  3. ugg: which is the long-hand version of the original post.
    ReplyDelete
  5. I consider this a blooper too:

    if (a little bit complex boolean expression) {
    return true;
    }
    else {
    return false;
    }
    better alternative:
    return (a little bit complex boolean expression);
    ReplyDelete
  6. This is our new star java developer:

    Remember that the object being referenced in the foreach loop can NEVER be null:

    for (Element e : l.getElements())
    {
    if (e == null) continue;

    ...
    }

    WTF?
    ReplyDelete
  7. "Remember that the object being referenced in the foreach loop can NEVER be null"

    >> This is wrong. The non-nullness has nothing to do with the foreach loop as this code proves:

    for (String s : new String[] { null }) {
    System.out.println(s == null);
    }

    prints out: true
    ReplyDelete
  8. someone suggested simply "return _visible;" this may give an outsider intimate access to a private variable.

    class Thing {
    private boolean _visible;

    boolean isVisible() {
    boolean copy = _visible;
    return copy;
    }
    }
    ReplyDelete
  9. @mark, that's not the case. There's no way to mutate the _visible field if its value is directly returned from isVisible().
    ReplyDelete

Subscribe to: Post Comments (Atom)