On: Avoid Else, Return Early (2013)

[viach]

"removing a whole line and more braces" - this is really fighting the wrong enemy. Code should be written in a way it is more readable, not shorter.

[moomin → viach]

I think the point OP’s trying to make is that reducing indentation levels makes code more readable.

I don’t agree with him about removing the braces, though. That way madness lies.

[thomasahle → moomin]

This particular refactor (changing `if (err) { return ... }` to `if (err) return ...`) didn't actually reduce indentation levels though. It only reduced the number of lines.

[moomin → thomasahle]

Yeah, I’ll file that under “maybe don’t do that”.

[timoxley → moomin]

what are the downsides

[moomin → timoxley]

In general terms, in C-like languages, mixing braces and semi-colons is horribly refactor unfriendly and bug prone. Meanwhile, sticking things on one line introduces a cognitive load that you might not want. (YMMV on the second point, but I’ve pretty strong views on the first.)

[fen4o → viach]

Exactly. This led to the famous OSX double goto bug [0]

  if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;
  ... other checks ...
  fail:
    ... buffer frees (cleanups) ...
    return err;

[0] https://www.dwheeler.com/essays/apple-goto-fail.html

[SideburnsOfDoom → fen4o]

The "Avoid Else, Return Early" blog post above appears to be about JavasScript code, which (along with Java, C#, Python, Ruby etc) do not ready do "buffer frees (cleanups)" much. They have other language and runtime mechanisms for this such as GC and try...finally blocks, which work fine with early return.

Yes, in the presence of teardown code at the end of the method, as is common idiom in C, avoid early return. This appears to be the case for apple's "goto fail" code.

However, don't generalise this to all languages.

[_pfxa → fen4o]

I don't really understand why the use of goto is to blame there. If it was not a double goto but a double return, that'd still lead to this bug, no?

[DougBTX → fen4o]

The goto fail snippet doesn't conform to the "one logical statement per line" guideline in the article though, so the author would advise against this style too.

[dep_b → fen4o]

`goto` wouldn't be used in a return early philosophy.

[wildmusings → dep_b]

Goto is heavily used to return early, by jumping to the cleanup section at the bottom of the function. Otherwise, you have to repeat the cleanup code at each exit point, or use the dreaded pyramid of doom where each failure point introduces another indent level, which is what the author here is trying to avoid in the first place.

[dep_b → wildmusings]

Not a C-programmer at all but I would separate out the resource creation/access from acting on the resource. So Open X, Do Y on X, Close X would have do Y on X in a separate function and possibly Open X and Close X as well. This way Close X would be executed regardless of the result of Do Y on X.

Only by doing multiple things in one function you really need to use goto's in C. (again, as far I can judge coding in C, not an expert!)

[SideburnsOfDoom → wildmusings]

"cleanup section at the bottom of the function" is pretty rare in JavaScript, which is what the original blog post is discussing. Same with Java, C#, Ruby etc.

No, it doesn't play well with early return, so avoid mixing them.

[vidarh → SideburnsOfDoom]

Actually, in Ruby we have "ensure"

    def foo arg
      return true if arg == 42
      puts "got past the guard"
      raise "blah"
    ensure
      puts "ensure always"
    end
    
    foo(42)
    foo(3)

The "ensure" blocks gets executed whether or not you return early, throw exception or return at the end of the block.

[SideburnsOfDoom → vidarh]

There are several constructs for it, see also "try ... finally" on C# and Java, and "using" statements in C#.

All make the "goto manual cleanup at end" less necessary, and make early return easier to use.

[wildmusings → SideburnsOfDoom]

I should have specified that I was talking about C. You usually don’t need this pattern when you have garbage collection or C++ RAII.

[fsloth → viach]

"Code should be written in a way it is more readable, not shorter."

Personally I think sometimes shorter code, that gets rid of unnecessary bureaucracy, is more readable. For example, C++ recently (in the past decade...) gained range based iteration.

Previously:

for(containe_type::iterator i = my_container.begin(); i != my_container.end(); i++){ do_stuff(i); }

Now:

for(auto& i : my_container) { do_stuff(i); }

This is really context sensitive. Some code benefits from verbosity while other don't.

[bjourne → viach]

Ceteris paribus, shorter code always is more readable. It's the only guideline I've found true regardless of programming language or environment. Shorter is better.

[timoxley → bjourne]

It depends. Short, dense code can make the code more difficult to understand/change later, while overly bloated code can have the same effect.

I often find inelegant, yet straightforward solutions are generally better options than dense, mathematically pure solutions, simply because inelegant code usually relies on fewer assumptions. Noting that code rarely/never evolves the way we expect it to, apply Occam's razor and strike a balance.

[bjourne → timoxley]

No it doesn't depends. That's what ceteris paribus means.

[GVIrish → bjourne]

Shorter code may or may not make things more readable. If brevity was the key to readability, we'd all be minifying all code we write to make it more readable.

Readability is about clarity of intent and function, as well as sensible structure. Sometimes that means writing slightly more code with more expressive naming so that the code is easier to follow. Sometimes that means using a given language's shortcuts to condense things down.