Sergey’s_Go_“style”_guide.md

Sergey’s Go “style” guide

This is a personal interpretation of Go’s nonexistent style/conventions guide with some experience-based kvetching & pet peeves thrown in; it’s a highlights reel and supplement to other guides.

Zen

• There are trade offs to everything. This is a sign, not a cop.
• Most of this advice/preferences comes down to keeping things ‘simple’—reducing interdependencies and knowledge duplication within the code (including textual clutter) and keeping things readable for Sergey (as I often, and repeatedly, skim code).
• Go is an opinionated language written by opinionated C programmers. There are better ways to write code in other languages that are hard to do in Go, by design. Don’t fight it—sometimes you just have to write a for loop and that’s OK.

Other references you want to read

• https://golang.org/doc/effective_go.html
• https://github.com/golang/go/wiki/CodeReviewComments
• https://github.com/golang/go/wiki/CommonMistakes (a.k.a. language design errors)
• https://github.com/golang/go/wiki/SliceTricks (a.k.a. you wish you had generics)

zen/philosophy

• https://www.youtube.com/watch?v=rI8tNMsozo0
• some good bits in https://gist.github.com/stettix/5bb2d99e50fdbbd15dd9622837d14e2b

Note that there’s no official google style guide for Go. The (google internal) guidance from the Go old-hands is to remain flexible and to think about everything in context—in part because Go seems to be a reaction to the rule- and boilerplate-heavy environment of the rest of google—but some of the guidance has turned to embracing the ‘suggestions’ as ‘rules’ (e.g. “you don’t have to wrap lines” morphing into “never wrap lines”) contrary to the original spirit (so beware!).
Consider context in preference to following any advice without question, and consider that preserving local code style may be more valuable than any improvement.

Line width

• Use a ‘suggested’ width that matches the view port on the most commonly used collaboration tool. That’s commonly 100 columns.
• Wrap comments at 100 columns (or shorter, depending on how wordy they are—readability is key).
• Don’t make code lines too long but it is ok to overflow the suggested width.

Comments

• Write comments
• Write less comments; they’ll go out of date quickly.
• Write comments that document ‘why’, not ‘what. Use them to augment the code, not re-state what it does.
• Don’t write comments just because the linter is bugging you to—if you can’t make a more meaningful statement than // Frobnicate frobnicates and returns a frob, don’t bother.
  • Do take a moment to ponder if you can make a meaningful statement—sometimes it’s there but not obvious.

Names

• Use meaningful names, i.e.
  • avoid abstract single letter names, but sometimes these will make sense in context
    • receivers are commonly short (1-3 letters) that are an abbreviation/initialism of the receiver type; this is a common exception and is pretty readable as long as the name is used consistently and exclusively, but use common sense: i is a terrible receiver name (it’s almost always a loop index)
  • avoid names that restate the type of the thing, preferring instead highlighting the reason why you have it, e.g. ‘user’ vs ‘purchaser’.
• Use short name that make sense in context; you should avoid repeating the same word excessively in a single line, and when naming members/functions/modules, consider their usage in context with the same attitude.
  • e.g. foo.New vs foo.NewFoo; customer = user.New() vs customerUser = user.NewUser()

Types

• Use typedefs (type UserID string) instead of raw primitives when it makes sense to have a distinct type .
  • E.g. a user ID has a limited range of values.
  • It makes the intent of your code clearer
  • It avoids accidentally passing nonsensical values and avoids accidents.
  • It makes function signatures more self documenting
  • It explicitly forces a conversion—this is potentially more boilerplate but also a good place to insert a validation function (e.g. string → UserId)
  • When you have stronger types, more bugs will move to the conversion boundary

Structs

• The canonical way to use it (pointer vs. value) can be discovered from whether most of the methods take a pointer or value. https://golang.org/doc/effective_go.html#pointers_vs_values
• Prefer passing structs by value.
• If the struct is non-copyable (or embeds a non-copyable member), use a pointer.
• If the struct is large (>cache line), use a pointer.

Interfaces

• Return structs (or pointers), accept interfaces.
• The function/module accepting arguments should define the interface, not the module implementing the struct.
• If it’s a common enough interface (e.g. Closer, Stringer) a module can define it; but still, prefer returning structs.
• Interfaces are groups of functions. If you’re accepting an interface, do you need all of the functions on it?
  • or just one? If yes, take a function, or a value that’s the result of calling that function
  • A couple? Maybe write a new interface, or take a struct of functions (rarer), or pre-calculate the value.
• Interfaces are great for making things testable when they depend on external behaviors; but it’s even better to avoid that dependency in the first place.

Functions

• Use small functions (this applies in other languages too). Especially, if you’ve got a long loop body, it can probably be a separate function.
  • This makes it easy to use early returns and defer.
• Avoid plain-bool arguments. If you have one, you can usually split the function into two with better names.
• If you have many arguments:
  • Could some be on a context? (request-level globals)
  • Could some be used to pre-calculate the needed value higher up in the stack?
  • Consider taking a struct of options (and properly understand zero-valued members in it)
  • Consider the functional options pattern https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis (which makes a friendlier struct-of-options interface); don’t over use it.
• Try to have pure functions when possible (i.e. accept & return data without side effects). They’re easier to test, reducing the overall testing burden.
  • Splitting a function that does side effects / mutations from the computation function lets you test them differently (often making it easier to get better coverage)
  • But note that Go standard lib patterns often prefer mutating things in place, often for performance reasons (GC is still slow-ish).

Errors

• Return errors along with values.
• Check your errors (yes, it’s terribly manual 😞 )
• Annotate/wrap your errors on the way up with context on what was being done (if not obvious from the error)
  • fmt.Errorf(``"``frobnicating: %w``"``, err)
• Avoid returning meaningful values if you return an error.
• Avoid custom errors.
  • Pick a standard ‘status’ error structure for your code with a canonical set of error codes and use that as often as possible. GRPC.status is an example of how google does it everywhere in their code https://godoc.org/google.golang.org/grpc/status
    • Avoid leaking implementation. If a dependency returned ‘Not Found’ but you actually mean to return ‘InvalidArgument’ at your API, don’t blindly return the error!
    • Note the status codes roughly match up to HTTP error codes, making these errors easy to reconstitute on the other end.
  • Sometimes custom errors may make sense. e.g. io.EOF

Globals

• Don’t use them
• Use them for flags
• Keep them limited to your main module (especially for flags)
• Use contexts to avoid needing globals

Context

• Contexts are request-level bag of variables that can be used to provide request-level globals, but can also be derived from to (temporarily) override these values.
• Have a root background context in your app.
• For each request, create a nested cancellable context. Cancel it when the request times out or when the client goes away.
• Pass that context everywhere in the request execution.
• Use the context for things like:
  • request-specific tags for logging and metrics (e.g. HTTP route or GRPC method name, client, etc)
  • propagating cancelation
  • trace identifiers (for distributed tracing)
  • authentication/authorization for the request
• If doing long running tasks, check the context for cancelation and interrupt/error out your task when it’s cancelled.
• If spawning background tasks, detach the context so that the background task wont get cancelled when the request returns. (Don’t create a new background context as you’ll lose any tracing/logging info that’s on the current context—see https://github.com/golang/tools/commit/5f9351755fc13ce6b9542113c6e61967e89215f6#diff-0be49677a248fa1df56dc1ccee44ee14)

Tests

• Write tests
• Avoid ‘mocks’—use ‘fakes’ and check state, not behavior. Behavior is harder to describe, more likely to change, and usually involves internals of the function—configuring the beginning state and checking the end state will be more robust and may be doable with code you already have on hand. Examples of ‘fakes’:
  • A hash-table based implementation of an S3 client: the initial state is an easily described constant value and the end state can be diffed with another constant.
  • A locally-running MonogDB instance that you can inspect using the existing clients.
• Use table-driven tests. If your functions are pretty pure, this is much easier to do and gets much better coverage of functionality.
  • Unit tests are easy to write and change. Integration tests are harder to set up and write, but catch the bugs in the squishy interconnections between modules.
  • Aim to use many unit tests (made manageable by using table-driven tests) to get good coverage of your algorithms and a few state-based integration tests to confirm everything got plugged correctly.
• Go team doesn’t like assertion frameworks. Use them, but use them wisely.
  • Remember to log your intention for assertions to make it easier to debug.
  • Show both wanted and expected values; try to be consistent on ordering.
  • Output diffs of structures/protobufs instead of just asserting they don’t match.
  • Fail an assertion but continue the test if you can to make each run more useful.