2 ms·

Worse, if res, err := actually_important_bits(...); err != nil { return nil, err } The actually important bits are hidden in the middle of lin

Worse, if res, err := actually_important_bits(...); err != nil { return nil, err } The actually important bits are hidden in the middle of line noise. In the "common case" where `actually_important_bits` is just a simple function call it's not necessarily as bad, but the problem is when you have ten successive instances of this and one is slightly different. It's impossible to notice the important difference at a glance. For an industry that is just starting to understand that code is read hundreds of times more often than it's written, golang fails at the few things we actually know for certain about what makes it easier to understand code at a glance.

jrockway 5y ago
If you're using `res`, then you aren't going to scope it to the `if` statement. That is maybe another problem, sometimes you have to do this: res, err := actuallyImportantBits(...) if err != nil { return nil, fmt.Errorf("actually important bits: %w", err) } And other times you have to do this: if err := actuallyImportantBits(...); err != nil { return nil, fmt.Errorf("actually important bits: %w", err) } The problem here is that you really don't want "err" to leak to the outer scope, so the second case is preferable from an absolute reliability and least-surprise perspective... but you can only do that in certain arbitrary cases. I think it's a bit of a wart. You can certainly handle "res" in an else block, or even write "err == nil" and handle it there, but that is surprsing. I would say it's simply not done, ever, but the Go codebase itself does it (src/go/parser/interface.go.ParseDir was the first example I found; but my search returned many screenfuls of candidates so there are probably more cases lurking in there). The fact that you have a choice is not ideal, basically. But, having actually important bits in if err := ...; err != nil {} blocks is not detrimental to readability. You will know how to read that after 5 minutes of reading any Go program.
jerf 5y ago
I almost never use that in my code, I almost always use res, err := ... if err != nil { return nil, fmt.Errorf("...: %w", err) } for that exact reason. This doesn't scan any worse than any other text-based code. (Page in the people selling visual programming here.)

jrockway 5y ago

If you're using `res`, then you aren't going to scope it to the `if` statement. That is maybe another problem, sometimes you have to do this: res, err := actuallyImportantBits(...) if err != nil { return nil, fmt.Errorf("actually important bits: %w", err) } And other times you have to do this: if err := actuallyImportantBits(...); err != nil { return nil, fmt.Errorf("actually important bits: %w", err) } The problem here is that you really don't want "err" to leak to the outer scope, so the second case is preferable from an absolute reliability and least-surprise perspective... but you can only do that in certain arbitrary cases. I think it's a bit of a wart. You can certainly handle "res" in an else block, or even write "err == nil" and handle it there, but that is surprsing. I would say it's simply not done, ever, but the Go codebase itself does it (src/go/parser/interface.go.ParseDir was the first example I found; but my search returned many screenfuls of candidates so there are probably more cases lurking in there). The fact that you have a choice is not ideal, basically. But, having actually important bits in if err := ...; err != nil {} blocks is not detrimental to readability. You will know how to read that after 5 minutes of reading any Go program.

jerf 5y ago

I almost never use that in my code, I almost always use res, err := ... if err != nil { return nil, fmt.Errorf("...: %w", err) } for that exact reason. This doesn't scan any worse than any other text-based code. (Page in the people selling visual programming here.)