pkg / errors

Simple error handling primitives
https://godoc.org/github.com/pkg/errors
BSD 2-Clause "Simplified" License
8.18k stars 692 forks source link

Clean up a couple places to replace verbose switch statements #162

Closed dchenk closed 5 years ago

dchenk commented 6 years ago

These changes make the code slightly simpler, cleaner. Some other places in the code in this package use simple conditionals the way I wrote it here.

puellanivis commented 6 years ago

My central concern here is that this alters the model/pattern used for Format, as it is typically expected that while testing s.Flag, a fmt.Formater will test a number of different flags, and Go tends to prefer switch { case cond1: … ; case cond2: … ; default: … } (switch statements) over using if cond1 { … } else if cond2 { … } else { … } (if-else cascades).

And it is because s.Flag() can be checked for more flags than just '+' that the switch is more appropriate here. c.f. https://golang.org/src/math/big/ftoa.go#L444

So, this is a “weird” corner-case where there is a confluence of a) Go prefers switch statements over if-else cascades, and b) despite only having at the moment one or two branches, the conditions being checked are actually if-else cascades, not just simple binary behavior.

“Simpler” and “cleaner” is not always shorter, one must also consider the semantics of the code when choosing the “best” syntax.

HaraldNordgren commented 5 years ago

Agree with @puellanivis 100%. If the code looked like the if-else patterns I might suggest changing it to a switch actually.

davecheney commented 5 years ago

Thank you but I prefer the code as is.