3 ms·
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.doesSo
by Zardoz84 8y ago
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.
- dahart 8y agoAh 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.
- XR0CSWV3h3kZWg 8y agoYeah 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.
- veebat 8y agoThere'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.
- Retra 8y agoJust surround it with // @formatter:off // @formatter:on