When reviewing Go code, if I run into a situation where I see an unnecessary deviation from idiomatic Go style or best practice, I add an entry here complete with some rationale, and link to it.
I can do this for the smallest and most subtle of details, since I care about Go a lot. I can reuse this each time the same issue comes up, instead of having to re-write the rationale multiple times, or skip explaining why I make a given suggestion.
You can view this as my supplement to https://go.dev/s/style.
This page is generated from the entries with label "Accepted" here. If you'd like to add a new suggestion, please provide convincing rationale and references (e.g., links to places in Go project that support your suggestion), and open a new issue. It'll show up here when I add an "Accepted" label.
Do this:
// marshaling
// unmarshaling
// canceling
// canceled
// cancellation
Don't do this:
// marshalling
// unmarshalling
// cancelling
// cancelled
// cancelation
For consistency with the Go project. These words have multiple valid spellings. The Go project picks one. See go.dev/wiki/Spelling.
Do this:
// Sentence one. Sentence two.
Don't do this:
// Sentence one. Sentence two.
For consistency with Go style. See CL 20022.
Do this:
// Package level exported error.
var ErrSomething = errors.New("something went wrong")
func main() {
// Normally you call it just "err",
result, err := doSomething()
// and use err right away.
// But if you want to give it a longer name, use "somethingError".
var specificError error
result, specificError = doSpecificThing()
// ... use specificError later.
}
Don't do this:
var ErrorSomething = errors.New("something went wrong")
var SomethingErr = errors.New("something went wrong")
var SomethingError = errors.New("something went wrong")
func main() {
var specificErr error
result, specificErr = doSpecificThing()
var errSpecific error
result, errSpecific = doSpecificThing()
var errorSpecific error
result, errorSpecific = doSpecificThing()
}
For consistency. See https://go.dev/talks/2014/names.slide#14.
Do this:
// Exported.
var OAuthEnabled bool
var GitHubToken string
// Unexported.
var oauthEnabled bool
var githubToken string
Don't do this:
// Unexported.
var oAuthEnabled bool
var gitHubToken string
Lowercase all letters, not only the first letter, of the first word when you want to make an unexported version.
For consistency with what the Go project does, and because it looks nicer. oAuth is not pretty.
Do this:
// This is a comment
// for humans.
Don't do this:
//This is a comment
//for humans.
For consistency, and because the no space style is reserved for comment pragmas, for example:
//go:generate go run gen.go
Temporarily commented out code should also have no spaces, but it's not meant to be committed.
Do this:
github.com/golang/example/hello
github.com/golang/example/outyet
golang.org/x/mobile/example/basic
golang.org/x/mobile/example/flappy
golang.org/x/image/...
github.com/shurcooL/tictactoe/player/bad
github.com/shurcooL/tictactoe/player/random
Don't do this:
github.com/golang/examples/hello
github.com/golang/examples/outyet
golang.org/x/mobile/examples/basic
golang.org/x/mobile/examples/flappy
golang.org/x/images/...
github.com/shurcooL/tictactoe/players/bad
github.com/shurcooL/tictactoe/players/random
Notice how "example", "image", "player" folder names simply contain many related packages, yet they use singular form.
Because it's consistent with the style the Go project uses.
Do this:
func (foo) method() {
...
}
Don't do this:
func (f foo) method() {
...
}
If f is unused. It's more readable because it's clear that fields or methods of foo are not used in method.
Do this:
if s == "" {
...
}
Don't do this:
if len(s) == 0 {
...
}
If you're checking if s is the empty string. Using len(s) is fine for other uses.
The first form is more readable because it's more obvious s is a string and not a slice.
See Russ Cox's comment in a golang-nuts thread:
if I care about "is it this specific string" I tend to write s == "".
struct {
...
rateMu sync.Mutex
rateLimits [categories]Rate
mostRecent rateLimitCategory
}
Here, rateMu is a mutex hat. It sits, like a hat, on top of the variables that it protects.
So, without needing to write the comment, the above is implicitly understood to be equivalent to:
struct {
...
// rateMu protects rateLimits and mostRecent.
rateMu sync.Mutex
rateLimits [categories]Rate
mostRecent rateLimitCategory
}
When adding a new, unrelated field that isn't protected by rateMu, do this:
struct {
...
rateMu sync.Mutex
rateLimits [categories]Rate
mostRecent rateLimitCategory
+
+ common service
}
Don't do this:
struct {
...
rateMu sync.Mutex
rateLimits [categories]Rate
mostRecent rateLimitCategory
+ common service
}
Do this:
flag.Usage = func() {
fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0])
flag.PrintDefaults()
}
Don't do this:
flag.Usage = func() {
fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0])
flag.PrintDefaults()
os.Exit(2)
}
The os.Exit call is unnecessary because flag package is already responsible for doing it. See here and here for examples.