Hacker News Reader

Comment by thomas_moon | Hacker News Reader

The early return pattern was the most effective single piece of advice I received from a senior dev on how to make my code more readable. You end up with clearer code paths than any other pattern I have seen so far and way less indentation making things look (and probably perceived) as less complex.

Pair it with naming your complex and chained expressions and suddenly you have some seriously readable code.

So far, I have never seen a valid scenario where a switch statement is actually any better than if.

View on HN

I think the early return pattern makes the most sense when it's less "logic" and more "validation." Eg in the example, the code isn't really _doing_ anything if there's no user or no first name, it's running a bunch of checks _before_ trying to do anything. Those validation checks are implemented with if/early return, but the actual pattern is more "validate first and only proceed if everything checks out."

In typed languages a lot of those checks get implemented in your actual types, but you still might have various business logic/data integrity checks you might implement in early return.

Seen in this light, this pattern's really not so much in tension with the idea of having a single return variable. It's just a way to implement the idea that invalid states should not be possible, which you accomplish in your type system when and if possible, and fall back to runtime checks for the gaps where it's not.

Early return can get into trouble if you're talking about a function that has a fair bit of state to unwind in the event of an error. Open filehandles, allocated memory, general cleanup stuff.

That said, I much prefer early return whenever it makes sense. In functions that do have a lot to unwind and many possible points of failure I'll pull out the old villain 'goto' and have the unwind code at the bottom of the function.

Strictly sticking to only one approach is usually a mistake. One that is repeated a lot in computer science. There are schools of thought that if everything is the same it will be easier to understand, but what happens is problems that don't exactly fit the mold end up being solved in awkward and inefficient ways. Or development gets slowed because you have to refactor your problem around the tools instead of the other way around.

> Early return can get into trouble if you're talking about a function that has a fair bit of state to unwind in the event of an error. Open filehandles, allocated memory, general cleanup stuff.

This is where Lisp's unwind-protect, finally blocks in some languages, defer in Go, and C++'s RAII pattern can come in handy. Especially if you have a healthy respect for goto and want to minimize its presence in your code for various reasons.

`finally` introduces additional blocks, so if you have 5 different resources that you acquire as your function goes along, you end up with 5 levels of curly braces. Not very elegant.

`defer`, on the other hand, is probably the only thing I wanted to take from Go to other languages I worked with. Beautifully explicit and wonderfully useful.

C#’s new syntax for `using` also helps:

Old:

    using (var file = OpenFile())
    {
        // use file
    }

New:

    using var file = OpenFile();
    // file will be disposed when it goes out of scope

> `finally` introduces additional blocks

It would be cool if someone made a way to scope variables at the function level in JavaScript instead of at the block level. I might write a transpiler for it…

I've never considered writing a proposal for a language feature before, but after 2 minutes of googling, this actually looks completely doable:

https://github.com/tc39/ecma262/blob/master/CONTRIBUTING.md

So, impostor syndrome aside, why not? Wanna team up on that?

whoosh.gif

Okay, it was 2am here and I must have misread your comment and lost the 'variables' part.

It is already possible: var makes a variable function-scoped, let/const makes a variable block-scoped. Or are you referring to something else?

I don't know of an equivalent in the native API, but Bluebird's "disposer" implements this in the context of promises: http://bluebirdjs.com/docs/api/disposer.html

It would be nice if there were an equivalent in the native API. You can use .finally() with chained promises, but as far as I know there's nothing comparable with async/await yet, and that's a much more comfortable syntax with which to work with promises overall.

Unpopular opinion: I think early-returns make code less readable if you don't put the rest of the function in an else clause. You lose visual parallelism, and suddenly instead of following a tree down to a series of leaves where every leaf terminates, you have to have the full context to know whether or not a line of code may not execute in some cases. For example:

  function foo(x) {
    if (x == null) {
      return null;
    }

    x += 2;
    return x;
  }

I can't just look at "x += 2;" and know whether or not it's conditionalized. I have to have the full context including the early-return, and then reason about the control flow from there. Whereas:

  function foo(x) {
    if (x == null) {
      return null;
    } else {
      x += 2;
      return x;
    }
  }

Here I can tell just from the else-block that this is one possibility which will execute if the other one does not, and vice-versa. Their indentation is the same, cementing their relationship. If I want to know whether this block will execute I need only look at the if()'s, not their contents.

On the contrary, if early returns allow you to outdent a larger portion of code then it's quite a readability win. As long as all the early return cases are bite-sized and the last case long-ish, it's a readbility win.

And certain languages allow:

return nil if n.nil?

I really miss postfix conditionals from perl/ruby/coffeescript in all of the other languages. (Along with until/unless, which are just while! and if!.)

I asked on the go-nuts mailing list about perhaps including them in go2, but it seems many of the people who replied hate them.

I think the bigger problem you may be experiencing, if I had to guess, code structure.

If people tend to write:

   function foo(x) {
     if (x == null) {
       return null;
     }

     // do some stuff to the code


     for(a reason) {
        // do some more stuff
        if(some detailed reason) {
          return null;
     }

     // more things
     return 7;
   }

Then the shape of the code and the "pattern" of early returns gets broken.

If you read code where all of the short-circuit, early-return logic is at the start of the function, and then all of the work is done, do you still have this opinion?

E.g.

   function foo(x) {
      if(x == 1) { return null; }
      if(x == 2) { return null; }
      if(x > 7) { return null; }

      // do things with x
      return 7;
   }
?

The problem is definitely much worse when returns are scattered throughout the function instead of grouped at the top, but I still prefer them to be "elsed" together (or in a switch/cond/match statement or whatever). Worth noting that in Lisp - one of the classical languages where early-returning is popular - there isn't a problem because it is done as I describe:

  (cond
    (early-return-condition early-value)
    (second-early-condition early-value2)
    ...
    (t final-else-value))

The "default case" is on the same footing as the other cases. Only in procedural languages does the control-flow get confusing if you aren't careful.

If you want elses everywhere, how do you protect yourself from nesting hell? I find nesting hell to be far, far, faaaar worse for readability than any of the alternatives, and will move to the early return model to escape all that nesting.

I dunno, I just rarely ever find myself nesting more than two or three levels of conditionals even in extreme cases. If my logic really is that complicated, it's probably a sign it needs to be broken up anyway. And if I did need that much branching, I would want to be more explicit about it, not less.

Also - at the very least - you can mimick the "flat" early-return style and just stick some else's in there and call it a day. That's my main point; less so the actual deeper nesting

I agree, but also I think this is misrepresenting the 'early return' pattern. It's more likely to be succinct and look like this instead:

    function check(x) {
        if (!test1 (x))   return false;
        if (!test2 (x))   return false;
        if (!test3 (x))   return false;
        if (!test4 (x))   return false;
        return true;
    }

In _this_ scenario, it is true that this is much more readable than:

    function check(x) {
        if (!test1 (x)) {
           return false; 
        } else if (!test2 (x)) {
           return false;
        } else if (!test3 (x)) {
           return false;
        } else if (!test4 (x)) {
           return false;
        } else {
           return true;
        }
    }

Worth noting that elses don't require curly braces, which are where most of the visual noise is coming from in your example:

  function check(x) {
    if (!test1 (x))      return false; 
    else if (!test2 (x)) return false;
    else if (!test3 (x)) return false;
    else if (!test4 (x)) return false;
    else                 return true;
  }

I think this is more clear than either of the above

> Worth noting that elses don't require curly braces

True. I still think the first is clearer though. Either all tests pass, or they don't.

(also I'd be very skeptical of such an if..else ladder. Prime target for bugs.)

Never ever use if without curly braces. That's one of the worst ideas of C's syntax.

Your first example would still look clean like this:

  function check(x) {
      if (!test1 (x)) { return false; }
      if (!test2 (x)) { return false; }
      if (!test3 (x)) { return false; }
      if (!test4 (x)) { return false; }
      return true;
  }

And it would be much safer from stupid copy/paste errors.

Instead of

    function check(x) {
        if (!test1 (x))   return false;
        if (!test2 (x))   return false;
        if (!test3 (x))   return false;
        if (!test4 (x))   return false;
        return true;
    }

why wouldn't you do something along the lines of

    (every (lambda (f) (funcall f x)) (list #'test1 #'test2 #'test3 #'test4a))

or whatever is the equivalent in your preferred language?

The tests are rarely that simple or regular.

Isn't that exactly the reason to put them somewhere else?

No, I just mean the actual tests might be something like:

    if (user == null) return false;
    if (IsCompleted(user)) return true;
    if (action == null) return false;
    if (value < 0 || value > 100) return false;

You're not usually just passing a single value to a bunch of "test" functions.

Well, the way it was written, I was assuming that conditions to be satisfied had separate named predicates (although combinators could alleviate even some of these things).

That code is ugly anyway, something I would have wrote in middle school... why not:

    function check(x) {
        return test1(x) && test2(x) && test3(x) && test4(x);
    }

Or, since I assume is JavaScript, just:

    const check = x => test1(x) && test2(x) && test3(x) && test4(x);

Nobody would actually write this, it's just a demonstration of the pattern at hand for the sake of discussion

I think your version isn't really as different as it seems to some. To me, the essential bit of early return is that you handle cases that eliminate the work first. That is, it's less about singular if statements separating the rest of the body of the function than it is clearly short circuiting the code as early as is clearly possible.

That is, I don't think it matters too much whether you do:

    function div(x,y) {
      if (y == null) {
        return null;
      }
      if (y == 0) {
        return null;
      }
    
      return x/y;
    }

    function div(x,y) {
      if (y == null) {
        return null;
      } else if (y == 0) {
        return null;
      } else {
        return x/y;
      }
    }

as they both accomplish the same major benefit of the pattern.

The real thing it's helping to avoid is accidentally creating something like this:

    function div(x,y) {
      if (y != null) {
        if (y != 0) {
          return x/y;
        } else {
          return null;
        }
      } else {
        return null;
      }
    }

which can quickly get very hard to reason about without in more complex cases.

You're right that both technically qualify as early-return, which is why I qualified my original statement with "if you don't put the rest of the function in an else clause".

In practice, in C-like languages, I tend to see the "else-less" version which is why I brought it up

If you haven't seen it, Kevlin Henney's talk about "The Forgotten Art of Structured Programming"[1] is interesting (if somewhat long in the examples later), and somewhat related. It's not specifically about return early (and a case could be made that it should or should not apply specifically to early return), but it is about if, else, return, and how they affect cognitive load when reading code.

1: https://youtu.be/SFv8Wm2HdNM

For a single condition it doesn't matter much either way for readability. But once you have a number of conditions the if/else style becomes unwieldy.

I put all my early returns at the top as these are the preconditions for the rest of the code; I don't think that's hard to understand.

I have the same opinion. I like the early-return pattern when it's doing input validation or raising errors but for most other things I prefer how the if-else highlights that there are two possible paths that the code can take. Just because the "then" branch has a return statement doesn't mean that we should get rid of the "else".

Nit, the x is a copy so it won't change the value of x outside the function. So I would rewrite ad

const foo = x => x !== null ? x + 2 : null

Sadly, a lot of junior and medior programmers see that as a wrong pattern, and opt for a return variable.. Drives me nuts.

They should really use both

I recall this pattern being taught in my first year or two of my computer science degree. Emphasizing only one return statement per function, utilizing a shared return variable.

I much preferred (and still greatly prefer in industry) the pattern of exiting the function as soon as possible.

Same here.

Returning and throwing quickly and strict.

It's like "goto considered harmless" -- structured code to max, to the point of unreadability. Dogmatism has drawbacks.

In C when you have nothing that needs cleanup, it's not the most horrible thing ever:

  int foo(thing_t \*work) {
    int ret = E_SUCCESS;

    if(work->thing == BAD_THING) {
      ret = E_BAD;
      goto exit;
    }
  
    do_some(work);
  
    if(work->thing2 == OTHER_BAD_THING) {
      LOG("whoopsies");
      ret = E_OTHER;
      goto exit;
    }
  
    ret = do_more(work);
    if (!ret) {
      LOG("lazy");
      goto exit;
    }
  
    ret = last_bit_of(work);
  
  exit:
    return ret;
  }

I find the safe way to program that is always to set the initial value to some error. You only set it to success at the point you succeed. Then you never have to worry about some odd goto or other break in the control glow being introduced by you or someone else that wasn't paying attention. It's also better self documentation, because then it's obvious at what point you're actually done, in the case your final work isn't aptly named "last_bit_of_work(work)", because you'll see the return variable set to E_SUCCESS immediately afterwards.

agreed, but some people dislike that because you have to have an E_UKNOWN or E_GENERAL or something like that in addition to more specific errors and others dislike it because now there's not something to conveniently store results from called functions in so that they can be checked for errors.

I see that as a sign that the person learned how to program in the 80s/90s when things were still in a phase of overreaction to the popularity of BASIC and GOTO. GOTO is bad, therefore return is good, and single return is best. Not actually good logic, but that was the thinking at the time.

Unwanted conditions should be handled before handling the process in a peaceful condition.

Not sure it makes much sense to indent everything within "if" and if you forget to "else", you've just potentially hidden a bug.

I haven't written else if in javascript in years, and else is a very very rare staple in my code. Return pattern was an eye opener from a senior dev.

Reply on news.ycombinator.com