javaalgorithmsoftware-quality

Java best practice: favor immutability or cleaner execution flow?


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 ?


Solution

  • 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