[nknighthb]

    if err == nil {
        _, err := w.Write([]byte(g.Name))
        if err == nil {
            err := binary.Write(w, binary.LittleEndian, g.Age)
            if err == nil {
                return binary.Write(w, binary.LittleEndian, g.FurColor)
            }
            return err
        }
        return err
    }

Why does anyone have to tell people not to do this? How does it enter anyone's mind as a thing to do in the first place? I've been known to go too far to minimize nesting. I get twitchy at the second level. By the third my brain is trying to crawl out my eyes and strangle me, even on code that doesn't have a potential exit point at every level.

[tptacek]

I do it in C code instinctively; it's very useful for instrumentation, debugging, and resource management in straight C to have a single return point. When I get too far to the right, that's a signal that it's time to further decompose my functions; that signal is also useful, which is another thing that keeps me doing it.

But that style doesn't make much sense in Golang. It makes even less sense in Ruby and Python, which have structured exception handling; if you're coming from Pythonistan, the idea of nesting conditions is probably entirely alien.

[cinger]

do you know of any languages that have a sort of inverted switch statement to clean up a list of if statements like this?

something like:

    invswitch err !=nil {
    case 'err := binary.Write(w, binary.LittleEndian, int32(len(g.Name)))': return err
    case _, err = w.Write([]byte(g.Name)): return err
    case err = binary.Write(w, binary.LittleEndian, g.Age): return err
    default: return binary.Write(w, binary.LittleEndian, g.FurColor)
    }

[stass]

Yes, CPS is essentially that and available in any modern language, or can be emuated by setcontext(2)/getcontext(2). However, it can become a mess very quickly.

Another option is monadic style, which will pass error checking along. I believe it will work well for this example, and can be implemented in Go, most likely (I don't know Go, bur it seems so).

Of course, the real problem with this code is that it is not decomposed properly. Nested error checking is the first sign of it, as somebody else already rightfully noted in the thread.

[pcwalton]

I think you'd probably need generics to implement monadic chaining in Go.

[pcwalton]

This is similar to what Haskell do-notation provides.

[xfax]

I agree, having a single return point makes debugging that much easier.

Back at Microsoft, the following pattern was used quite extensively in the OS group:

  int someFunc() {

    DWORD error = ERROR_SUCCESS; 

    error = foo();
    if (error != ERROR_SUCCESS) {
      goto Clean0;
    }

    error = bar();
    if (error != ERROR_SUCCESS) {
      goto Clean0;
    }
    
      ....

    Clean0:
    return error;
  }

[philsnow]

This style is used in the linux kernel as well; there can be multiple resources acquired that sometimes need to be released in reverse order, so there are often multiple labels that you can goto, depending on how much stuff you have to unwind before you return.

[copx]

Quite a few people write deeply nested code for some reason unless you tell them not to. Maybe it depends on how your brain works. There is surprising variance within the human population.

I find deeply nested code ugly and unreadable, others nest eight levels deep and love it. Some people even claim they find parentheses soup (LISP-like syntax) readable. Personally, I cannot comprehend that.

[James_Duval]

I find Lisp-like syntax highly readable - more readable than C-like syntax for sure.

I find that anything more verbose (EDIT: I'm having difficulty thinking of the right word for what I mean here - hopefully you understand anyway) is useful while you're writing the code, but actually increases the difficulty and length of time to understand what's going on when you come back to it.

Symbolic representations of relationships - including parentheses among many others - just make more instant sense to me. To the extent that a characterisation of the Lisp family as "parenthesis soup" just does not make sense to me -- it implies an arbitrary jumble of symbols, while for me, as none of the parentheses could possibly be moved, they're in a perfect and immovable pattern.

I'll admit, though, that without [ and ] I would sometimes be more than a little lost.

I think I might enjoy an APL-like language if I ever had time to learn it...

[EdiX]

> Some people even claim they find parentheses soup (LISP-like syntax) readable. Personally, I cannot comprehend that.

Lisp is read like python, you read indentation, ignore the parenthesis.

[LBarret]

Even the correct version is not that good. The multiple checks on nil is an obvious pattern and as such should be abstracted away. Haskell does it with Maybe but as long as you have function as first class object, it should be doable. Like in Python, Ruby, etc.

[dragonwriter]

> Even the correct version is not that good. The multiple checks on nil is an obvious pattern and as such should be abstracted away.

From a pedagogical point of view, showing the first transition from the anti-pattern is a good practice, because once you learn that, those first transitions are composable. So, while I agree that the correction for this isn't the best end-state, its an appropriate way to show how to correct the nested-conditionals-with-error-returns anti-pattern.

[eckzow]

The very next slide cleans it up using a "utility type" which reminds me of the "monadic" Haskell solution.

[RyanZAG]

No way, the next slide is the very definition of evil unmaintainable code.

Go through what is happening quickly:

    bw := &binWriter{w: w}
    bw.Write(int32(len(g.Name)))
    bw.Write([]byte(g.Name))
    bw.Write(g.Age)
    bw.Write(g.FurColor)
    return bw.err

If an error happens on L2, we will still run writes L3-L5. Why is this bad? Because in the future, we might come in and add logic after the writes complete. We have to make sure to do a check against the error or we will run this logic even if the write did not work. This is an incredibly easy trap to fall into.

I'd go as far as to call that an anti-pattern - you are hiding the control flow in non-obvious ways.

[Someone]

I would prefer to see something like (probably not legal Go, but would work in C, if err is of type bool):

   err = Write(int32(len(g.Name)))
   err |= Write([]byte(g.Name))
   err |= Write(g.Age)
   err |= Write(g.FurColor)
   return err;

If err is of type int with 0 == noError, this will require some new syntax if you want to return the correct error code (which you should want to do). I would suggest introducing

   x ||= y    <===>    x = x || y

In English: if x (still) equals its default, evaluate y and assign the result to x"

One could even shortcut this to

   err = Write(int32(len(g.Name)))
         || Write([]byte(g.Name))
         || Write(g.Age)
         || Write(g.FurColor)
   return err;

Advantage: that's how the shell works, too. Disadvantage is that 'logical or' is not what one associates with "run items until first failure", but that can be learned.

An alternative could be to have a language construct that takes a sequence of lambda's, executes them in sequence until the first one that fails (if any) and returns the index and the result code of the failing lambda. That would get you something like:

   index,err = SEQ([
           Write(int32(len(g.Name)))
           , Write([]byte(g.Name))
           , Write(g.Age)
           , Write(g.FurColor)
         ]);

I like the first idiom better, though.

[AYBABTME]

I've used a pattern such as this in my code [1]:

   	do := func(n int, err error) {
		if err != nil {
			panic(err)
		}
	}

        do(string.Write(/* 1 */))
        do(string.Write(/* 2 */))
        // ...
        do(string.Write(/* n */))

In fact, the pattern I've found is to declare throw-away lambdas [2] that I then call (few lines later). I haven't looked at all the pros/cons of doing this, but so far I find it's a very versatile way of doing.

[1]: https://github.com/aybabtme/graph/blob/master/digraph.go#L61 [2]: https://github.com/aybabtme/graph/blob/master/digraph.go#L10...

[dragonwriter]

> An alternative could be to have a language construct that takes a sequence of lambda's, executes them in sequence until the first one that fails (if any) and returns the index and the result code of the failing lambda.

You could write a function in Go as it is that takes a sequence of lambdas and does that, so why would you need a language construct?

[pcwalton]

> You could write a function in Go as it is that takes a sequence of lambdas and does that, so why would you need a language construct?

You'd want generics if you wanted the chain to return a result though (or if you wanted intermediate steps to thread results through).

[alco]

You can panic inside Write() and recover at the top of the DumpBinary() function. This has been demonstrated by Rob Pike and Andrew Gerrand in a talk at Google IO.

As long as you don't leak panics outside of your package, it's OK to use them for non-local error returns.

[Someone]

Yes, but that would work best if the culture was to panic to signal errors. If it is not (as IIRC in go), you need to wrap common library calls to do so.

[gnuvince]

So basically, monads.

[dragonwriter]

The only problem I see is that binWriter is poorly named: it should be called unreliableBinaryWriter (alternatively, and possibly better, its Write method should be called UnreliablyWrite.)

The purpose of the type and its method is to provide a mechanism to (1) abstract different writing methods for different data types, and (2) silently swallows but records errors to provide a try-but-don't-care-if-I-fail write where you can check for errors later.

Its not a problem that adding logic after those writes that assumes that they were reliable produces incorrect behavior, though it is a problem that the code where the type and its method are used doesn't clearly reflect the intentionally unreliable nature of the operations.

[sanderjd]

Absolutely, I'm going through some of my code now where I had thought of the first improvement as obvious but hadn't considered that second, much nicer solution in Go. It really is like making a custom Maybe type. It might be another pattern to be improved on to have tons of these sorts of custom types running around, though...

[ajanuary]

I normally see a form of it from people that don't like early returns. But if you try to do that with code that explicitly passes pack errors, you end up having to twist the code to avoid early returns. I guess this form pushes all the returns into a list at the bottom, which makes them more comfortable?

[MetaCosm]

People don't like early returns because sometimes they return before some basic cleanup / must have function component is run. Normally added by someone unfamiliar with the function accidentally. Go's defer statement makes this much less of an issue, you bind your work piece with your defer piece so that early returns are fine.

[btilly]

Historically some people were against early returns, loop flow control, and the like as part of structured programming's overreaction to the widespread unstructured use of goto for flow of control. They wanted every body of code, be it a function or a loop, to have one entry point and one exit point.

Over time programmers have learned that one entry point, multiple exit points, is OK. But I feel that it is a good habit to flag that with a comment in capital letters so that someone skimming the code can't miss it.

[dragonwriter]

> Why does anyone have to tell people not to do this? How does it enter anyone's mind as a thing to do in the first place?

Its pretty much the natural naive composition of conditionals. I think eventually most people come to the point where the rightward-marching blocks become irritating and they look for ways to avoid them, and in the case of conditionals where one branch ends in a return there is an easy way to do that, but that particular case (and, thus, the solution) may not be as familiar to people coming from languages where error returns aren't idiomatic (though you do run into it elsewhere, just not as often, so its less likely to result in deep nesting if you don't pay special attention to it.)

[lazyjones]

People end up doing this because code gets written incrementally and often we start out wrong (e.g. here with the wrong/inverted condition). Rewriting large code blocks for a little more clarity is often a PITA.

It would help very much if editors supported this better (e.g. single keystroke inversion of an "if"). Go with its easy syntax would be a particularly good target for automatic rewrites for code like the above, or even just editor-level hints for better style.

[mateuszf]

You can do that in Intellij Idea and related tools.

[brianberns]

Because the alternative is worse. If you test-and-return after every call, your function has multiple exit points and is far less maintainable. If you nest like this, you at least have a chance of maintaining a single exit point in your function (even though this example fails to do so).

This is why the Lord invented exceptions, which it seems that Go does not use. This one example is enough to convince me to never use Go for anything. What a huge step backwards.

[AYBABTME]

I encounter this argument daily and I believe it usually comes from programming dogma of people who read very assertive quotes from Dijkstra.

Most of the time when error checking, returns (or in C, gotos) are fine and lead to more readable code that's easy to make sense of and easy to step through with a debugger.

    Please don't fall into the trap of believing that I am 
    terribly dogmatic about [the go to statement]. I have the 
    uncomfortable feeling that others are making a religion 
    out of it, as if the conceptual problems of programming 
    could be solved by a simple trick, by a simple form of 
    coding discipline!

- Dijkstra (1973) in personal communication to Donald Knuth , quoted in Knuth's "Structured Programming with go to Statements"

https://en.wikiquote.org/wiki/Edsger_W._Dijkstra

[gnaritas]

> your function has multiple exit points

That's a good thing.

> and is far less maintainable

I'd say it's more maintainable.

> you at least have a chance of maintaining a single exit point in your function

Maintaining a single exit point is IMHO an anti-pattern; it makes ugly unmaintainable deeply nested code.

[EdiX]

> If you test-and-return after every call, your function has multiple exit points and is far less maintainable.

There is nothing wrong with multiple exit points as long as:

a. the language has a mechanism for scoped resource allocation (ie. defer, finally, with, unwind-protect or "RAII")

b. the early exit doesn't happen in the middle of a long and complex function

[azth]

> a. the language has a mechanism for scoped resource allocation (ie. defer, finally, with, unwind-protect or "RAII")

Just being pedantic here. RAII is more specific than the defer statement that Go offers. Strictly speaking, defer is not RAII.

[krichman]

I always use a single exit point in my functions and also a goto to get there from the middle.

[mrweasel]

>How does it enter anyone's mind as a thing to do in the first place?

Because a lot of people make up code as they go a long.

Think of it as story telling vs. ordering. When story telling something happens and the your characters/code react to the outcome of that event, but stays in the same context.

Instead it would be better to write code as your being ordered around by an old drill sergeant. He'll always tell you to do one thing and one thing only. When you've completed that task, you given the next instruction and are no longer in the context of the previous action.

It's something I thought about often when reading code, both that of others and my own and it's the best explanation I've been able to come up with.

[lmm]

Short circuit returns are the devil - they make it much harder to factor out part of a function into a smaller function. A function should have one entry point and one exit point; that's the whole point of structured programming. If you're going to return from some random point in the middle of your function you might as well be using goto.

(Of course, good programming languages provide a better solution than pyramid-of-doom nesting)

[nknighthb]

> Short circuit returns are the devil

vi!

Naïve, absolutist positions in areas of long-standing debate between programmers of great experience and the highest imaginable competence just makes you look ridiculous.

[jemfinch]

Naive, absolutist positions in areas of long-standing consensus between programmers of great experience and the highest imaginable competence makes one look even more ridiculous.

By and large, the best programmers eschew nesting in favor of early returns. Invariably (in my experience) those who argue against early returns are inferior programmers (and not only by virtue of lacking taste in this particular debate).

[stass]

Where did you get that idea of consensus? A lot of languages do not even have a return statement, neither does lambda calculus. Furthermore, CS community has long abandoned statement based languages in favor of expressions and relations which do not feature "return" for onvious reasons in forms other than equalent to jump.

[jemfinch]

I'm talking about programmers, not computer scientists. That is, people who actually accomplish things in the real world by hacking on software, rather than pontificating about it from their monadic ivory towers :) The latter have "abandoned statement-based languages," but the former absolutely have not.

[stass]

I don't see that distinction. The are languages designed to map directly to register machines internal operation, like C or C++ or fortran and those are statement based because that's how the machine works. There are also high level languages, designed to express computation, and those are expression based, again, beacause computation is inherently based on expessions. There are other kinds of languages as well, for other purposes (relations, queries, etc).

Do you really belive that only people working on low level register transfer level things "accomplish things"? That's souds like a very old assembler argument.:-)

[gnaritas]

> By and large, the best programmers eschew nesting in favor of early returns.

Yup.

> Invariably (in my experience) those who argue against early returns are inferior programmers (and not only by virtue of lacking taste in this particular debate).

Yup.

[gnaritas]

> Short circuit returns are the devil

No, they're not.

> they make it much harder to factor out part of a function into a smaller function

They generally make the function smaller, that's what guard clauses are for.

> A function should have one entry point and one exit point

That's absurd and makes for very ugly and unnecessary code.

[lmm]

> They generally make the function smaller, that's what guard clauses are for.

My point is: frequently, one wants to extract a part from the middle of a function to make a new function. This is very easy (and can usually be done automatically) unless said part contains a return.

[gnaritas]

Suffering the unbearable burden of a single exit point just so your refactoring tool has an easier time breaking up the code isn't a trade worth making, especially when it's the single exit point that's probably making it too long to begin with.

[sneak]

> A function should have one entry point and one exit point; that's the whole point of structured programming.

[citation needed]

[munificent]

> A function should have one entry point and one exit point; that's the whole point of structured programming.

I think a reasonable primary source for "the whole point of structured programming" is Dijkstra's "Go To Statement Considered Harmful"[1]. It's a short article and once you get past his writing style, his point is simple and lucid.

In the first part he lays out a simple question: given a program text, how much information do you need to track of to correctly identify where the program is currently executing at some point in time and how it got there? Roughly, if you were paused in the debugger, how much state does the debugger have to hold in order to be able to resume where it left off?

He's asking this because the smaller amount of state required for this, the easier it is for a human to look at a program text and figure out what can happen while it's running dynamically.

If all your language had was assignment statements, it's simple: you basically just need a single line number. Adding "if" and "switch" for branching doesn't add any more complexity. And, of course, reading code like this is pretty trivial.

If you have procedure calls (and by implication, recursion), you need a stack of those numbers, which is exactly what the callstack in your debugger holds.

When you add "while" and "for" for looping, you need to keep track of how times you've gone around the loop.

Now, if you add "go to" everything goes to hell. If you're on line 10, did you get there because you were on line 9 before, or because you jumped to it, or some random combination of those? It's a total mess.

Then he says:

> I do not claim that the clauses mentioned are exhaustive in the sense that they will satisfy all needs, but whatever clauses are suggested (e.g. abortion clauses) they should satisfy the requirement that a programmer independent coordinate system can be maintained to describe the process in a helpful and manageable way.

Here he's saying you can add any other "clauses" (flow control constructs) to a language that you want as long as you don't add to the amount of state you need to store to keep track of how you got there.

For example, adding an "unless" statement that works like "if" but only has an "else" clause instead of a "then" clause is peachy. You don't need any additional data to keep track of where you are.

You know what else doesn't require adding any additional data? Early returns.

So as far as Dijkstra is concerned, no, avoiding early returns is not at all the point of structured programming.

[1]: http://www.u.arizona.edu/~rubinson/copyright_violations/Go_T...

[jamesaguilar]

Having trouble picturing a better solution that isn't also morally equivalent to goto. Would you mind providing an example of your preferred approach?

[lmm]

Straight exceptions are also goto-like, but there's the slight improvement that they allow you to (automatically) refactor out the middle of a function without changing the behaviour.

The solution I prefer is a monad with some light notation (Haskell do, scala for/yield). So it looks like:

    def doSomething(): Validation[Whatever] =
      for {
        result1 <- callThatCanFail()
        result2 <- callThatCanFail2(result1, 4)
        intermediate = callThatCantFail()
        result3 <- anotherCallThatCanFail(result2 + 4, "Hello", intermediate)
      } yield result3 + result1

The <- syntax clues you in that this computation is going on in some kind of "context" - if you're working with futures it might happen on a different thread, if you're working with collections you're iterating over the elements. Here we're working with a validation, and our computations only happen if the previous one succeeds.

Is this goto-like? You could argue so - once one computation fails, the rest turn into noops and we pass immediately to the end. But to my mind this is like replacing an if/else with polymorphism - our validation context is an object with certain behaviours, and we're calling methods on it with our functions as parameters, which will behave differently depending on which subtype our context instance is.

[jamesaguilar]

I don't see how the <- clues you in any better than the error handling that things might exit early. It seems from your response that the goto-ness of the go solution is not what you're really objecting to, because your proposal is equally goto-y. As far as I can tell, it's really the if-else verbosity that you don't like, which is fine (and I agree, exceptions are better), but it has nothing to do with the "whole point of structured programming." And, unless I'm mistaken, structured programming also lacked exceptions in its initial formulation.

[lmm]

I think the main thing I'm objecting to is the possibility of multiple paths through the function. Doing it this way there's only one possible flow: a series of calls that pass blocks to validation objects (that then may or may not execute them).

Compare how smalltalk didn't have a conditional control flow statement. Rather, the boolean type has a polymorphic method that takes a block and then executes it or not.

The other nice thing about this approach is it makes the language simpler and more regular because it's implemented using standard language constructs rather than a special statement. E.g. you can write a "gather" function that takes a collection of monads and returns a monad of the collection, and this is generic in the monad (so the same function works to turn a List of Futures into a Future of a List, a List of Validations into a Validation of a List, etc.)

[sultezdukes]

No, no, no. That's what college professors with no real world experience tell you. In the real world, professionals use guard clauses to exit early all the time.