Separate returns in an if statement?


if (true) {
   return this;
   }
else {
    return that;
    }

I wrinkle my nose. I don’t know if it’s just structured programming dogma, or if there are good reasons.

I’m wondering about readability and the shared-codebase ethic, and also about bug prevention. It just doesn’t look clean to me.

What do you say? One return, or several?

GeePaw sez…

Neither “never do this” nor “never forget to do this” actually work as rules. Since you’re skeptical of the multiple-returns, I’ll start with an example of bad code directly resulting from single-exit strategies.

public void SingleReturns(arg,arg2,arg3)
{
	if(arg is okay) {
		if( arg2 is okay )
		{
			if( arg3 does not make the world blow up )
			{
				do some work
				write a (calculated) message about success
			}
			else
			{
				construct stupid message and print it
			}
		else
		{
			construct another stupid message and print *that*
		}
	else
	{
		for crying out loud, all this logging is killing me
	}
}
 

public void MultiReturns(arg,arg2,arg3)
{
	if(ArgIsHosed()) return;
	if(Arg2IsScrewedUp()) return;
	if(arg3IsDegenerateOrAtLeastTypedByADegenerate()) return;
	do some work
	write a (calculated) message
}

Notice I’ve combined to different patterns two make this solution:

  1. I’ve reversed the logic of the conditionals. In Eclipse, that’s a mechanical refactoring.
  2. I’ve put the code that handles failures inside the extracted conditionals.

I will say, tho, that I don’t have good names. To me it feels like the second one is missing some word or words that make the idiom clear. I’ve played around with names like “ArgOneReturn”, or whatnot. Nothing, yet, satisfies. The pattern of moving all the failure-handling to the conditional function, I call Extract-Conditional-And-Branch.

15 thoughts on “Separate returns in an if statement?

  1. nuttycom

    This is an unfortunately common idiom in Java. There are, however, at least two better ways to handle this sort of structure: either break out the cases into separate methods and use the ternary operator, or else call an abstract method and implement each case in a different subclass; this is cleaner but of course only really works if the condition being dispatched on can be evaluated at the time that the instance is created.

    I try to just avoid Java completely in favor of Scala or Clojure. But if you’re working with existing Java codebases, you’ll see this kind of crap all the time.

    Reply
  2. Sean McMillan

    As you know, I’m totally fine with that structure. I think there was a dogma of “one entry, one exit,” back when functions were chunks of assembler that you used via goto. “one exit” doesn’t provide anything useful, and “one entry” is provided by the language.

    @nuttycom has good advice when he suggests separating the implementation by subclass, and pernicious evil when he suggests using the ternary — ick! The use of ternaries strikes me as a much larger code smell than multiple returns.

    Reply
  3. Rich Dammkoehler

    I am very strongly a single return point person. My motivation for being this way was probably born in some structured dogma, but I’ve come to understand it as a readability and maintainability issue.

    Let me start with a caveat that I also encourage very short methods 5-10 lines where possible and a bare minimum of logic in each method (1-2 branch statements is enough) but I also recognize that these things aren’t always possible. Sometimes a method may go on for as many as 16-20 lines and have 3 or 4 branch statements.

    So, why then, the insistance on a single return point. Its about readability and maintainability.

    If I know that all methods always return at the end and never anyplace else, I always know where to look. I always know that any branch I might be looking at bubbles up to the top of the statement and control flows down to the end, where the single return point is. This means that no matter what I’m looking at I can always assume that if it returns something, I’ll find that return at the end of the method. The end result is I can turn that part of my thinking off and focus on something more important, like a conditional that I want to modify or understand. Or the nature of a loop. No wondering if someplace in the middle does this return null. I know it returns at the end. So for me, that’s the big reasoning behind single return point.

    I don’t have a good metaphor handy, but the reduction in thought-cycle time in managing and understanding the code really pays off.

    As an additional note I was just talking with an interview candidate on this very subject. He pointed out that the overall maintenance risk of multiple return points and the potential for resource management errors (unclosed connections etc.) is increased dramatically by allowing multiple return points. Those are good points that developers should be thinking about when they argue for the multiple return point position; it might persuade them to argue otherwise.

    Also, nuttycom’s suggestions are good ones. Eliminate the IF entirely and this becomes a non-issue.

    As a related topic there is the idea of throwing exceptions if pre-conditions to execution are not satisfied. As a concession to throwable things, I’m not overly critical of methods that place guards (clearly) at the top and throw exceptions; maybe I should be. An ideal remedy to this code-smell however is to have two methods, an outer method that checks the pre-conditions and throws the exception if they are not satisfied and an inner method that does not. By separating these bits of logic the code is cleaner and more refactorable. Unfortunately it does still leave you with what is effectively two return points from the code.

    Reply
  4. Sean McMillan


    // Only one return, so It's clear.
    function findWaldo() {
    var ret;

    ret = isInKitchen() ? "kitchen" : isInBedroom() ? "bedroom" : isInGarage ? "garage" : undefined;

    if(!ret) {
    //check the dom elements he could be hiding in
    for(var i = 0; i < 100; i++) {
    if (!ret) {
    var elem = "#hiding" + i.toString();
    if($(elem).hasWaldo()) {
    ret = elem;
    }
    }
    }
    }

    return ret ? ret : "couldn't find him";
    }

    Reply
  5. Sean McMillan


    // Multiple returns -- totally unreadable.
    function findWaldo() {
    var ret;

    if (isInKitchen()) return "kitchen";
    if (isInBedroom()) return "bedroom";
    if (isInGarage()) return "garage";

    //check the dom elements he could be hiding in
    for(var i = 0; i < 100; i++) {
    var elem = "#hiding" + i.toString();
    if($(elem).hasWaldo()) {
    return elem;
    }
    }

    return "couldn't find him";
    }

    Reply
  6. Rich Dammkoehler

    Sean, I take it that your trying to make a counter argument through a straw man? Before even considering the return points I’d object to the amount of logic in the method findWaldo() in either implementation. Further, the use of ternaries as part of a straw man makes my point about readability. If you reworked the code and eliminated the ternary branching the code would be considerably more readable.
    Too further my point. If I injected some code in either example between isInGarage() and the DOM search, I could find myself wondering why it never executes. The answer would be that Waldo is in my bedroom. But unless I really understood that I’d be chasing my tail to resolve the issue. The former structure (sans-ternaries) would reveal immediately why my code was not executing or more likely have prevented my gaff by the nature of its structure.

    Reply
  7. Sean McMillan

    I’ve seen code that bad, and worse. Now this example is certainly a storm of bad code in either case.

    Ignore the ternaries — let’s focus on the loop. If the return is always at the end of the function, you always complete the loop. You can’t short circuit the processing, and you have to have the loop/if structure.

    If there’s a better way to do it and still provide the single exit point, I’d love to learn it.

    Reply
  8. Sean McMillan

    Apparently I don’t know how to use gist. I added this variant, which I prefer to both of the above. It uses a lookup table, so all of the possible hiding spots use the same processing logic. It only has one return, but that’s an early return inside a loop.


    // Lookup Table
    var waldoChecks = {
    kitchen: isInKitchen,
    bedroom: isInBedroom,
    garage: isInGarage
    };

    //add the dom elements he could be hiding in
    for(var i = 0; i < 100; i++) {
    (function(i) {
    var elem = "#hiding" + i.toString();
    waldoChecks[elem] = function() {
    return $(elem).hasWaldo();
    };
    })(i)
    }

    function findWaldo() {
    for (var loc in waldoChecks) {
    if (waldoChecks[loc]()) {
    return loc;
    }
    }
    }

    Reply
  9. Peter Harkins

    Single exit point makes a lot of sense when you have to manually manage memory. When you have garbage collection, multiple returns let you have guard clauses without weird contortions and deep nesting.

    Reply
  10. Michael Avila

    My understanding is that the “single entry, single exit” rule had to do with making the program easier to reason about. But I would assume that would only be a problem with a method whose flow is so complicated that a premature exit would be detrimental to its understandability. IMHO when methods are small enough and have a need for “multiple exits” they are easier to understand with multiple returns as you don’t have to understand the whole flow of the method. My two cents.

    Reply
  11. Angela Post author

    @peter Avoiding contortions is one of my guiding principles in life, so I like this idea. But what are guard clauses?

    @michael I’m just about convinced that this is a non-issue. When I get used to writing compact, pretty methods, I’ll probably wonder how I could have ever worried about this. :)

    Reply
  12. Michael Avila

    http://c2.com/cgi/wiki?GuardClause

    That’s the technical answer, but I like to think that a Guard Clause protects the Normal Flow. Where the Normal Flow is the sequence of statements whose execution is not constrained by a conditional. I mean “protects” in the sense that a Guard Clause allows the statements of the Main Flow to exist on the Normal Flow. By Main Flow I mean the sequence of statements necessary for a method to achieve it’s Single Responsibility. I find this topic of flows very interesting when writing clean code.

    You should take a look at Kent Beck’s Implementation Patterns!

    Reply
  13. Angela Post author

    @geepaw has spoken! Yay! (If you’re on rss, that means the post has been updated, and you should read it again.)

    So, Mike, I think you are telling me that trying to follow rules instead of writing what is beautiful is going to get me in trouble, right?

    I’m a huge believer in following the love, and doing what seems best in the moment. I also want to learn how to have a better sniffer, though, so I ask these questions so maybe next time, for example, multiple returns won’t look ugly to me, because I know better.

    Thanks for the clarity!

    Reply

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>