1. 28
  1.  

  2. 32

    Well written article and an enjoyable read. Only part I disagree with is your stance on “early exit”, it turns out that this is the tiny hill i’m willing to die on, one I was unaware I cared about until now. I think this is primarily because if I read code that has a return within an if block then anything after that block is inferred else.

    I could become pedantic and retort that all control flow is a goto at the end of the day but I wont, because that would be silly and this was a genuinely good read, thank you for sharing.

    1. 17

      I also was surprised how much I disagreed about the early exit.

      When I originally learned programming, I was told that multiple returns were bad and you should restructure your code to only return once, at the end. After learning go (which has a strong culture of return early and use returns for error handling), I tend to favor early returns even in languages that don’t use returns for error handling.

      The thought process I’ve adopted is that any if/return pair is adding invariants, eg if I’m at this point in the program, these previous statements must be true (or they would have exited early). If you squint at it, you’re partway to pre/post-conditions like in Eiffel/Design By Contract.

      1. 3

        When I originally learned programming, I was told that multiple returns were bad and you should restructure your code to only return once, at the end.

        Ah, so functional programming </sarcasm>

        1. 2

          Pure functional programming is all about early returns, if anything. There’s just no return keyword. When everything is an expression, you can’t store now and return later.

          1. 1

            In a pure functional language the whole function is a single expression – I fsil to see how it is “all about early returns”? Certainly you can simulate imperitive return or raise using various tricks, but ultimately there is always just one expression and that is what gets returned, anything else is syntactic sugar.

            1. 3

              Conditionals and pattern matching are expressions. This means you’d have to put effort to avoid an early return.

              Consider a function that converts boolean values to string in the canonical structured style with a single return.

              function bool_to_string(x) {
                var res
                if(x) { res = "true" } else { res = "false" }
                return x
              }
              

              In a functional style it’s most naturally written like this:

              bool_to_string x = if x then "true" else "false"
              

              We could put an extra effort to store the return value of if x then "true" else "false" but it looks like obviously useless effort:

              bool_to_string x =
                let res = if x then "true" else "false" in
                res
              
        2. 3

          I had a similar experience, from “only one return” to “return early” And I think it depends on the domain and language you are using too.

          One project I worked on was initially written in C and then moved to C++ and started by people who mostly wrote java. There is a common pattern in C to use goto near the return statement to free memory when you exist (think of it as a defer in go but written by hand), and since goto‘s are the hallmark of bad programmers and returning early was not an option the dev’s came up with an ingenious pattern

          int result = -1;
          do {
            if (!condition) {
              break;
            }
            result = 1;
          } while (false)
          
          return result;
          

          it took a while to decipher why it was there but then become common place because your promotions were heavily influenced by your “coding capability”

        3.  

          Strongly agree with you.

          I first came upon early returns as a recommended style in Go, under the name of “line of sight” (the article makes a good case for its benefits), and have since become a huge advocate, and use it in other languages as well.

          Knowing all your if-indented code represents a “non-standard” case makes the code really easy to scan and interpret. Aside from leading to flatter code, it leads to semantically uniform code.

          1. 2

            I think early exit is OK in the sense of basically guards against invalid inputs and your language lacks the ability to express it in other says - you know, C. (Probably the same for freeing resources at the end, since you don’t have finally or defer.)

            1.  

              +1 on early exits. They’re clarifying: the first thing I do in many functions is check various corner cases and then early exit from them, freeing up my thought process for everything afterwards. Knowing that those corner cases no longer apply means I don’t have to worry that someone — and “someone” might even just be me, in the future — adds some code that doesn’t handle the corner cases well after the terminating } of the else block (or de-indent, or insert syntax here for closing an else block).

            2. 17

              In rule 2 he refactors this:

              if (a and not b) {
                    do this
                } else if (a) {
                    do that
                } 
              
                if (b) {
                    do the other thing
                }
              

              To this:

                if (a) {
                    if (not b) {
                        do this
                    } else {
                        do that
                        do the other thing
                    }
                } else {
                    do the other thing
                } 
              

              And I stop reading…

              If you have such logic then types and case analysis (i.e. pattern matching) is much clearer and easier to reason about. The fact that he makes a mistake just underlines this point. if is a very implicit way to state your assumptions.

              I appreciate people who handle all possible inputs and I think that is a case where early return is somewhat idiomatic, i.e. something like:

              (progn
                (when (!preconditions) (return error))
                (...logic))
              
              1. 20

                I would actually write that as:

                if (a and not b) {
                	do this
                	return
                }
                
                if (a) {
                	do that
                	return
                } 
                
                if (b) {
                	do the other thing
                }
                // Or, if "do the other thing" is a lot of code, "if (!b) return" and write it unindented.
                

                As I wrote a few weeks ago, I usually find it’s best to minimize the amount of things I need to think about when reading code, and the more I can read it from top-to-bottom the better.

                The “refactored” second example especially requires me to remember a lot of things in my short-term memory for no good reason IMO.


                Perhaps needless to say, I disagree with most of this article; the whole “return is like goto”-argument doesn’t seem convincing.

                That section mentions “we already established that goto is bad” but it actually didn’t really; it just asserted it, and if we remember that Dijkstra was talking about usage of goto in 1960s BASIC and FORTRAN, most of the arguments in that paper don’t really apply to modern usage of goto since it’s an entirely different context. This includes those usages mentioned in the Stack Overflow link where the author says that “some people are still defending the use of goto”. Absolutely no one “defends” the style of goto programming Dijkstra was talking about.

                1. 3

                  For anybody else who was confused and had to work through it, the two cases diverge when both are false.

                  He’s since fixed it. I find this a bit funny because you can also rewrite it this way:

                  if (b) {
                    do the other thing
                    if(a) {
                      do that
                    }
                  } else {
                    if (a) {
                      do this
                  }
                  

                  Which is in line with what the post is arguing for. It’s funny to me because it’s simpler than the new “fixed” version, but requires rethinking how a and b interact (instead of just applying rote rules.)

                  1.  

                    That’s still not right if the order of operations matters. You’d need to “do that” before “do the other thing”, though obviously this is an easy change.

                    1.  

                      Oop, wasn’t even thinking of order of operations! You’re right, this is incorrect too. Programing is haaaaaaard

                  2. 1

                    Care to elaborate, aren’t ifs that deal with a single variable equivalent (or at least similar) to pattern matching?

                    1.  

                      Pattern matching generally requires all cases to be handled and cuts the visual noise of a series of if/else blocks.

                      You can also match on more complicated expressions. Here’s one matching on a tuple.

                      (match (a b)
                        ((t t) (do that; do the other thing))
                        ((t f) (do this))
                        ((f t) (do the other thing))
                        ((f f) (do nothing)))
                      

                      Which I think is clearer and easier to read than any of the if/else approaches.

                      The table approach at the end can also be expressed nicely with a match.

                      (Match ((≤ age 18) pro (≤ subscribed_for 5))
                        ((t t t) ...
                      

                      Or you can match on (age, pro, subscribed_for) and do the less than comparisons in the cases.

                      1.  

                        It certainly would be clearer in certain cases, but I don’t think it that pattern matching can replace the if statement altogether.

                        1.  

                          It definitely can. Erlang had case for a long time without if at all; if was added later, as a convenience, but is still somewhat discouraged. Trivially, (case (pred) (‘true foo) (‘false bar)) is equivalent to an if.

                  3. 13

                    I can’t agree with the “early exit” argument. Additionally, the “solution” presented after the ‘early exit via throw’ example, adds a problem: you now essentially have two versions of the same function, one that has error handling, and one that doesn’t.

                    I will bet you real money, the variant without error handling will get called from somewhere else by someone in the future.

                    Furthermore, this argument is just factually wrong:

                    We all know that if statements should handle all possible cases

                    No. A single if statement should handle a single possible case.

                    Even an if + elseif series of statements don’t necessarily handle every possible case.

                    This is perfectly exemplified in one of the author’s earlier “real world” references:

                    “Bring me a coffee if you get home early.”

                    Is the author really suggesting that what’s meant here is “Bring me a coffee if you get home, otherwise don’t bring me a coffee”. Sometimes there is nothing else to do, if the condition isn’t met.

                    Not every conditional execution has a negated action required. This is evidenced by the need for a very specific example to prove the “need” for an else.

                    1. 12

                      Incredible to see such an opinionated (and flawed) article with that many upvotes.

                      This approach doesn’t works every single time and it just gives more ammunition to those annoying people who don’t understand that.

                      1. 9

                        My biggest pet peeve with these articles is that this self-assured (or, rather, pointlessly arrogant) style tends to trickle down to the audience. Five years from now someone is going to take this article very literally and – lacking the required experience – not realize that you can’t really apply it in every case. And their next code review exercise will devolve into something like “conflating multiple logical conditions in one statement is bad style” - “It may be bad style in general but I want to do this explicitly because this way the conditions in the implementation match the ones in the formal spec, so it’s easier to check compliance.” “False. Conflating multiple logical conditions in a single statement is messy and results in code that’s hard to debug.” and so on and so forth.

                        The whole article reads like narrow dogma, even in areas where there’s really no reason for that, such as the history section, which is kind of upside down. As far as I recall, the earliest description of the if-then-else formalism (e.g. McCarthy, Minsky) are both purely functional and make no reference to the idea of a “block”, which arguably precedes structured programming (early FORTRAN versions, for example, had subroutines and DO loops). Or that bit about GOTO which is not even wrong – nobody who is defending the use of GOTO today is doing it in the context Djikstra was talking about in his famous paper, so no, obviously they don’t “talk like that” (which isn’t just untrue, it’s really just a gratuitous jab).

                        It’s even more ironic when the author has to show up and correct his examples, after spending a few hundred words on preaching why you’re always wrong and he’s always right.

                        1.  

                          Five years from now someone is going to take this article very literally and – lacking the required experience – not realize that you can’t really apply it in every case.

                          I stopped using Pylint when I upgraded to a new version and my code started failing checks. The reason for it was that they’d added a new rule (that I didn’t agree with). I went and found the PR where someone added that rule, and the justification was simply “here’s a single random blog post that says don’t do X,” which (shockingly) got the rule added.

                        2. 4

                          This is what happens with dogmatic statements like the ones in this article. People flock to their favorite ideas, they might sound good (and invite discussion) and might even work in some situations, but in the end it’s down to taste and you can’t rigidly codify that.

                          Right now I’m working on a Clojure codebase that uses multimethods here and there. This is another way of doing conditionals, but it’s very very implicit. I still haven’t made up my mind whether that’s a good or a terrible idea. More likely it is just something you need to apply very carefully, like everything else in programming.

                        3. 8

                          You’ve got a few mistakes in your example code (the conversion of assembly to structured, the a and not b example, and I think at least one more). Because your argument is so focused on correctness and readability, I think the errors undermine your argument.

                          1. 1

                            Thanks, corrected.

                            1. 4

                              I think you should clarify that you fixed the code in the essay. “These examples were originally wrong and I had to correct them” is important information here.

                              1.  

                                Don’t understand exactly why, but OK, added a note.

                          2. 5

                            A lot of these examples work well when the code fits in ~20 lines. In almost all code I write for work, functions are 100s of lines easily, for ~reasons~. They start with error handling and bounds checks - which this article doesn’t address.

                            Once you’re up to 100’s of lines, I would get very pissed off if a coworker did not use early returns to rule out boring edge cases in the main function. The example sentence:

                            If you don’t get home early ignore everything I’d say after this sentence. Bring me a coffee.

                            Which seems weird, but instead if it was:

                            If don’t use our service, you should go back and read the front page! Anyways, <insert entire description of ToS>.

                            1. 4

                              “Stop me if you have heard this one…”

                            2. 9

                              Rule 1 - DO NOT use return for control-flow

                              Stopped reading here. Anyone who believes in saying words like “early exit” is not an influence I need in my life.

                              1. 4

                                Forgive me if I am wrong here, but my understanding of Dijkstra’s essay was that goto should not be used in situations where a function call could be used instead. The reason he gives is that it undermines the usefulness of the stack trace as function calls are traceable and gotos are not. This would not really apply to if statements. If statements do not influence the stack directly and therefore are not really any different from gotos where the stack is concerned.

                                This also applies to the seemingly most controversial part of the article, early exit. A function that completes is not traceable regardless of where or when it returns, once it returns it is popped off the stack and there is no record of whether it exited early or not.

                                I also wondered why the nested if was not refactored to:

                                if (b) {
                                  do the other thing
                                  if(a) {
                                    do that
                                  }
                                }
                                else if (a) {
                                  do this
                                }
                                
                                1. 2

                                  Goto considered harmful definitely hurts more now than it did then.

                                  Yes, use if statements. No, just because you CAN express it as goto doesn’t mean it’s bad.

                                2. 4

                                  Some of the examples provided in the text look like watered-down versions of real use cases which happen to benefit from the proposed refactorings. But, for instance, if one has to check more than one error condition, that will lead to either a bunch of cascading ifs or an unreasonable number of auxiliary functions. At that point, the readability benefits of keeping separate branches of the same if clause vanish quickly.

                                  1. 1

                                    Since the point of an example is to take a point across, this is probably true, but regarding the case with the multiple errors, I would be pretty happy with

                                    if (error1) {
                                       throw exception1
                                    } else if (error2) {
                                       throw exception2
                                    } else {
                                       doStuff()
                                    }
                                    

                                    (The rule for not conflating is only valid for “multiple conditions that are dependent on one another” )

                                    Otherwise, I would be happy to see some examples where the rules don’t work. After all, we know that every rule has exceptions.

                                    1. 1

                                      So, I was thinking in scenarios such as this one, which is a very common pattern in C. Every single function call can raise an error, and their results in case of success are feeded into the next call.

                                      I tried to follow your recommendations and it resulted in extremely unidiomatic C.

                                      In these cases, early returns—or even gotos jumping to the cleanup section—are much more readable.

                                      fd = open(path, O_RDONLY);
                                      if (fd < 0) {
                                        status = -1;
                                      } else if ((buf = malloc(sz)) == NULL) {
                                        status = -2;
                                      } else if ((n = read(fd, buf, sz)) < 0) {
                                        status = -3;
                                      } else {
                                        /* TODO consume `buf` and `n` */
                                        status = 0;
                                      }
                                      /* TODO cleanup */
                                      return status;
                                      
                                      1. 2

                                        I am not a C programmer, but for me this looks pretty readable.

                                  2. 3

                                    How is a function called signup_internal or _signup better than a early return right at the top? If you can’t give something a meaningful name, better avoid naming it!

                                    If this was focused more on types, you’d see how the error type is part of a “presentation layer” and probably uses a presentation-layer monad that the business logic shouldn’t need to care about ;)

                                    1. 2

                                      I think weaker versions of rules 1 and 3 are helpful under limited circumstances.

                                      I slightly like a much weaker version of the “no early returns” rule (1). I often prefer to have no early returns in a subroutine after the first point where it does something that has an externally visible effect. Before that point everything will be read only. (I guess this implies that in pure functional code, keeping flow control simple is less important.)

                                      The “always do something in else” rule (3) makes sense in event handlers for things like form submits or button clicks, when refraining from going ahead with the thing the user asked for because something was not available. I certainly do not want to log every branch anywhere that was not taken: the log output will be too spammy to read.

                                      The “do not encode date in if statements” rule (4) is bad. The “before” code is simpler and easier to read than the “after” code. The mini interpreter that had to be written to make use of the data table is more complicated & more likely to be buggy than the code it replaced. By all means put the amounts and bands in config, but otherwise that stuff is business rules, not data.

                                      we already have a mechanism to edit the rules without code changes

                                      This embodies soft coding. You can end up with a system where supposedly ordinary users can configure the system’s behaviour, but the configuration is code (for a weird machine with terrible debugging and IDE support) so ordinary users fear it, refuse to touch it and demand that programmers be the ones who update the configuration. At that point you might as well have written that code in a programming language which has nice tooling.

                                      The bit under “Data-driven error handling” is good, polite behaviour for user interfaces. Reporting errors one at a time in UIs is rude. I would not call it strongly desirable in APIs though. I’d rather have APIs with simple, less-likely-to-be-buggy code behind them then APIs that try to be extra polite about error handling and risk bugs by making it complicated.

                                      This feels very 1970s. I’m expecting the ghost of Fred Brooks to pop up and scowl at me.

                                      (This is likely an urban legend but I could swear I saw it accused somewhere that the real reason for “no early exits” was because some people were writing programs up as flow charts prior to entering them into the computer — punch cards cost money! — and their flow chart notation was missing a notation for early returns. Another more plausible supposed reason is that prehistoric variations of Hoare logic didn’t accommodate them in proofs, despite the fact that simple mechanical translations to other control flow forms exist.)

                                      1. 2

                                        I like most of this except the rule against early returns. Consider the suggested alternative:

                                        if (input less than waterleve) {
                                            turn motor on
                                        } else {
                                            turn motor off
                                        }
                                        

                                        When reading this code, you ask yourself “when can the motor be turned off”? To answer that, you need to reason about the control flow that can enable that statement to be reached. You have to look up at the if condition. When that condition is true, the then block is run. Then you negate the condition in your head and understand that the else can only be reached when the condition is false. That’s exactly the same mental processing you do for an early return.

                                        In particular, consider a more complex example using early returns:

                                        if (file does not exist) return file not found error;
                                        
                                        if (file is empty) return file empty error;
                                        
                                        if (file cannot be read) return read error;
                                        
                                        read file;
                                        

                                        The author argues that this is better written as:

                                        if (file does not exist) {
                                          return file not found error
                                        } else if (file is empty) {
                                          return file empty error
                                        } else if (file cannot be read) {
                                          return read error;
                                        } else {
                                          read file;
                                        }
                                        

                                        But, again, ask yourself, “When does the file get read?” The answer to that is “When the file exists, and it’s not empty, and it can be read.” Even though there are technically no “early returns”, the chain of else if clauses still means that you have to maintain all of those if conditions in your head when reasoning about the control flow. The underlying control flow is the same, as is the cognitive load.

                                        Also, for what it’s worth, I think Dijstra’s “Goto Considered Harmful” letter to be a weak, poorly-argued claim and Dijkstra’s own “guarded control flow” language is just as bad as goto according to his own logic.

                                        1. 1

                                          We have to remember all those conditions in both cases, but the second one makes it much more easier for us to to identify what they are and to separate them from the rest of our code.

                                          1.  

                                            the second one makes it much more easier for us to to identify what they are

                                            Do you have evidence to support that claim?

                                            1.  

                                              Well, in the first case, they (the conditions) may be scattered all over the function, while in the second one they are all contained in this one if-else clause. Consider an example where the read file; operation is 100 lines of code long - if an early return is permitted, then you would have to read all those 100 lines in order to ensure that they don’t contain a return statement, while if it’s not, then all of those would be in one big else block, and you would know they do not contain extra conditions.

                                        2. 2

                                          Thanks for this article. Even if I’m not sure I agree on these things, I like these articles because they try to address what is good programming, and it feels like there are not a lot of them. Currently there is a lot more talk about cool programming languages. I also enjoy those, however I’m starting to think that learning a lot of programming languages doesn’t necessarily turn you into a skilled programmer. It may expose you to new ideas but it doesn’t necessarily teach you anything. A bit like copying math formulas doesn’t necessarily teach you math.

                                          1. 1

                                            In typescript, an early return of an object literal lets it be typechecked properly, while a variable returning doesn’t seem to have the same effect. Might be related to specific versions.

                                            1. 1

                                              Short history of the if statement? Was McCarthy exagerating when he claimed to invent it in 1960?

                                              https://en.wikipedia.org/wiki/McCarthy_Formalism#McCarthy's_notion_of_conditional_expression

                                              1. 2

                                                Fortran (well, FORTRAN at the time, I guess) had conditional code execution. So it could jump from one place to another based on some Boolean expression. This was a mechanistic, imperative thing.

                                                McCarthy invented the conditional expression. It was a way in mathematics of defining recursive functions that didn’t have an infinite regress. It also just happened to turn out that you could take McCarthy’s math and make a computer implement it. It’s not clear to me how much innovation McCarthy’s conditional expression really is. It seems conceptually quite similar to piecewise functions, which I’m guessing are much older.

                                                1. 1

                                                  The if statement existed in FORTRAN, I think, but the blocks were a later idea. The first language that had blocks was ALGOL 60. And yes, McCarthny was involved.

                                                  http://www-formal.stanford.edu/jmc/recursive.pdf (see footnote 2).

                                                  https://craftofcoding.wordpress.com/2017/04/29/the-evolution-of-if-i-from-fortran-i-to-algol-60/

                                                2. 1

                                                  I couldn’t upvote this more (besides that I literally can’t). Also, this article has made finally understand the Elixir design decision of not having a return statement.

                                                  1. 3

                                                    Elixir has features which cover the early return use case mentioned by other comments.

                                                    For example, author’s example could be written as:

                                                    def signup(username) do
                                                      with :ok <- validate_username(username) do
                                                        # Work goes here
                                                      end
                                                    end
                                                    
                                                    defp validate_username(username) 
                                                      when is_binary(username) and length(username) > 3, 
                                                      do: :ok
                                                    
                                                    defp validate_username(_), do: {:error, :invalid_username}
                                                    

                                                    Nice thing here is that you can add more validations without additional indentation.

                                                    1. 2

                                                      Isn’t this equivalent to just:

                                                         if (validateUsername) {
                                                             # Work goes here
                                                         }
                                                      

                                                      ?