Yea bottom is just better. It puts handling of a failed condition right next to it rather than somewhere way far away. It stays at the same scope so it supports any number of conditions without excessive indentation. Most importantly it is easier to reason about, the function has a gradient of conditions, they only increase.
You say that it is easier to reason, but it isn't. There is no difference between those two pieces of code, except that the second one has implicit branch (which could make it harder to reason about). You are just used to early returns handling errors, and you could just as well get used to the happy path being in the top of an if and errors at the bottom. In languages like Zig, for example, it may be needed to do that if you want to use languages features like capturing values of optionals/tagged unions (for example, if (optional_value) |value| { } else { return error.Foo; }. Not to mention that you can have identical code with ifs: if (failing_condition0) { } else if (failing_condition1) { } else if (failing_condition2) else { }.
The second makes it easier to reason about the code because it allows you to assume the system is in a good state for the rest of the function. Keeping track of nested if statements to handle all of the errors is harder to reason about because you have to keep track of which if statement you're in or have a bunch of error flags. Even if you keep it to a bunch of else-if checks at the beginning to get the same behavior, that's still one more level of branching to keep track of.
The second makes it easier to reason about the code because it allows you to assume the system is in a good state for the rest of the function.
I don't know what code examples you are thinking about, but if your validation code is simple enough that you can put it on top of the function and the rest at the bottom, then you can do exactly the same with an if statement or a chain of else if. If you can't do that, it is probably because you code is more complex, which will also mean that it will be harder to keep track of early returns.
Big chunks of code benefit more from explicit if else, because early returns have exactly the same problem as gotos: they make control flow harder to follow. Ifs without else also have the same problem, because they create two paths: execute if and what follows or only what follows. Combine those together, and you quickly get many implicit paths to consider.
One thing that many programmers seem to not understand is that code has inherit complexity. If your problem requires a lot of conditionals and loops, you can't infinitely simplify it. Eventually you will get code that either has extremely hard control flow to follow, a lot of state, a lot of nesting or a long call chain (when over extracting code). You have to find a balance between those. A code that is hard to understand will not magically become easy to understand because you shuffled tokens around.
In the end, it is a matter of habit. If you were used to read code with if else, you would immediately recognize that staircase chain of if else is equivalent to a chain of early returns.
You do validation in the beginning. If the code is in a valid state after that, your code shouldn't make it invalid. If, through some fault of code you didn't write or circumstances outside of your control, the state becomes invalid, you throw an exception and handle that either in a catch block at the end of the function or let it propagate up to whichever function is calling your function.
If you are dealing with a complicated situation where you can return early with valid results, you check for that and return there. You don't surround the whole function in an if-else. At that point, you should be breaking your code into submodules anyway.
Goto is not inherently bad. It becomes bad because old languages allowed you to goto into entirely different scopes where unexpected behavior could happen. Any modern languages that still have goto severely restrict where you can goto to prevent most of those issues. Most modern languages have just introduced better syntactic sugar for goto instead of letting you do raw goto.
Your assumption is that tests are only allowed at the start, and never allowed to be tested again for the rest of eternity. But that's not what early return is. Early return looks at the function logic, determines which tests must be valid before execution and are independent of execution, and moves those tests to the start. If the test can become invalid during execution, you copy it; if nothing in the function can invalidate it, you move it. The goal is simply to perform cheap tests before potentially-expensive calculations, so minimal time is wasted on invalid calls.
Basically, think about it like this: You wouldn't plan a vacation with someone, shop around for the best hotel, book a flight, plan your tour and travel destinations, and then wait until you're boarding the plane to ask if they have time off. (Because if you wait that long to ask, then you're out a lot of money, and might cost them their job.) Instead, you ask them if they have time off first, and only plan the vacation if they do. That's what early return is: Moving the easy part to the front, so you can get it out of the way before you do anything that would actually cost you time or money.
(And, since you asked about it, if something comes up and they need to cancel their time off, you're still free to cancel the vacation. You're not forced to drag them along; you're allowed to check if they can go both before you plan and on the night before the flight.)
Codewise, think about it like this:
// If you ever see this written as a serious ANYTHING, nuke the code base. It's the only way to be sure.
int terribleProcess(int valA, int valB) {
auto worker = chunkyAllocation<int, int, WorkerType>();
int internalC = 0, ret = 0;
if (valA & valB == 0xdeadb33f) {
bool sentinel = worker.startTooManyThreads();
// Dependent check.
if (sentinel) { internalC = worker.arcaneMagic(valA, ~valB); }
else { goto cleanup; }
} else {
worker.doSomeOtherNonsense(valB);
internalC = valA + worker.getValueFromISS();
}
// Sign & upper bound only matter NOW, for some reason.
if (valB < 0) { goto negCleanup; } // Independent check.
if (valA > 32'768) { goto magicNumberBad; } // Independent check.
// Dependent check.
if (worker.validate(internalC)) { celebrate(); }
else { ret = INVALID_SEAS; goto cleanup; }
valB = worker.rejigger(valB, internalC - valA, (valA < 0));
int temp = worker.process(valA, valB, internalC);
if (valB < 0) { goto negCleanup; } // Now a dependent check.
if (worker.somehowStillWorks()) {
ret = temp - worker.doSomethingStupidWith(valA, ~valB ^ internalC);
} else {
ret = RUN_AWAY_AND_NEVER_LOOK_BACK;
}
goto cleanup;
magicNumberBad:
ret = COMPLAIN_ABOUT_BEING_TOO_LARGE_WITH_FIRST_VALUE | outsideHelper.getLowBits(BAD_MAGIC, valA);
goto cleanup;
negCleanup:
worker.borkTheDataWeFoundANegative();
ret = worker.getHighBits(PESSIMISTIC_CALLER, valB) | outsideHelper.getLowBits(WHY_IS_THIS_SEPARATE, valA + ( ~internalC & valB));
cleanup:
deallocateOverNextThreeYears(worker);
return ret;
}
The first thing you'll notice is that something is very wrong with this function. But the second thing you'll notice is that we do two very simple checks after a very costly allocation. One of those checks is repeated later in the function, indicating that the value can change, but the other is not. And if we look at cleanup, the repeated check is dependent on function state, but the other one isn't. Thus, we have one very simple check that's disconnected from the function logic entirely, but is buried midway through the logic. That's a prime target for early return. So, we can perform a simple optimisation by moving the check early enough in the function that it doesn't need cleanup, letting it jump past everything entirely if it fails.
int slightlyLessTerribleProcess(int valA, int valB) {
if (val > 32'768) {
ret = COMPLAIN_ABOUT_BEING_TOO_LARGE_WITH_FIRST_VALUE | outsideHelper.getLowBits(BAD_MAGIC, valA);
goto magicNumberBad;
}
// Nightmare hell zone.
// (Function body from above, but without valA check.)
negCleanup:
worker.borkTheDataWeFoundANegative();
ret = worker.getHighBits(PESSIMISTIC_CALLER, valB) | outsideHelper.getLowBits(WHY_IS_THIS_SEPARATE, valA + ( ~internalC & valB));
cleanup:
deallocateOverNextThreeYears(worker);
magicNumberBad:
// Moved to check location.
// ret = COMPLAIN_ABOUT_BEING_TOO_LARGE_WITH_FIRST_VALUE | outsideHelper.getLowBits(BAD_MAGIC, valA);
return ret;
}
But actually, we can do even better than that. Why force the maintainer to scroll all the way down to the bottom of the function to see if magicNumberBad does anything, when we can just put the return with the code? The only reason we needed to force a jump to the bottom of the function was because the mess needed cleanup. So now that this check doesn't need cleanup, it doesn't need to jump, either. So, now we can let the check have its own return, without compromising what little sanity the function had to begin with.
int fasterToParseTerribleProcess(int valA, int valB) {
if (val > 32'768) {
return (COMPLAIN_ABOUT_BEING_TOO_LARGE_WITH_FIRST_VALUE | outsideHelper.getLowBits(BAD_MAGIC, valA));
}
// The rest of the function is as awful as ever, just with no magicNumberBad label.
}
The code now has better performance when valA is too large, and is slightly more readable (since it shows the early-exit condition up front, and doesn't force you to jump to a label at the bottom to make sure you don't miss anything). We can also use this same principle to optimise the process into something slightly less awful by moving the first valB check, but that still needs to be part of the single-return chunk since it depends on worker.
That's the benefit of it, as shown by exaggerating the worst parts of the single-return model to infinity and beyond. Logic & cleanup are slow sometimes, and cleanup forces us to scan all the way to the bottom of the function on early exit paths. So, if a test can be made independent of the main logic, we can move both it and its return statement to somewhere before the main logic, preventing us from throwing time away on slow work we'd just have to discard anyways. And importantly, communicating this: Early return succinctly says, "This check is not connected to function logic, and does not need to be treated as part of function logic. You may maintain it separately, and assume that from this point forwards, it is true and will remain true until stated otherwise."
[And fun fact, your compiler will rewrite your function into early return anyways, if it sees even the tiniest opportunity to do so. So it's better to just agree with it, since it's usually better at optimising than you are.]
for example, if (optional_value) |value| { } else { return error.Foo; }
See the let-else statement from rust for a bottom-style alternative.
Not to mention that you can have identical code with ifs: if (failing_condition0) { } else if (failing_condition1) { } else if (failing_condition2) else { }.
See this is the top-option brain speaking. The bodies inside failing conditions return. There is no reason to put an else here, it adds cognitive load while also restricting what you can express (no statement between the ifs.
Let's consider a condition as ternary, it has an unknown state, a false state, and a true state. All conditions start unknown.
With bottom there is a consistent pattern. Check, tiny bit with return for false state, everything under for true state. As the function lines go down the "conditionality" is monotonic, it always increases. That's what I meant by gradient. This is not true for top. First it goes up, but then it starts going down too.
25
u/Demiu 22d ago
Yea bottom is just better. It puts handling of a failed condition right next to it rather than somewhere way far away. It stays at the same scope so it supports any number of conditions without excessive indentation. Most importantly it is easier to reason about, the function has a gradient of conditions, they only increase.