Ditch That Else | Hacker News Reader

Ditch That Else

preslav.me

58 pointsthunderbong44 comments

I remember in my early days of programming one of the most errant pieces of advice I received was that every method should have one and only one return statement. Before I was told that, I had always cleanly handled edge cases at the top of the method like this article suggests, then handled the bulk of the logic.

I was young and impressionable so when a senior developer advised me of this nugget of "best practice" I spent the next couple of years writing awful code, jumping through hoops to mangle the logic in my methods such that they would only ever have a single return statement at the end.

Thank goodness I eventually awoke from that terrible practice. I still occasionally hear people espouse this, but not too often nowadays.

I think the "errant advice" might originate with languages like C, in which an early return is an easy way to forget crucial deinitialization code, and where "goto"s can always be used to skip to the single exit point where all the deinitialization code is. In more sane languages, deinitialization happens automatically when leaving the stack frame, so following a "one return per function" rule is not necessary.

I had a similar early experience. Amusingly, what broke me out of it was coming back to imperative languages after a good stint with Clojure. My initial instinct, coming back to an imperative language, was to try to return a single expression. You can do that, but it makes for rather unidiomatic code.

Hewing more towards the idiom while trying to force functions towards a single return resulted in… a lot of statements, producing a lot of intermediate state. And it all gets much harder to follow.

I’ve since embraced early edge cases and early returns, largely because it allows more idiomatic use of a functional style in an imperative context. And all of that makes the code a great deal easier to understand than it would be otherwise.

This is surprising to me.

As a self taught hobbyist (crap) programmer that doesn't try too hard to make 'good' code. It nevertheless seems obvious to me that you'd want to deal with the errors first. I kind of disagree with the articles Pareto principle. 80% of programming is working out all the ways that things could go wrong, so it's better to think about those first, rather than getting into to juicy bits and leave the error handling for 'later'

Maybe it was from my ASM phase, and wanting to avoid the extra branch?

Or maybe it's because 'professional' programming is more cargo culting than we'd like to admit?

https://www.youtube.com/watch?v=I0Pt9Lf9JzQ

That's a reaction to deep if-else's - and they can happen in the happy path, independently of error handling.

I don't recall this requiring much code mangling though?

It means defining the return variable at entry and set its value to the "failed" value (eg NULL or false) and return it once at the bottom.

So you only touch the return on the happy path which reduces the number of error related else's, can reduce the need for guards too, it's consistent and you don't get caught out by early returns.

I still use this when it suits the application.

This piece of advice (some claim) originates in FORTRAN [0], where apparently a function could be entered in multiple places and return to multiple places. Most other languages don't support these features in the first place, so the rule is nonsensical (because it's impossible to violate) and got misinterpreted.

[0] https://softwareengineering.stackexchange.com/a/118793

Oh yeah, I got that advice, too, in the early 00s. I thought it was quite strange then.

I was frustrated with a memory leak in someone else’s code of a dozen or more if-if-if-if-else-else-else statements like the second example, it was probably 400 or more columns wide. It was my first real job and I began to doubt my abilities, I couldn’t follow the logic.

I refactored it all as the author suggests, out of desperation to better understand it in a debugger if more code could fit on a page. To my astonishment the memory leak disappeared, so it has always since been a habit, sometimes I still rewrite code this way just to better understand it, I think it looks better.

Funny, before reading the article was thinking about the eslint rule - believe it's called the "no-else-return". While reading and before finishing the article was thinking "isn't that the reason why we use guard clauses?".

It can be confusing but I really appreciate one-liner `unless` statements in ruby for guards

edit: not a fan of unless/else though

no-else-return is correct https://eslint.org/docs/latest/rules/no-else-return

This rule is actually what got me into the practice of returning early from the edge case so many years back. I think it was enabled by default in the—then very popular—AirBnB coding standard.

> It can be confusing but I really appreciate one-liner `unless` statements in ruby for guards

Gosh I am working on beating that out of my Ruby folks. Sure I came into the team from other languages and they're all Ruby natives, and sure this is accepted practice in the Ruby community. But it's just awful and I won't accept it.

What? You've inherited a functioning team with a certain code culture, and you're establishing your dominance by making them conform to your "superior" standards? Are they still writing Ruby or will you make them switch to a different language as well?

I agree. It might be hard to adjust one's practices, like when one first starts living in another community with a different culture, but it's the right thing to do in most cases.

"Functioning" is arguable. The codebase is a mess.

If the codebase is actually a mess, you should focus on the actual mess rather than Ruby idioms that are uncomfortable to you.

…said every newly installed manager ever.

I've been there 3 years.

And you are still fighting basic Ruby language idioms from the perspective of a newcomer rather than dealing with actual problems?

And your codebase is still a mess you say? Huh.

Maybe try adding some `unless` statements, I've heard they really help.

It's a 14 year old code base mate. Have you ever worked on very large codebases?

Sounds like the regular beatings haven’t improved morale.

Do you make them replace `each` iterators with `for` loops as well, just because that’s “how it’s done” in your preferred language?

The “fail early” principle is a widely accepted pattern, and that’s exactly what the `unless` statement facilitates, while communicating that is exactly what it’s doing.

Early returns are very useful, just like break/continue. For example, I find it much easier to write

    for filename in os.listdir(dirname):
        if not filename.endswith(".png") and not filename.endswith(".jpg"):
            continue
        # process a png or jpg image

than to try and have a nested `if filename.endswith(".png") or filename.endswith(".jpg")`, particularly as it drops one nesting level and is easier to read. Unfortunately, Python lacks targeted break/continue so you can't break out of two nested loops without some contortions; in such cases, I find it useful to refactor code into smaller functions just to be able to use early return from a nested context.

the early return solution to this would be to filter os.listdir before the loop

It does looks simpler but unlocks several footguns: order of edge cases,edge cases deeply nested within other condition with different context, early return that doesn't reset some mutex/setting/return condition.

Before blindly rewriting, have a mental picture of what is context/state at the point of early return, which at each time has to achieve the goals of function, setting background states/data points/return pointers/etc. Its easy only in simple functions.

Any good modern language has ways of ensuring that resources are properly released on early returns: RAII, using/with, defer, etc.

This is my preferred style by far, but it seems like most junior programmers I work with don't seem to mind the style with deeply nested ifs and far away exceptional elses. I try to avoid commenting on such stuff in code reviews unless it is egregious, but I will rework it later if I happen to be in the same code. I hope it doesn't come across as passive-aggressive on my part.

It probably comes across as passive aggressive

It's also robbing them of a learning opportunity, and it's creating more work for you.

Mention it on the PR, maybe show an example of how you would write it in a code snippet, and re-review after they've taken a stab at it.

you should mention it. When I was a junior programmer I also wrote the nested ifs and I had a mentor show me how much cleaner the code could look which I really appreciated. I continue to use this style which is far superior than the one I started with.

This is why having your language support nested procedures is so helpful.

Having it completely separate when it's only ever going to be used from one place isn't exactly the clearest and means extra variable passing. Pulling it into a nested procedure, though, makes it very clear what's going on and much easier to follow. My general rule is no flow control other than bailout in anything more than a dozen lines or so.

Rust embraces this philosophy with the ? operator (when you just want to early return an error) or the let else construct (for more involved short-circuiting). If you type the data you're dealing with properly, this is probably enough for the majority of cases and the resulting code is super clean.

Sometimes someone HAS to take the role of Captain Obvious and write articles like this :)

This is good advice - I wish there was another article talking about when your error handling isn't as simple as just hitting one path of an if-else. But I guess that would be a specific article on error handling.

This is also being called a “never-nester”. As in, you never nest your own code more than three levels deep.

There is one very well-made YouTube video on it, IIRC.

The guard statement in swift is pretty much designed for this pattern, and it works well imo.

I sincerely hope that most people are not dealing with functions of that length, such that error handling is lost at the bottom of hundreds of lines of code.

Whether you do return-early or nested if-else, there's little reason for having such lengthy functions. Dealing with THAT should be the priority, not the nesting vs return-early.

Your logic has to flow somewhere. I prefer shorter functions too, but sometimes the actual “unit” is just not so concise without breaking it up in confusing ways. Handling edge cases first makes that much more digestible, IME.

Forcefully breaking up your logic in places where it doesn’t make sense is an antipattern IMO, and makes it very easy to write spaghetti.

I’ve inherited codebases with this overemphasis on very short functions and what I saw were very specific utility functions in dozens of different modules, all over the codebase, often with funky names, surrounded by dead code, and never a clear path to and from it. Sometimes a single function that spans even a couple of hundred lines, is preferable.

I learned early on to avoid nesting conditionals. I assumed this was standard practice.

Universities teach the very opposite, unfortunately...

You put the condition that is most likely to be met first, not the least likely.

Lol why? You are coding definitive rules not heuristics. The odd of the branch getting hit has no bearing here.

When the edge cases get bloated, it’s time to make a validator function for your input and leave the main function with happy path only

LoL! languages that support short-circuit evaluation will exit the conditional evaluation as soon as they reach a condition that is true, without evaluating subsequent conditions

... it also makes branch predictions more accurate

Reply on news.ycombinator.com