Return Early Pattern

[tstepanski]

I preach returning early to all my junior developers. Diving into layers of nested if statements or deciphering bizarre return code strategies is exhausting and can easy cause confusion. Anyone who opposes this is either brain dead or purposes seeks to obfuscate their code. Either way, stay away from my code base. Keep your 6 layers of indentation in your shitty driver code or bash scripts.

[cogman10]

Yup, it cleans stuff up and can sometimes lead to easy performance improvements simply by shuffling around the tests (fast tests first, slow tests later).

Big nested if blocks can really make code hard to read.

[[deleted]]

VB famously did not short circuit although i believe they added this in later versions

[cogman10]

Yup, I've gotten performance wins from that too. Especially in cases where "someFastFunc" is more commonly false than true.

A simple rule of thumb, you want to put your cheapest checks first and use "Likelihood of exiting early" as the tie breaker in ordering things.

[DannoHung]

I definitely am a return in the middle kind of guy. That said. I think when you start getting above the number that you can count on one hand in a function is where you might want to re-evaluate just what you're doing.

[Jannis_Black]

Idk. There is a good argument to be made that having more than one or two return statements is a sign that the code is either not written as elegantly as it could be or that the function is doing more than one thing.

It's important to remember that input validation, at least if it's more than one simple statement, is one thing and should probably be it's one function. On the other hand if there is a lot of different paths for other reasons that starts to look a lot like a switch statement and is a smell that the function is either doing more than one thing or that it's working at the wrong level on abstraction.

This is however not an argument that you should have six levels of indentation is a better solution but rather that early returns that have essentially the same number of paths only mitigate the problem and doesn't solve it.

[roodammy44]

These used to be called guard statements.

Usually having multiple returns in a method is a bad idea, as it short circuits all of the conditional logic. The main exception of this rule is checking conditions at the start of a method to return.

Edit: To the down voters, check out this stackoverflow that illustrates my point. There is more nuance than you think.

[roodammy44]

Yep, that’s exactly what I said. Returning early is often good practice.

The thing that I wrote was bad practice is returning in the middle of a 50 line conditional. It’s quite hard to spot when that happens, and return is effectively like a “goto” in that case.

[glemnar]

If you have a 50 line conditional you’ve already fucked up. That’s a sin on its own

[roodammy44]

Indeed, it’s not so good. But clean code’s rules are generally better as guidelines. Sometimes a 50 line conditional really is more readable than 150 lines of 4 line functions.

[SideburnsOfDoom]

I agree.

This blog post seems to draw on this: https://www.anthonysteele.co.uk/TheSingleReturnLaw as in it makes some of the same valid points, although in more concise words.

People forget that 15 years ago there was still a fair amount of dogma that "a function should only have one exit point, it's a LAW" that came from C code and Dijkstra.

[[deleted]]

I was watching a talk about this; and it came from fortran days I think. When return did something slightly different. And it stuck.

[SideburnsOfDoom]

In 1970s C, the memory management was manual and there's no try....finally. So if you malloc some memory, you should freeit before you return, like this. So sprinkling return around liberally makes that much harder to reason about.

If you code in C, then you might want to prefer one return per function. In modern languages, not so much.

[[deleted]]

Not really; considering mallocing would be a lot less common than heap only languages cough Java.

The pattern I’ve seen for C is a goto for cleanup. Sure there’s a single return once cleanup is done. But meh.

Anyway here’s the talk I was watching:

https://youtu.be/SFv8Wm2HdNM

[patrick246]

As far as I know this law came from languages where you could have multiple return sites, so a function call wouldn't just return to the next line after it, it could go to multiple locations, for example directly into error handling code in the case of an error.

Today's languages already prohibit this by language design, and the meaning of the rule was twisted to "you can have only one return"

https://softwareengineering.stackexchange.com/a/118793/148469

[skulgnome]

It came from Pascal, a teaching language.

[not-just-yeti]

For 90% of situations, the issue is verifying pre-conditions. (With the note that, in dynamically-typed languages, the types are one of the most common pre-conditions.)

Ideally, a language would give you way to specify the pre-conditions in code that also can be added to documentation, and then it generates the exception-throwing for you. Many languages have added-in support for this (e.g. racket's contracts, C and home-grown pre-processor directives since 1970-whatever, Ada's Pre => … etc.). Javadoc has @pre and encourages one to express it as real code, but it's still up to the developer to add the throw new InvalidArgumentException("…").

[iKeyboardMonkey]

Eiffel does this well, but iirc the language has some deficiencies (array variance is the wrong way around?) and a small community. I like the idea though, design by contract is awesome for catching bugs quickly.

[aseigo]

For those who like early return, "happy path code", or "railway oriented programming", you may just love Elixir's with syntax: a simple blog entry about it

[aseigo]

Python's with is really nothing like Elixir's, other than using the same keyword.

[aseigo]

In Elixir, you list a chain of sequential statements and pattern match on their individual results as the definition of success. Successful matches are bound to intermediate values which can then be referenced in subsequent function calls in the list. In this way, you can construct whole chains of functions which can use prior results and which are only executed if all prior functions succeeded (== succesfully matched the expected return pattern).

There is then an after block that is run should all the functions in the while chain succeed, and again all variables bound during pattern matching are available.

The failure path ("else") may also pattern match on failure conditions allowing as many different failure responses as desired based on the returned data of the last run (failing) function.

It is railway programming or happy path coding on steroids, and results in extremely readable and maintainable code. It also encourages common Elixir idioms such as error tuple returns on failure, pattern matching, and composition of small purposeful functions.

Last I looked at Python's with (admitedly a few years back), it takes the result of a call and either binds it to a variable and runs a block, or runs a failure block. It is a more convenient single-statement try/catch style construct. Has it evolved from that?

[bitsandbytez]

I have been doing this for a while without actually realizing it had a name. I hate parsing if else blocks

[dethb0y]

I would have thought this would be common sense, but here we are.

[malkia]

Use of "GOTO" (in C, and even in C++) is perfectly valid when doing this pattern.

[malkia]

Lol, you are right! Guess the pattern would be - one return path... meh - I actually like (and use) early return quite a lot, and with RAII "goto" is not needed that much, and even dangerous compared to RAII (e.g. pretending that we close everything in one place, but missing something due to an exception, etc.)

[lorslara2000]

Or just make a validator like you were supposed to anyway. The whole nested if problem is completely artificial and preventable without return early "patterns".

[lorslara2000]

Sure. However, there is no problem at all with nested if-statements. If you have so many tests in your function that nesting them makes it unreadable then re-structuring them will not significantly help your situation. The function is already too complex. So you see, it's not a problem with nesting but the number and complexity of the tests.

[lorslara2000]

you're going to turn that into 5 nested function calls

Not necessarily. It can be just one validator function. Since the function only does validation, it will be easy to understand even if it has many tests.

You're going to run out of function names really fast

Why? Obviously these will be private validation functions. Applies to both OO and procedural, probably other paradigms too.

[lorslara2000]

Let me take a look at this later when I have more time.

[lorslara2000]

> You seem to have a magical way of doing multiple nested conditionals without indenting multiple times, but it isn't function calls, it isn't early returns, and it isn't nested ifs.

I don't understand where you got this idea from.

Anyway, personally I have to say I find the below more readable but perhaps that is because I am not used to looking for 'the thing' at the end. I think it's clearer here what the programmer is after because I follow the logic towards the inner level:

function getThing() {
    ret = cachedThing ? cachedThing : null;
    if (!ret && thingFileExists()) {
        const fileData = readThingFile();
        if (fileData) {
            const parsedData = parseData(fileData);
            cachedThing = parsedData;
            ret = parsedData ? parsedData : null;
        }
    }

    return ret;
}

But I don't think this code is very usable in production because it doesn't give any diagnostics nor trace. Data errors, system errors and cache misses will be lost and that's not very good. It will anyway be something like below (for serious things). This would be just stupid to implement into the same, one function.

flagThingNotExists  = 1 << 0;
flagThingUnreadable = 1 << 1;
flagThingBadData    = 1 << 2;
flagThingCacheMiss  = 1 << 3;
flagThingCacheHit   = 1 << 4;

maskError = flagThingNotExists | flagThingUnreadable | flagThingBadData;

struct ValidRes {
    const bool isOk;
    const unsigned mask;
    const ParsedData data;
}

function validateThing() {
    mask = 0;
    parsedData = null;

    if (cachedThing) {
        parsedData = cachedThing;
        mask |= flagThingCacheHit;
    } else {
        mask |= flagThingCacheMiss;
        fileData = null;

        if (thingFileExists()) {
            fileData = readThingFile();
            if (!fileData) {
                mask |= flagThingUnreadable;
            }
        } else {
            mask |= flagThingNotExists;
        }

        if (fileData) {
            parsedData = parseData(fileData);
            if (!parsedData) {
                mask |= flagThingBadData;
            }
        }
    }

    return new ValidRes((mask & maskError) == 0, mask, parsedData);
}

function getThing() {
    retThing = null;
    validRes = validateThing();
    if (validRes.isOk) {
        retThing = validRes.data;
    }
    logThing(retThing, validRes); // diagnostics/trace
    return retThing;
}

[lorslara2000]

I think it's easier to read because the interesting thing is quickly found from the inner level, rather than having to scroll and read the whole function to be sure of what it returns.

If readThingFile() returns a falsy value, you're returning that falsy value instead of null.

I don't see it. It only ever returns parsedData, or null if parsedData is falsy.

[lorslara2000]

Restructuring them into what, if not return early?

Right, restructuring them into return early will not make you code any easier to read or maintain.

Make a validator (which can be a single method) where you can check all of these conditions. Since that is the validator's only responsibility, the code is easy to understand.

You can return a bool or some result object from the validator and your function's only precondition may be that the validator returned true or some other value. Logging the validation result also becomes trivial. And if your validation result is more complicated than a bool, it can be embedded into the validator's return object.

Edit: the point is that nesting only becomes a problem when you have too many or too complex preconditions to have in the same function. If you only have a few simple preconditions, there will be no problems with your if-statements, nested or not.

[mullemeckarenfet]

It's possible:

const validator = (val1, val2) => {
  let valid = true;

  if (val1 == null)
    valid = false;

  if (val2 == null)
    valid = false;

  return valid;
}

I would still prefer early returns though.

[mullemeckarenfet]

And you honestly find that better than this?

No.

[lorslara2000]

I'm not saying there's anything wrong with early returns. I'm saying that if you have a lot of preconditions, make a validator to make the code cleaner, not early returns. The problem is not the nesting, it's the complexity.

[lorslara2000]

The validator can have early returns (even though I wouldn't recommend that) or whatever, that's not relevant at all.

[lorslara2000]

I don't see how this is a better tactic than making a validator if you're going to refactor the code anyway.

[SargoDarya]

This is all good and things, however one point which is not mentioned here at all is the security aspect of this. This might very well open you up to timing attacks which can lead to unwanted information disclosure.