I've been reviewing java pull requests for a while now, and I often encounter code like this:
String getBar(Foo foo) {
String bar = "Unknown"; // default value
if (foo.getBarHandler() != null) {
bar = foo.getBarHandler().getBar(); // probable mutation
}
return bar; // single return
}
Of course this is pseudo code, the point here is that we instantiate a "default" bar
, maybe we mutate it, and then we return it.
I think mutability here is a bit evil, and I always propose do refactor it this way:
String getBar(Foo foo) {
if (foo.getBarHandler() == null) { // invert the condition to act like a guard clause
return "Unknown"; // default value is indented
}
return foo.getBarHandler().getBar(); // happy path the least indented
}
The drawback of this solution is that we have now two return
and the execution flow of this method might be more confusing.
I am seeking opinions here, what do you think is the best solution, and why ?
Just my humble opinion here: both methods are O(1) and in terms of performance doesn't change anything.
it's just a matter of "clean code" apparently. Personally I prefer the second version even if multi-return is often discouraged. But here we are talking about a 3 line code, should not be an issue here.
this method could be even 1 line:
String getBar(Foo foo) {
return foo.getBarHandler() == null ? "Unknown" : foo.getBarHandler().getBar();
}
I suggest here to don't "look for the evil" but instead focus on the functional aspect