The other day I was pair-programming with a colleague at my client's site, and after we had a newly written unit-test fail, the implementation that my colleague wrote was something like this:
public boolean isConsumable() {
if (STATUS_NEW.equals(getStatus())) {
return true;
} else {
return false;
}
}
After the test passed, I went ahead and did a refactoring by turning the method into:
public boolean isConsumable() {
return STATUS_NEW.equals(getStatus());
}
The test passed again. However, my colleague wasn't happy with that implementation. I was taken aback because I thought since it was shorter, that meant we had less code to maintain. But, my colleague expressed that he found it more understandable as an English-statement to say something like:
if a.equals(b) return true else return false
instead of
return a.equals(b)
The first time I personally learned the ternary expression "condition ? true-result : false-result", I found it a mind-bender and preferred to use traditional If-Else statements instead. After a while, I got used to the ternary expression and it became a basic construct in my thinking. In the same token, right now whenever I read a line like "return a.equals(b)" under a boolean method, I quickly translate it in my mind to something like: method value is determined by equality of a to b. Therefore, I believe it is just a matter of habbit and getting used to the shorter syntax.
Still, I am curious to know other people's perspectives on this. Which syntax do you prefer and why (If-Else statement or shorter direct return statement)?
18 comments:
I'm with you, I hate typing so prefer the short forms. Also when quickly looking through the code I hate looking through redundant code. It slows me down.
I don't think there is room for argument here, using an if/else to produce a boolean value is silly.
Readable programs are programs that are easy to be read by programmers, not by English speakers.
"return a.equals(b)" is even pronounceable in English as "return whether a equals b", perhaps that makes it easier to adopt by your colleagues.
if a.equals(b) return true else return false adds a few more potential problem and requires unnecessary time to understand the construct.
Is it
if a.equals(b) return false else return true
or
if a.equals(b) return true else return false
And the reader has to silently think of all possibilities and why you have chosen this one.
My conclusion: use return a.equals(b)
Concise is better.
I grew up in the school of a single exit point. Multiple returns are just harder for me to follow. I'm with you Andy.
Ok, using if/else for computing a boolean value and single exit points are unrelated issues, and I believe this post is about the former only.
I also came from a school were single exit points was a hard rule (Pascal's exit was forbidden). But I outgrew that quite a while ago when I transitioned to OOP and learned to live by the guard clause pattern.
+1. Writing too much stuff because it 'looks like english' than something that's readable, still concise does not quite make sense to me.
Programs are written for programmers, not folks with doctorates in english literature.
Also multiple return statements are a bit confusing.
The next step would be:
if (a) {
b = true;
} else {
b = false;
}
I guess.
We all have our little quirks I suppose. Personally I prefer to compare to false instead of using the not(!) operator to negate a boolean. Simply on the basis that I think it is more readable.
The problem I have with the long version is that in addition to being overly verbose it has multiple exit points. I find that usually methods with multiple exit points are better off refactored in a form that results in only one exit point at the bottom. This makes the program flow easier to understand. Especially in longer methods having multiple returns is a definite code smell in my view.
So in this case, your version is definitely preferred.
I'd prefer the return a.equals(b); form. It's easier to follow, once you get used to it. OTOH the if-else is more verbose and a lot of students would prefer it here, because it's more explicit.
I'd prefer the return a.equals(b); too.
I don't think there are any quick translation inside.
You can read it in a very simple way: "return what it return".
If he wants English, do comment / javadoc.
FWIW, IntelliJ IDEA and (if I remember it right) eclipse do warn about the if-else construct in order to optimize it. So the a.equals(b) is definitely more preferable.
I think this is one of those places where it just isn't worth the argument. Some times one feels right and some times the other one does. 'feel' is a matter of personal preference, and this can change over time, even in reference to the same original context.
For reading and writing I go with the ternary operators when I can.
However, when trying to step through code or set a breakpoint on the 'false' return, the multi-line approach works better.
Since your colleague already expressed his inability to read it in an easy manner. I would just type the extra lines. Software Development is a team effort after all.
You just saved some branch mispredictions that could happen in your colleague's code, going from a cpu architecture point of view. Your approach is recommended. :)
Post a Comment