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.
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)
}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.
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;
}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.
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...
Lisp is read like python, you read indentation, ignore the parenthesis.
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.
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.
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. 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...
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).
As long as you don't leak panics outside of your package, it's OK to use them for non-local error returns.
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.
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.
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.)
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.
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.
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"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.
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
Just being pedantic here. RAII is more specific than the defer statement that Go offers. Strictly speaking, defer is not RAII.
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.
(Of course, good programming languages provide a better solution than pyramid-of-doom nesting)
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.
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).
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.:-)
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.
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.
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.
[citation needed]
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...
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.
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.)