3 ms·
Relying on code review to catch bugs is just leaving it up to a different programmer. Code review is wonderful, but expecting it to catch every occurrence of ev
by plorkyeran 5y ago
Relying on code review to catch bugs is just leaving it up to a different programmer. Code review is wonderful, but expecting it to catch every occurrence of even just the categories of bugs easily caught by code review is wildly overoptimistic.
Ignoring the second return value is also the easy case to catch. Ignoring an error-only return from a function doesn't produce a warning, and won't stand out in code review unless you adopt a standard of never writing void functions.
- preseinger 5y ago> Relying on code review to catch bugs is just leaving it up to a different programmer. Go is a language for programminging "in the large" -- it explicitly defers a lot of stuff to layers "above" the compiler, like code review. If you don't buy that as legitimate that's fine!
- plorkyeran 5y agoDefering things to other layers such as code review is the exact opposite of Go making it non-optional to explicitly handle errors. Idiomatic Go has developed very rigid patterns around how to handle errors specifically because if you want to write happy-path code which ignores errors the language itself does nothing to discourage you from doing that, and in fact makes that the easiest thing to do.
- preseinger 5y agoI really don't understand the point you're making. > if you want to write happy-path code which ignores errors . . . . . . it's obvious that's what you're doing in code review, and it's trivial to correct. Right? --- > if you want to write happy-path code which ignores error If you're doing this consistently you're going to have a lot of `value, _` i.e. underbar assignments in your code, and `_` isn't passing any code review process in any reasonable Go shop I've ever seen.
- plorkyeran 5y agoTo loop back to the original claim I disagree with: > Go asserts that "error handling" and "main flow" (a.k.a. "happy path") code paths are equivalent, and that you can't ignore the bad stuff as you program against the good stuff. Error handling _cannot be_ an afterthought -- this is the path to fragile, even broken, code. Go doesn't assert anything about error handling. Go itself is extremely unopinionated about error handling. If you want to do something with the errors it returns you can, but if for some reason you want to "ignore the bad stuff as you program against the good stuff" it won't try to get in your way. Idiomatic Go never just drops errors on the floor and ignores them, but it does involve a lot of just bubbling errors upwards without spending any time thinking about what happens in the error path. It's very easy to just write `v, error := foo() if error != nil { return error }` without considering if everything's actually in a sensible state if you return here or if bubbling the error up is even the correct thing to do. It _is_ the correct thing to do so often that anyone reading the code is also going to just glance over it. If a programmer doesn't spend time thinking about error handling, Go does nothing to encourage them to do so. If they do, Go offers very little to help them do so. It certainly compares well in that regard to C and languages where every expression can throw an exception, but that is a very low standard.
- preseinger 5y ago> Go doesn't assert anything about error handling. Go itself is extremely unopinionated about error handling. If you want to do something with the errors it returns you can, but if for some reason you want to "ignore the bad stuff as you program against the good stuff" it won't try to get in your way. The compiler will let you do this, but reasonable code review won't. > Idiomatic Go never just drops errors on the floor and ignores them, but it does involve a lot of just bubbling errors upwards without spending any time thinking about what happens in the error path. It's very easy to just write `v, error := foo() if error != nil { return error }` without considering if everything's actually in a sensible state if you return here or if bubbling the error up is even the correct thing to do. It _is_ the correct thing to do so often that anyone reading the code is also going to just glance over it. Not true? Keeping error handling in the same frame of reference as happy-path code makes programmers consider the two things equivalently, as peers. `if err != nil { return err }` is _not_ idiomatic Go! At a minimum you annotate the error. And that's often enough to be the right thing to do. > If a programmer doesn't spend time thinking about error handling, Go does nothing to encourage them to do so. Simply lifting error-path code into the same visible control flow as happy-path code is by itself a huge win for making programmers think about error handling. If you don't agree, fine. Many many others do.
- Mawr 5y ago> [...] unless you adopt a standard of never writing void functions. To be fair, that is a good principle to follow ;) Non-void, especially pure, functions are much easier to understand in the context of an entire codebase. > Ignoring an error-only return from a function doesn't produce a warning, It does if you use the standard set of linters, which with Go you almost certainly do. Go has a strong linting culture, in part due to one being built-in. By the way, in vscode said standard set of linters is one click away in the config. --- There are only 2-3 cases that I know of where failing to handle an error passes by silently. They are edge cases, not something you'll run into writing everyday code.