5 ms·

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 bu

• dragonwriter 13y ago
  > 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 13y ago
  The very next slide cleans it up using a "utility type" which reminds me of the "monadic" Haskell solution.
  • sanderjd 13y ago
    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...
  • RyanZAG 13y ago
    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 13y ago
      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.
      • gnuvince 13y ago
        So basically, monads.
      • alco 13y ago
        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 13y ago
          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.
      • dragonwriter 13y ago