Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Just today, I was talking with a team mate about this. We had some pice of code that it's like this :

    public boolean foo() {
      

      if (!this.doesSomethingWithA()) {
        return false;
      }
      
      if (!this.doesSomethingWithB()) {
        return false;
      }
      
      if (!this.fooBar2000.isEmpty()) {
        return false;
      }
      
      if (!this.anotherLongAttribute) {
        return false;
      }
      
      if (!this.anotherMethod()) {
        return false;
      }
      
      return true;
   }
We try to replace to a "one liner" return !A && !B && !C ... Well, the expression was very long and we noted that was more hard to read that the multiple if's. I suggested split the one liner expression on multiples lines doing something like this :

    return !this.doesSomethingWithA()
        && !this.doesSomethingWithB()
        && !this.fooBar2000.isEmpty()
        && !this.anotherLongAttribute
        && !this.anotherMethod();
However, the code formatter (eclipse), changes every time it to the hard to read one liner expression. So we ended using the multiple if's.


There's a semantic thing going on with this expression that is rather interesting.

First, consider this reworking:

    boolean a = this.doesSomethingWithA();
    if (a) {a = a && !this.doesSomethingWithB();}
    if (a) {a = a && !this.fooBar2000.isEmpty();}
    if (a) {a = a && !this.anotherLongAttribute;}
    if (a) {a = a && !this.anotherMethod();}
    return a;
And then this one:

    boolean a = this.doesSomethingWithA();
    a = a && !this.doesSomethingWithB();
    a = a && !this.fooBar2000.isEmpty();
    a = a && !this.anotherLongAttribute;
    a = a && !this.anotherMethod();
    return a;
In the first version and your original, it's clear that there's short circuiting and it can return early, but automatic formatting will bloat up the vertical size of the code. In the one-liner there's also short circuiting, but it doesn't format well. In my second version you get the formatting, but it will always evaluate every possibility.

In general, when I'm favoring code formatting, the variable declarations come out. Quickly aliasing something into a name when it could exist purely as an expression adds a degree of conceptual flexibility. It keeps the code local(no new function name and jumping over into it). But it does also result in this kind of unnecessary computation.


Ah yeah, the one-liner would get killed by my work's code formatting rules too. (clang-format)

Total tangent, but I'm very much looking forward to more and better alignment rules becoming pervasive. I love it when similar things line up, it's easier to read, easier to modify, easier to see mistakes, better supports column selection and multi selection. I'll take alignment over almost any other code formatting issue that programmers debate endlessly. ;)

Having a code formatter is a greater good though, so yeah sometimes multiple ifs is just a necessity. At least the original version is clear and easy to follow.


Just surround it with // @formatter:off // @formatter:on


Yeah the first version looks like something that many people would try to 'optimize' but in reality it is extremely readable and should be very easy to change/extend.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: