There are ways to do early returns in C without duplicating resource releasing code though, using gotos (it's one of the few cases where gotos make sense, IMHO). Code looks like this:
int aFunctionWhichMightFail() {
int result = FAIL;
Foo* foo = allocateSomething();
if (!someFunction(foo)) {
goto fail;
}
moreWork(foo);
result = SUCCESS;
fail:
releaseFoo(foo);
return result;
}
The Linux kernel makes heavy use of this.Gotos like that are fine. Despite the title of the paper I don't think Djikstra's meaning was that gotos should be removed entirely.
Like, imagine the spaghetti lovecraftian horror of a non trivial program that's just if and goto. At the time, there was a particularly vocal camp that essentially made the argument that it shouldn't matter since if/goto is equivalent to what exists in the machine code anyway, and you can technically do all the same things.
These things are hard to notice, and even harder to find elegant and effective solutions!
The point is to reduce the problem space so all you are left to look at, when you do come back to your code, is what's important to reason about without removing important details or adding trivialities. These problems haven't gone away just because the literal word goto isn't used anymore.
Such as?
I am aware of writing this in different ways, of architecting the logic quite differently. But the only control-flow construct I've seen as alternatives are to abuse exceptions or labelled while with named breaks.
What were you thinking of when you wrote this? I'm genuinely interested.
You can however give named return values and reference those names, and I believe it will work.
I've not seen any other constructs in any language that would come close, and I still maintain `goto fail` is a reasonable construct.
I'm actually not sure offhand how multiple deferred statements are handled. I could envision that the specification might make no explicit guarantee about the runtime order or it might create and pop off of a stack (at least in behavior).
Foo foo = acquireFoo()
defer releaseFoo(foo)
if !isValid(foo) {
return FAIL
}
moreWork(foo)
return SUCCESS
That is impressively clear. I don't use go, myself.I agree, I am quite comfortable using goto in C/C++ in this context. Though it may be better in C++ to use RAII in this specific example.
(with-open-file (fhandle path)
(if (has-record fhandle)
(process-record (get-record fhandle))))
Python basically has this, but Python doesn't give you the macros needed to write your own for other kinds of resource.http://clhs.lisp.se/Body/m_w_open.htm
> If a new output file is being written, and control leaves abnormally, the file is aborted and the file system is left, so far as possible, as if the file had never been opened.
Shades of PCLSRing!
Python doesn't use macros, true. But it absolutely gives you the ability to do 'with' for other kinds of resource (and anything else). Look for the with-statement and documentation on defining your own context manager.
The ease of writing macros such as with-open-file means that most libraries that need to have similar types of features for their resources that needs releasing also provides similar functionality.
try {
doSomething();
}
catch (Exception e) {
handleException();
}
finally {
cleanUpResources();
}
When the code exits the try block, you are guaranteed that it will run the finally block, possibly entering the catch block first if an exception was thrown. Since the most common use of `goto fail` pattern is freeing memory, the `finally` block isn't actually used a lot in Java/C# code in practice. Foo foo = acquireFoo();
try {
if (!isValid(foo)) {
return FAIL;
}
moreWork(foo);
return SUCCESS;
} finally {
releaseFoo(foo);
}
Python could also use the same approach. Though it also has context managers / the with-statement for this specific application.That there is no high-level equivalent, and Djikstra, while he sometimes had useful insights, was ultimately a self-righteous twit.
Sorry to disappoint; I would also be interested a (not-even-worse-than-goto-like-exceptions-are) answer to your question.
[0] https://www.cs.utexas.edu/users/EWD/transcriptions/EWD02xx/E...
There's just too much cleanup, such that avoiding gotos means you're either nested 200 columns into the screen or have half your function filled with partially-duplicated cleanup code.
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attribute...
So we ended up with a Best Practice of one return clause per function. Which is triply stupid because 1) long functions are part of the problem, 2) break to the end with multiple assignments to the same variable is almost as bad. It’s just trading one stupid for another which is a waste of time and energy.
And then for many years a loud minority of us, some of whom actually have a vague understanding of how the human brain processes data, insisted on the pattern you describe here. Some of us call it “book ending”. All the returns are fast or proceed to the end. Any method that can’t accomplish either has too high a complexity anyway and gets split until you get either one return, or book ends.
[another responder calls them guard clauses, but this doesn’t quite apply in all situations. A clause that handles a default or a bad input is why you can rearrange, but it misse why you should: ergonomics]
At the same time, C does not have any features for reliably managing resources. No GC. No C++'s RAII. Instead, you have to be careful to release every resource you acquire in the body of a function, along every execution path.
In that world, avoiding multiple returns makes a lot of sense because it ensures every execution path reliably reaches the end of the function so it's easier to ensure resources are released.
We aren't in that world anymore, so the guideline no longer applies. But it wasn't a stupid rule. It's just unhelpful to take it out of its original context and expect it to apply to a new one.
"Check your boots for scorpions before putting them on" isn't a dumb rule in the desert, but it is in Antarctica.
Makes no sense to turn your code inside out for resource management when you picked a language where you don't have to turn your code inside out for resource management.
I got taught this matter of style in the context of 90s C/C++. The general idea was that many return points likely means many point of having clean up any state you've been trying to manage/encapsulate with the function... in particular the state of manually managed memory. Which probably means mistakes.
It's a reasonable argument for manually managed memory, loses much of its power once you're not. And these days, most of us are not.
To me the second version makes it more difficult to differentiate the structure:
if (...) {
foo()
return ...
}
return bar()
from: if (...) {
foo()
}
return bar()
I find that when I'm reading code I'm generally more interested in the overall structure than in the specific implementation details. Especially since if the first block was complicated enough to tax my cognitive overhead, then I would probably extract it into a function anyways. function foo(bar) {
if is_null(bar) {
return TypeError
}
if !validate_input(bar) {
return ValueError
}
if cached(bar) {
return cache(bar)
}
// Added as a workaround for client XYZCorp since...
if (bar == "A special value) {
return "A special result"
}
return do_a_complex_thing_to(bar)
}
You could equivalently structure it as a bunch of else ifs, but there's no real reason to do it that way. if cache.contains(key) {
return cache.get(key)
} else {
v = db.query("..")
expiry = calcexpiry(v)
cache.put(k, v, expiry)
return v;
}I also would like to add that it helps to fail early.
The article shows a null check for an add function. But the question is: why not throw an exception when the input is not what you expect.
This helps to find bugs very early.
return dbcache.getOrCompute(key, function() {
v = db.query("..")
expiry = calcexpiry(v)
return (v, expiry)
});I see these early returns as a list of assumptions. It helps making them nicely lined up, with perhaps some comments before each, and an empty line between them.
And then when you get below to the, now non-nested, code that actually does some work you have a smaller mental burden needed to understand what is actually done.
if !cache.contains(key) {
v = db.query("..")
expiry = calcexpiry(v)
cache.put(k, v, expiry)
}
return cache.get(k)
Only one return, and everything is still in the logical order.In your example above, the branch does merge back, and clearly that's fine for a small function - and common pattern - like this. But the general idea would be to avoid it if possible, because that removes unnecessary complexity.
One small problem is that depending on how your threading and cache works it might be possible for the cache to be cleared after the contains call but before the get.
That’s inefficient and, worse, introduces a concurrency bug. The cached value may expire between the check using ‘contains’ and the call to ‘get’.
A good API would have something akin to C#’s TryGet (https://msdn.microsoft.com/en-us/library/bb347013(v=vs.110)....).
plus this arrangement would seem vulnerable to a pathological case where you found the V from the db, but have a full/failed/partitioned cache and end up faulting or otherwise not returning the V.
instead, make the branch be the least complex option and exit early, and skip the else. That way, it can be read as "if this condition, do this tiny logic and exit", and that branch can then be mentally pruned.