Hacker News Reader

Comment by mega_dean | Hacker News Reader

After that step, they say "The resulting code is shorter and has no nested logic." The resulting code has the same logic as before, it's just not visually represented as being nested. I've seen the same argument ("nesting is bad so indentation is a code smell") used to say that it's better to use early returns and omit the `else` block, eg:

    if (some_condition) {
      // do stuff here
      return;
    }
    // do other stuff here

is "better" than:

    if (some_condition) {
      // do stuff here
    } else {
      // do other stuff here
    }

If you have very-deeply nested code then it usually becomes easier to work with after splitting it up into smaller pieces. But IMO rewriting code like this to save a single level of indentation is bikeshedding.

View on HN

I would say (as all good technical people know) it depends.

I have come to appreciate the style of early returns rather than else statements as I have found over the years it generally makes the code easier for me to follow when I’m looking at it possibly years later.

It really depends on the particular condition, but sometimes it just reads better to me to not use the else, and this is because as a style I tend to try have “fail conditions” cause an early return with a success being at the end of the method. But again there are regularly exceptions where trying to do this “just because” would contort the code, so returning an early success result happens often enough.

I have however found that sometimes ReSharper’s “avoid nesting” suggestion (particularly in examples like yours) results in less clear code, but it’s almost always at least not worse and maybe slightly better for the sake of consistency.

EDIT: Having thought about this more, here is why I find early returns generally easier to read than else statements.

With an early return the code is generally more linear to read as when I get to the end of the if block I can instantly see there is nothing else of relevance in the method, I save myself having to needlessly scan for the end of the else block, or even worse, past more code blocks only to find that the rest of the method’s code is irrelevant.

Again, not a hard rule, but a consistent style in a code base also generally makes it easier to read.

It definitely depends, but personally I find early returns to be a bit of an antipattern IF they're based on business logic. If a function has lots of ifs, you can glance at the nesting to see which ones affect the line you want to edit, and ignore the others. But if the function has lots of returns, you have to check every one before a given line in order to know what constraints are true at that point.

OTOH early returns are great for anything the type checker knows about:

    function foo(msg: 'OK' | 'ERR') {
        // ...
        if (msg === 'ERR') return someValue
        // ...
    }

Doing that is hugely cleaner than branching, and there's no added complexity to the developer since tooling can easily tell you what values `msg` can have at any given point in the function.

> you can glance at the nesting to see which ones affect the line you want to edit, and ignore the others.

Only if all the cases return! Only then is it obvious that you have independent cases. E.g. suppose we have three Boolean inputs x, y, z and want to do something for each binary combination:

  if (x) {
    if (y) {
      if (z) {
         return 7;
      } else {
         return 6;
      }
    } else { // x && !y
      if (z) {
         return 5;
      } else {
         return 4;
      }
    }
  } else { // !x
    if (y) { // !x && y
      if (z) {
         return 3;
      } else {
         return 2;
      }
    } else { // !x && !y
      if (z) {
         return 1;
      } else {
         return 0;
      }
    }    
  }

How would this look with early returns? One obvious way:

  if (x && y && z)
    return 7;
  if (x && y && !z)
    return 6;
  if (x && !y && z)
    return 5;
  // ... etc

(Let's ignore that we can just calculate the output with some bit twiddling; that's not the point).

Early return can be very clear, if we can bear repeatedly testing some conditions.

> Only if all the cases return!

My comment was about using if blocks as opposed to early returns. I.e. where the nested ifs run exhaustively and return afterwards.

Also, obviously deep nested ifs aren't good, so I wasn't advocating them - I just think it's better to fix them by splitting functions or simplifying control flow, than by adding early returns.

So it is more about multiple returns, versus setting some local return variable and returning in one place.

However, setting that return variable can be recognized as a simulated return. If we know that after "ret = 42", there are no other state changes; that the whole if/else mess will drop out to the end where there is a "return ret", then we can just read it as "return 42".

Sure, in the narrow case where the function only calculates a single return value and has no side effects.

Or where it produces an effect (or effect group) in every case just before returning, without multiple effects interspersed among multiple condition tests.

That's isomorphic to what I said, so... also yes :D

Calling it an anti pattern (as opposed to a subjective preference) is in my opinion super dangerous and reeks of cargo-culting as it implies an active avoidance of it which can result in deep nesting or contorted and hard to read logic.

There is no hard rule about early returns being always good or always bad, it depends on the particular situation.

> > It definitely depends, but personally I find..

> ..(as opposed to a subjective preference) is in my opinion super dangerous and reeks of cargo-culting..

o_O

I call them guard conditions and it’s my preferred pattern too. Deal with all the exceptional cases at the top and then move on to the happy path as a linear flow.

And at the start of the happy path, I tend to put "Do it" as a comment (a convention I learned from reading Emacs Lisp code).

Early returns are easy to read because although return is a staunchly imperative construct (a form of "go to"), early returns are structured such that they simulate a multi-case conditional from a functional language.

You know that each early return completely handles its respective case; if that branch is taken, that's it; the function has ended. There is only way to reach the code past the if/return, which is that the condition has to fail.

The conditionals inside a function that has a single return are harder to read, because no conditional is necessarily final.

if/elses that all return can be readable:

  if (this) {
    if (that) {
      return x;
    } else {
      return y;
    }
  } else {
    return z;
  }

still, it can be flattened:

  if (!this)
    return z;
  if (that)
    return x;
  return y;

It's shorter and less nested, and so that's a readability improvement. It's not as easy to see that x is returned when both this and that hold. The intermediate version below helps with that:

  if (this) {
    if (that)
      return x;
    return y;
  }

  return z;

If the conditions are cheaply tested variables, or simple expressions easily optimized by the compiler, there is also:

  if (this && that)
    return x;
  if (this)
    return y;
  return z;

I usually find the opposite (personally). Get rid of all the exceptional cases and error handling up front like you have in the first example and then spend the remaining body of the function doing the main work of that function.

It’s not so much indentation that’s an issue, but coupling control flow with errors and exceptions.

Swift does a nice job with `guard` statements that basically bake this in at the language level - a condition succeeds or you must return or throw.

If that control flow is part of business logic, I don’t think there’s any issue with your second example. That’s what it’s there for.

Reply on news.ycombinator.com