[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...