Mid-Function Returns are STILL Controversial
Long introductions are bad for reader retention.
So this is going to be rough, because anything that has aroused so much controversy needs some introducing.
I will state my own opinion upfront: returning from a function before its end is a bad practice.
A typical function or method performs a series of parameter checks or setup operations such as allocating a buffer, opening a file or network connection, each of which must be undone before exiting. These pseudocode examples are based on a C-like language, while much of the code shown here would be handled now by destructors and garbage collection. But it was looking at code like this that led me to abandoning early returns.
- open a file
- allocate a buffer
- seek to a file offset
- read from the file into the buffer
Yes it’s a contrived example but it’s illustrative of the core points. I chose C because operations are visible and it is still in very wide use, especially among those working with new chips.
Here the actual steps are separated by blank lines for clarity; most developers will not do this, the code will all be crammed together as though whitespace enlarges the binary.
But even with this rare courtesy the reader has to jump over ever-larger blocks of error handling code. And yes a scroll to the end shows the final operation, what we came to do, but even that is followed by yet another block of cleanup code.
In the early return layout most of the code visible in one screen is error-handling. To follow the path of correct execution the reader must skip over increasingly larger blocks of error-path code and locating the line that does the method’s nominal purpose takes some work. Each setup condition needs cleanup calls in each error handling block.
There is one call to fopen() and four calls to fclose(); one call to malloc() and three calls to free(). Without the blank lines between the operations it would be close to unreadable.
It is hard to imagine how anyone could prefer this. Mix this with exception throwing and you have code that cannot even be meaningfully analyzed.
In the one-return layout the order is reversed and a successful setup operation enters a new scope, with the accompanying cleanup operation invoked only once at the end of that scope.
In the one-return case the money shot is on line 17. In the first example line 17 is barely at the beginning, an error check for the buffer allocation. From the opening of the function to the extraction of the critical value all code follows the path of error-free execution, and all error handling is in the else blocks. The debugger will advance from one line to the next and only jump on an error.
There is one call to fopen() and one call to fclose(). There is one call to malloc() and one call to free(). When an operation succeeds you enter a new scope so the innermost scope is always the critical operation, and any operation that needs to be undone is at the close of its scope as there is no way to not pass through it.
The Great Objection to single-return structure is always the indentation. As many times as I have heard this nobody has ever been able to articulate what is wrong with that; “it’s just bad, that’s all. I don’t like it.” Well, sorry, but there is no substance to that. I wrote single-return code on 80-column CGA monitors and the indentation rarely went off the screen and when it did it usually meant my functions needed to be broken apart. Now my monitor has significant acreage and this is in no way an issue.
People will say “it’s hard to follow,” when it really isn’t, and never saying why
But Wait, There’s More
This is more than just a function architecture, it is a different way of thinking. Please consider this carefully, this is important. When I realized I was doing things backwards with mid-function returns my bug count dropped very sharply immediately. I found that it changed not just how I laid out my work but how I thought about it
“Just Get Out”
I’d seen comments like this hundreds of times
if (malloc(cb) == NULL)
// could not allocate so JUST GET OUT
This is a reflection of a “bail out at first excuse” kind of thinking, driven by anxiety more than intelligence. It’s positively eerie how common this “just get out” phrase is in comments over early returns. I just did something that could fail, stomach churns, quick! do the error handling. (Whew) OK now I can continue.
I am not advocating anything different except to do it in an else block instead of inline with the primary operation of the function.
Follow the Path of Goodness, Not the Path of Failure
Always check for errors but write in a “positive-going” manner in which successful operation is the presumption and errors are the exceptions. This is easier to read because the upper part is always the path of successful execution instead of separating the actual steps with error-checking.
It’s more than how many return statements a function has.
Edit: this is the more important point; a function should show its path of expected execution, not separated into islands of code by oceans of error handling. No MFRs naturally falls out of that.
Many responses talk about functions that are too long or do too many things; this is tangential and it’s a problem regardless of the layout and logic. Others act on the unexamined presumption that “nesting” is really bad and desperately seek ways to keep everything flush left. I like seeing scopes clearly laid out and indentation doesn’t bother me. Same reason I use BOL curlies instead of EOL.