Crew Dev Go Style Guide
This style guide is based on uber’s style guide for Go lang
Table of Contents
Introduction
Styles are the conventions that govern our code. The term style is a bit of a
misnomer, since these conventions cover far more than just source file
formatting—gofmt handles that for us.
The goal of this guide is to manage this complexity by describing in detail the
Dos and Don’ts of writing Go code at Uber. These rules exist to keep the code
base manageable while still allowing engineers to use Go language features
productively.
This guide was originally created by Prashant Varanasi and Simon Newton as
a way to bring some colleagues up to speed with using Go. Over the years it has
been amended based on feedback from others.
This documents idiomatic conventions in Go code that we follow at Uber. A lot
of these are general guidelines for Go, while others extend upon external
resources:
- Effective Go
- The Go common mistakes guide
All code should be error-free when run through golint and go vet. We
recommend setting up your editor to:
- Run
goimports on save
- Run
golint and go vet to check for errors
You can find information in editor support for Go tools here:
https://github.com/golang/go/wiki/IDEsAndTextEditorPlugins
Guidelines
Pointers to Interfaces
You almost never need a pointer to an interface. You should be passing
interfaces as values—the underlying data can still be a pointer.
An interface is two fields:
- A pointer to some type-specific information. You can think of this as
“type.”
- Data pointer. If the data stored is a pointer, it’s stored directly. If
the data stored is a value, then a pointer to the value is stored.
If you want interface methods to modify the underlying data, you must use a
pointer.
Verify Interface Compliance
Verify interface compliance at compile time where appropriate. This includes:
- Exported types that are required to implement specific interfaces as part of
their API contract
- Exported or unexported types that are part of a collection of types
implementing the same interface
- Other cases where violating an interface would break users
| Bad | Good |
|---|
|
```go
type Handler struct {
// ...
}
func (h *Handler) ServeHTTP(
w http.ResponseWriter,
r *http.Request,
) {
...
}
```
|
```go
type Handler struct {
// ...
}
var _ http.Handler = (*Handler)(nil)
func (h *Handler) ServeHTTP(
w http.ResponseWriter,
r *http.Request,
) {
// ...
}
```
|
The statement var _ http.Handler = (*Handler)(nil) will fail to compile if
*Handler ever stops matching the http.Handler interface.
The right hand side of the assignment should be the zero value of the asserted
type. This is nil for pointer types (like *Handler), slices, and maps, and
an empty struct for struct types.
type LogHandler struct {
h http.Handler
log *zap.Logger
}
var _ http.Handler = LogHandler{}
func (h LogHandler) ServeHTTP(
w http.ResponseWriter,
r *http.Request,
) {
// ...
}
Receivers and Interfaces
Methods with value receivers can be called on pointers as well as values.
Methods with pointer receivers can only be called on pointers or addressable values.
For example,
type S struct {
data string
}
func (s S) Read() string {
return s.data
}
func (s *S) Write(str string) {
s.data = str
}
sVals := map[int]S{1: {"A"}}
// You can only call Read using a value
sVals[1].Read()
// This will not compile:
// sVals[1].Write("test")
sPtrs := map[int]*S{1: {"A"}}
// You can call both Read and Write using a pointer
sPtrs[1].Read()
sPtrs[1].Write("test")
Similarly, an interface can be satisfied by a pointer, even if the method has a
value receiver.
type F interface {
f()
}
type S1 struct{}
func (s S1) f() {}
type S2 struct{}
func (s *S2) f() {}
s1Val := S1{}
s1Ptr := &S1{}
s2Val := S2{}
s2Ptr := &S2{}
var i F
i = s1Val
i = s1Ptr
i = s2Ptr
// The following doesn't compile, since s2Val is a value, and there is no value receiver for f.
// i = s2Val
Effective Go has a good write up on [Pointers vs. Values].
Zero-value Mutexes are Valid
The zero-value of sync.Mutex and sync.RWMutex is valid, so you almost
never need a pointer to a mutex.
| Bad | Good |
|---|
|
```go
mu := new(sync.Mutex)
mu.Lock()
```
|
```go
var mu sync.Mutex
mu.Lock()
```
|
If you use a struct by pointer, then the mutex can be a non-pointer field.
Unexported structs that use a mutex to protect fields of the struct may embed
the mutex.
|
```go
type smap struct {
sync.Mutex // only for unexported types
data map[string]string
}
func newSMap() *smap {
return &smap{
data: make(map[string]string),
}
}
func (m *smap) Get(k string) string {
m.Lock()
defer m.Unlock()
return m.data[k]
}
```
|
```go
type SMap struct {
mu sync.Mutex
data map[string]string
}
func NewSMap() *SMap {
return &SMap{
data: make(map[string]string),
}
}
func (m *SMap) Get(k string) string {
m.mu.Lock()
defer m.mu.Unlock()
return m.data[k]
}
```
|
</tr>
| Embed for private types or types that need to implement the Mutex interface. |
For exported types, use a private field. |
Copy Slices and Maps at Boundaries
Slices and maps contain pointers to the underlying data so be wary of scenarios
when they need to be copied.
Receiving Slices and Maps
Keep in mind that users can modify a map or slice you received as an argument
if you store a reference to it.
| Bad | Good |
|---|
|
```go
func (d *Driver) SetTrips(trips []Trip) {
d.trips = trips
}
trips := ...
d1.SetTrips(trips)
// Did you mean to modify d1.trips?
trips[0] = ...
```
|
```go
func (d *Driver) SetTrips(trips []Trip) {
d.trips = make([]Trip, len(trips))
copy(d.trips, trips)
}
trips := ...
d1.SetTrips(trips)
// We can now modify trips[0] without affecting d1.trips.
trips[0] = ...
```
|
Returning Slices and Maps
Similarly, be wary of user modifications to maps or slices exposing internal
state.
| Bad | Good |
|---|
|
```go
type Stats struct {
mu sync.Mutex
counters map[string]int
}
// Snapshot returns the current stats.
func (s *Stats) Snapshot() map[string]int {
s.mu.Lock()
defer s.mu.Unlock()
return s.counters
}
// snapshot is no longer protected by the mutex, so any
// access to the snapshot is subject to data races.
snapshot := stats.Snapshot()
```
|
```go
type Stats struct {
mu sync.Mutex
counters map[string]int
}
func (s *Stats) Snapshot() map[string]int {
s.mu.Lock()
defer s.mu.Unlock()
result := make(map[string]int, len(s.counters))
for k, v := range s.counters {
result[k] = v
}
return result
}
// Snapshot is now a copy.
snapshot := stats.Snapshot()
```
|
Defer to Clean Up
Use defer to clean up resources such as files and locks.
| Bad | Good |
|---|
|
```go
p.Lock()
if p.count < 10 {
p.Unlock()
return p.count
}
p.count++
newCount := p.count
p.Unlock()
return newCount
// easy to miss unlocks due to multiple returns
```
|
```go
p.Lock()
defer p.Unlock()
if p.count < 10 {
return p.count
}
p.count++
return p.count
// more readable
```
|
Defer has an extremely small overhead and should be avoided only if you can
prove that your function execution time is in the order of nanoseconds. The
readability win of using defers is worth the miniscule cost of using them. This
is especially true for larger methods that have more than simple memory
accesses, where the other computations are more significant than the defer.
Channel Size is One or None
Channels should usually have a size of one or be unbuffered. By default,
channels are unbuffered and have a size of zero. Any other size
must be subject to a high level of scrutiny. Consider how the size is
determined, what prevents the channel from filling up under load and blocking
writers, and what happens when this occurs.
| Bad | Good |
|---|
|
```go
// Ought to be enough for anybody!
c := make(chan int, 64)
```
|
```go
// Size of one
c := make(chan int, 1) // or
// Unbuffered channel, size of zero
c := make(chan int)
```
|
Start Enums at One
The standard way of introducing enumerations in Go is to declare a custom type
and a const group with iota. Since variables have a 0 default value, you
should usually start your enums on a non-zero value.
| Bad | Good |
|---|
|
```go
type Operation int
const (
Add Operation = iota
Subtract
Multiply
)
// Add=0, Subtract=1, Multiply=2
```
|
```go
type Operation int
const (
Add Operation = iota + 1
Subtract
Multiply
)
// Add=1, Subtract=2, Multiply=3
```
|
There are cases where using the zero value makes sense, for example when the
zero value case is the desirable default behavior.
type LogOutput int
const (
LogToStdout LogOutput = iota
LogToFile
LogToRemote
)
// LogToStdout=0, LogToFile=1, LogToRemote=2
Use "time" to handle time
Time is complicated. Incorrect assumptions often made about time include the
following.
- A day has 24 hours
- An hour has 60 minutes
- A week has 7 days
- A year has 365 days
- And a lot more
For example, 1 means that adding 24 hours to a time instant will not always
yield a new calendar day.
Therefore, always use the "time" package when dealing with time because it
helps deal with these incorrect assumptions in a safer, more accurate manner.
Use time.Time for instants of time
Use time.Time when dealing with instants of time, and the methods on
time.Time when comparing, adding, or subtracting time.
| Bad | Good |
|---|
|
```go
func isActive(now, start, stop int) bool {
return start <= now && now < stop
}
```
|
```go
func isActive(now, start, stop time.Time) bool {
return (start.Before(now) || start.Equal(now)) && now.Before(stop)
}
```
|
Use time.Duration for periods of time
Use time.Duration when dealing with periods of time.
| Bad | Good |
|---|
|
```go
func poll(delay int) {
for {
// ...
time.Sleep(time.Duration(delay) * time.Millisecond)
}
}
poll(10) // was it seconds or milliseconds?
```
|
```go
func poll(delay time.Duration) {
for {
// ...
time.Sleep(delay)
}
}
poll(10*time.Second)
```
|
Going back to the example of adding 24 hours to a time instant, the method we
use to add time depends on intent. If we want the same time of the day, but on
the next calendar day, we should use Time.AddDate. However, if we want an
instant of time guaranteed to be 24 hours after the previous time, we should
use Time.Add.
newDay := t.AddDate(0 /* years */, 0, /* months */, 1 /* days */)
maybeNewDay := t.Add(24 * time.Hour)
Use time.Time and time.Duration with external systems
Use time.Duration and time.Time in interactions with external systems when
possible. For example:
When it is not possible to use time.Duration in these interactions, use
int or float64 and include the unit in the name of the field.
For example, since encoding/json does not support time.Duration, the unit
is included in the name of the field.
| Bad | Good |
|---|
|
```go
// {"interval": 2}
type Config struct {
Interval int `json:"interval"`
}
```
|
```go
// {"intervalMillis": 2000}
type Config struct {
IntervalMillis int `json:"intervalMillis"`
}
```
|
When it is not possible to use time.Time in these interactions, unless an
alternative is agreed upon, use string and format timestamps as defined in
RFC 3339. This format is used by default by Time.UnmarshalText and is
available for use in Time.Format and time.Parse via time.RFC3339.
Although this tends to not be a problem in practice, keep in mind that the
"time" package does not support parsing timestamps with leap seconds
(8728), nor does it account for leap seconds in calculations (15190). If
you compare two instants of time, the difference will not include the leap
seconds that may have occurred between those two instants.
Error Types
There are various options for declaring errors:
When returning errors, consider the following to determine the best choice:
If the client needs to detect the error, and you have created a simple error
using errors.New, use a var for the error.
| Bad | Good |
|---|
|
```go
// package foo
func Open() error {
return errors.New("could not open")
}
// package bar
func use() {
if err := foo.Open(); err != nil {
if err.Error() == "could not open" {
// handle
} else {
panic("unknown error")
}
}
}
```
|
```go
// package foo
var ErrCouldNotOpen = errors.New("could not open")
func Open() error {
return ErrCouldNotOpen
}
// package bar
if err := foo.Open(); err != nil {
if err == foo.ErrCouldNotOpen {
// handle
} else {
panic("unknown error")
}
}
```
|
If you have an error that clients may need to detect, and you would like to add
more information to it (e.g., it is not a static string), then you should use a
custom type.
| Bad | Good |
|---|
|
```go
func open(file string) error {
return fmt.Errorf("file %q not found", file)
}
func use() {
if err := open("testfile.txt"); err != nil {
if strings.Contains(err.Error(), "not found") {
// handle
} else {
panic("unknown error")
}
}
}
```
|
```go
type errNotFound struct {
file string
}
func (e errNotFound) Error() string {
return fmt.Sprintf("file %q not found", e.file)
}
func open(file string) error {
return errNotFound{file: file}
}
func use() {
if err := open("testfile.txt"); err != nil {
if _, ok := err.(errNotFound); ok {
// handle
} else {
panic("unknown error")
}
}
}
```
|
Be careful with exporting custom error types directly since they become part of
the public API of the package. It is preferable to expose matcher functions to
check the error instead.
// package foo
type errNotFound struct {
file string
}
func (e errNotFound) Error() string {
return fmt.Sprintf("file %q not found", e.file)
}
func IsNotFoundError(err error) bool {
_, ok := err.(errNotFound)
return ok
}
func Open(file string) error {
return errNotFound{file: file}
}
// package bar
if err := foo.Open("foo"); err != nil {
if foo.IsNotFoundError(err) {
// handle
} else {
panic("unknown error")
}
}
Error Wrapping
There are three main options for propagating errors if a call fails:
- Return the original error if there is no additional context to add and you
want to maintain the original error type.
- Add context using
"pkg/errors".Wrap so that the error message provides
more context and ["pkg/errors".Cause] can be used to extract the original
error.
- Use
fmt.Errorf if the callers do not need to detect or handle that
specific error case.
It is recommended to add context where possible so that instead of a vague
error such as “connection refused”, you get more useful errors such as
“call service foo: connection refused”.
When adding context to returned errors, keep the context succinct by avoiding
phrases like “failed to”, which state the obvious and pile up as the error
percolates up through the stack:
| Bad | Good |
|---|
|
```go
s, err := store.New()
if err != nil {
return fmt.Errorf(
"failed to create new store: %s", err)
}
```
|
```go
s, err := store.New()
if err != nil {
return fmt.Errorf(
"new store: %s", err)
}
```
|
|
```
failed to x: failed to y: failed to create new store: the error
```
|
```
x: y: new store: the error
```
|
</tbody></table>
However once the error is sent to another system, it should be clear the
message is an error (e.g. an `err` tag or "Failed" prefix in logs).
See also [Don't just check errors, handle them gracefully].
[`"pkg/errors".cause`]: https://godoc.org/github.com/pkg/errors#Cause
[don't just check errors, handle them gracefully]: https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully
### Handle Type Assertion Failures
The single return value form of a [type assertion] will panic on an incorrect
type. Therefore, always use the "comma ok" idiom.
[type assertion]: https://golang.org/ref/spec#Type_assertions
| Bad | Good |
|---|
|
```go
t := i.(string)
```
|
```go
t, ok := i.(string)
if !ok {
// handle the error gracefully
}
```
|
### Don't Panic
Code running in production must avoid panics. Panics are a major source of
[cascading failures]. If an error occurs, the function must return an error and
allow the caller to decide how to handle it.
[cascading failures]: https://en.wikipedia.org/wiki/Cascading_failure
| Bad | Good |
|---|
|
```go
func run(args []string) {
if len(args) == 0 {
panic("an argument is required")
}
// ...
}
func main() {
run(os.Args[1:])
}
```
|
```go
func run(args []string) error {
if len(args) == 0 {
return errors.New("an argument is required")
}
// ...
return nil
}
func main() {
if err := run(os.Args[1:]); err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
}
```
|
Panic/recover is not an error handling strategy. A program must panic only when
something irrecoverable happens such as a nil dereference. An exception to this is
program initialization: bad things at program startup that should abort the
program may cause panic.
```go
var _statusTemplate = template.Must(template.New("name").Parse("_statusHTML"))
```
Even in tests, prefer `t.Fatal` or `t.FailNow` over panics to ensure that the
test is marked as failed.
| Bad | Good |
|---|
|
```go
// func TestFoo(t *testing.T)
f, err := ioutil.TempFile("", "test")
if err != nil {
panic("failed to set up test")
}
```
|
```go
// func TestFoo(t *testing.T)
f, err := ioutil.TempFile("", "test")
if err != nil {
t.Fatal("failed to set up test")
}
```
|
### Use go.uber.org/atomic
Atomic operations with the [sync/atomic] package operate on the raw types
(`int32`, `int64`, etc.) so it is easy to forget to use the atomic operation to
read or modify the variables.
[go.uber.org/atomic] adds type safety to these operations by hiding the
underlying type. Additionally, it includes a convenient `atomic.Bool` type.
[go.uber.org/atomic]: https://godoc.org/go.uber.org/atomic
[sync/atomic]: https://golang.org/pkg/sync/atomic/
| Bad | Good |
|---|
|
```go
type foo struct {
running int32 // atomic
}
func (f* foo) start() {
if atomic.SwapInt32(&f.running, 1) == 1 {
// already running…
return
}
// start the Foo
}
func (f *foo) isRunning() bool {
return f.running == 1 // race!
}
```
|
```go
type foo struct {
running atomic.Bool
}
func (f *foo) start() {
if f.running.Swap(true) {
// already running…
return
}
// start the Foo
}
func (f *foo) isRunning() bool {
return f.running.Load()
}
```
|
### Avoid Mutable Globals
Avoid mutating global variables, instead opting for dependency injection.
This applies to function pointers as well as other kinds of values.
| Bad | Good |
|---|
|
```go
// sign.go
var _timeNow = time.Now
func sign(msg string) string {
now := _timeNow()
return signWithTime(msg, now)
}
```
|
```go
// sign.go
type signer struct {
now func() time.Time
}
func newSigner() *signer {
return &signer{
now: time.Now,
}
}
func (s *signer) Sign(msg string) string {
now := s.now()
return signWithTime(msg, now)
}
```
|
|
```go
// sign_test.go
func TestSign(t *testing.T) {
oldTimeNow := _timeNow
_timeNow = func() time.Time {
return someFixedTime
}
defer func() { _timeNow = oldTimeNow }()
assert.Equal(t, want, sign(give))
}
```
|
```go
// sign_test.go
func TestSigner(t *testing.T) {
s := newSigner()
s.now = func() time.Time {
return someFixedTime
}
assert.Equal(t, want, s.Sign(give))
}
```
|
### Avoid Embedding Types in Public Structs
These embedded types leak implementation details, inhibit type evolution, and
obscure documentation.
Assuming you have implemented a variety of list types using a shared
`AbstractList`, avoid embedding the `AbstractList` in your concrete list
implementations.
Instead, hand-write only the methods to your concrete list that will delegate
to the abstract list.
```go
type AbstractList struct {}
// Add adds an entity to the list.
func (l *AbstractList) Add(e Entity) {
// ...
}
// Remove removes an entity from the list.
func (l *AbstractList) Remove(e Entity) {
// ...
}
```
| Bad | Good |
|---|
|
```go
// ConcreteList is a list of entities.
type ConcreteList struct {
*AbstractList
}
```
|
```go
// ConcreteList is a list of entities.
type ConcreteList struct {
list *AbstractList
}
// Add adds an entity to the list.
func (l *ConcreteList) Add(e Entity) {
return l.list.Add(e)
}
// Remove removes an entity from the list.
func (l *ConcreteList) Remove(e Entity) {
return l.list.Remove(e)
}
```
|
Go allows [type embedding] as a compromise between inheritance and composition.
The outer type gets implicit copies of the embedded type's methods.
These methods, by default, delegate to the same method of the embedded
instance.
[type embedding]: https://golang.org/doc/effective_go.html#embedding
The struct also gains a field by the same name as the type.
So, if the embedded type is public, the field is public.
To maintain backward compatibility, every future version of the outer type must
keep the embedded type.
An embedded type is rarely necessary.
It is a convenience that helps you avoid writing tedious delegate methods.
Even embedding a compatible AbstractList _interface_, instead of the struct,
would offer the developer more flexibility to change in the future, but still
leak the detail that the concrete lists use an abstract implementation.
| Bad | Good |
|---|
|
```go
// AbstractList is a generalized implementation
// for various kinds of lists of entities.
type AbstractList interface {
Add(Entity)
Remove(Entity)
}
// ConcreteList is a list of entities.
type ConcreteList struct {
AbstractList
}
```
|
```go
// ConcreteList is a list of entities.
type ConcreteList struct {
list *AbstractList
}
// Add adds an entity to the list.
func (l *ConcreteList) Add(e Entity) {
return l.list.Add(e)
}
// Remove removes an entity from the list.
func (l *ConcreteList) Remove(e Entity) {
return l.list.Remove(e)
}
```
|
Either with an embedded struct or an embedded interface, the embedded type
places limits on the evolution of the type.
- Adding methods to an embedded interface is a breaking change.
- Removing methods from an embedded struct is a breaking change.
- Removing the embedded type is a breaking change.
- Replacing the embedded type, even with an alternative that satisfies the same
interface, is a breaking change.
Although writing these delegate methods is tedious, the additional effort hides
an implementation detail, leaves more opportunities for change, and also
eliminates indirection for discovering the full List interface in
documentation.
### Avoid Using Built-In Names
The Go [language specification] outlines several built-in,
[predeclared identifiers] that should not be used as names within Go programs.
Depending on context, reusing these identifiers as names will either shadow
the original within the current lexical scope (and any nested scopes) or make
affected code confusing. In the best case, the compiler will complain; in the
worst case, such code may introduce latent, hard-to-grep bugs.
[language specification]: https://golang.org/ref/spec
[predeclared identifiers]: https://golang.org/ref/spec#Predeclared_identifiers
| Bad | Good |
|---|
|
```go
var error string
// `error` shadows the builtin
// or
func handleErrorMessage(error string) {
// `error` shadows the builtin
}
```
|
```go
var errorMessage string
// `error` refers to the builtin
// or
func handleErrorMessage(msg string) {
// `error` refers to the builtin
}
```
|
|
```go
type Foo struct {
// While these fields technically don't
// constitute shadowing, grepping for
// `error` or `string` strings is now
// ambiguous.
error error
string string
}
func (f Foo) Error() error {
// `error` and `f.error` are
// visually similar
return f.error
}
func (f Foo) String() string {
// `string` and `f.string` are
// visually similar
return f.string
}
```
|
```go
type Foo struct {
// `error` and `string` strings are
// now unambiguous.
err error
str string
}
func (f Foo) Error() error {
return f.err
}
func (f Foo) String() string {
return f.str
}
```
|
Note that the compiler will not generate errors when using predeclared
identifiers, but tools such as `go vet` should correctly point out these and
other cases of shadowing.
### Avoid `init()`
Avoid `init()` where possible. When `init()` is unavoidable or desirable, code
should attempt to:
1. Be completely deterministic, regardless of program environment or invocation.
2. Avoid depending on the ordering or side-effects of other `init()` functions.
While `init()` ordering is well-known, code can change, and thus
relationships between `init()` functions can make code brittle and
error-prone.
3. Avoid accessing or manipulating global or environment state, such as machine
information, environment variables, working directory, program
arguments/inputs, etc.
4. Avoid I/O, including both filesystem, network, and system calls.
Code that cannot satisfy these requirements likely belongs as a helper to be
called as part of `main()` (or elsewhere in a program's lifecycle), or be
written as part of `main()` itself. In particular, libraries that are intended
to be used by other programs should take special care to be completely
deterministic and not perform "init magic".
| Bad | Good |
|---|
|
```go
type Foo struct {
// ...
}
var _defaultFoo Foo
func init() {
_defaultFoo = Foo{
// ...
}
}
```
|
```go
var _defaultFoo = Foo{
// ...
}
// or, better, for testability:
var _defaultFoo = defaultFoo()
func defaultFoo() Foo {
return Foo{
// ...
}
}
```
|
|
```go
type Config struct {
// ...
}
var _config Config
func init() {
// Bad: based on current directory
cwd, _ := os.Getwd()
// Bad: I/O
raw, _ := ioutil.ReadFile(
path.Join(cwd, "config", "config.yaml"),
)
yaml.Unmarshal(raw, &_config)
}
```
|
```go
type Config struct {
// ...
}
func loadConfig() Config {
cwd, err := os.Getwd()
// handle err
raw, err := ioutil.ReadFile(
path.Join(cwd, "config", "config.yaml"),
)
// handle err
var config Config
yaml.Unmarshal(raw, &config)
return config
}
```
|
Considering the above, some situations in which `init()` may be preferable or
necessary might include:
- Complex expressions that cannot be represented as single assignments.
- Pluggable hooks, such as `database/sql` dialects, encoding type registries, etc.
- Optimizations to [Google Cloud Functions] and other forms of deterministic
precomputation.
[google cloud functions]: https://cloud.google.com/functions/docs/bestpractices/tips#use_global_variables_to_reuse_objects_in_future_invocations
## Performance
Performance-specific guidelines apply only to the hot path.
### Prefer strconv over fmt
When converting primitives to/from strings, `strconv` is faster than
`fmt`.
| Bad | Good |
|---|
|
```go
for i := 0; i < b.N; i++ {
s := fmt.Sprint(rand.Int())
}
```
|
```go
for i := 0; i < b.N; i++ {
s := strconv.Itoa(rand.Int())
}
```
|
|
```
BenchmarkFmtSprint-4 143 ns/op 2 allocs/op
```
|
```
BenchmarkStrconv-4 64.2 ns/op 1 allocs/op
```
|
### Avoid string-to-byte conversion
Do not create byte slices from a fixed string repeatedly. Instead, perform the
conversion once and capture the result.
| Bad | Good |
|---|
|
```go
for i := 0; i < b.N; i++ {
w.Write([]byte("Hello world"))
}
```
|
```go
data := []byte("Hello world")
for i := 0; i < b.N; i++ {
w.Write(data)
}
```
</tr>
|
|
```
BenchmarkBad-4 50000000 22.2 ns/op
```
|
```
BenchmarkGood-4 500000000 3.25 ns/op
```
|
</tbody></table>
### Prefer Specifying Container Capacity
Specify container capacity where possible in order to allocate memory for the
container up front. This minimizes subsequent allocations (by copying and
resizing of the container) as elements are added.
#### Specifying Map Capacity Hints
Where possible, provide capacity hints when initializing
maps with `make()`.
```go
make(map[T1]T2, hint)
```
Providing a capacity hint to `make()` tries to right-size the
map at initialization time, which reduces the need for growing
the map and allocations as elements are added to the map.
Note that, unlike slices, map capacity hints do not guarantee complete,
preemptive allocation, but are used to approximate the number of hashmap buckets
required. Consequently, allocations may still occur when adding elements to the
map, even up to the specified capacity.
| Bad | Good |
|---|
|
```go
m := make(map[string]os.FileInfo)
files, _ := ioutil.ReadDir("./files")
for _, f := range files {
m[f.Name()] = f
}
```
|
```go
files, _ := ioutil.ReadDir("./files")
m := make(map[string]os.FileInfo, len(files))
for _, f := range files {
m[f.Name()] = f
}
```
|
|
`m` is created without a size hint; there may be more
allocations at assignment time.
|
`m` is created with a size hint; there may be fewer
allocations at assignment time.
|
#### Specifying Slice Capacity
Where possible, provide capacity hints when initializing slices with `make()`,
particularly when appending.
```go
make([]T, length, capacity)
```
Unlike maps, slice capacity is not a hint: the compiler will allocate enough
memory for the capacity of the slice as provided to `make()`, which means that
subsequent `append()` operations will incur zero allocations (until the length
of the slice matches the capacity, after which any appends will require a resize
to hold additional elements).
| Bad | Good |
|---|
|
```go
for n := 0; n < b.N; n++ {
data := make([]int, 0)
for k := 0; k < size; k++{
data = append(data, k)
}
}
```
|
```go
for n := 0; n < b.N; n++ {
data := make([]int, 0, size)
for k := 0; k < size; k++{
data = append(data, k)
}
}
```
|
|
```
BenchmarkBad-4 100000000 2.48s
```
|
```
BenchmarkGood-4 100000000 0.21s
```
|
## Style
### Be Consistent
Some of the guidelines outlined in this document can be evaluated objectively;
others are situational, contextual, or subjective.
Above all else, **be consistent**.
Consistent code is easier to maintain, is easier to rationalize, requires less
cognitive overhead, and is easier to migrate or update as new conventions emerge
or classes of bugs are fixed.
Conversely, having multiple disparate or conflicting styles within a single
codebase causes maintenance overhead, uncertainty, and cognitive dissonance,
all of which can directly contribute to lower velocity, painful code reviews,
and bugs.
When applying these guidelines to a codebase, it is recommended that changes
are made at a package (or larger) level: application at a sub-package level
violates the above concern by introducing multiple styles into the same code.
### Group Similar Declarations
Go supports grouping similar declarations.
| Bad | Good |
|---|
|
```go
import "a"
import "b"
```
|
```go
import (
"a"
"b"
)
```
|
This also applies to constants, variables, and type declarations.
| Bad | Good |
|---|
|
```go
const a = 1
const b = 2
var a = 1
var b = 2
type Area float64
type Volume float64
```
|
```go
const (
a = 1
b = 2
)
var (
a = 1
b = 2
)
type (
Area float64
Volume float64
)
```
|
Only group related declarations. Do not group declarations that are unrelated.
| Bad | Good |
|---|
|
```go
type Operation int
const (
Add Operation = iota + 1
Subtract
Multiply
ENV_VAR = "MY_ENV"
)
```
|
```go
type Operation int
const (
Add Operation = iota + 1
Subtract
Multiply
)
const ENV_VAR = "MY_ENV"
```
|
Groups are not limited in where they can be used. For example, you can use them
inside of functions.
| Bad | Good |
|---|
|
```go
func f() string {
var red = color.New(0xff0000)
var green = color.New(0x00ff00)
var blue = color.New(0x0000ff)
...
}
```
|
```go
func f() string {
var (
red = color.New(0xff0000)
green = color.New(0x00ff00)
blue = color.New(0x0000ff)
)
...
}
```
|
### Import Group Ordering
There should be two import groups:
- Standard library
- Everything else
This is the grouping applied by goimports by default.
| Bad | Good |
|---|
|
```go
import (
"fmt"
"os"
"go.uber.org/atomic"
"golang.org/x/sync/errgroup"
)
```
|
```go
import (
"fmt"
"os"
"go.uber.org/atomic"
"golang.org/x/sync/errgroup"
)
```
|
### Package Names
When naming packages, choose a name that is:
- All lower-case. No capitals or underscores.
- Does not need to be renamed using named imports at most call sites.
- Short and succinct. Remember that the name is identified in full at every call
site.
- Not plural. For example, `net/url`, not `net/urls`.
- Not "common", "util", "shared", or "lib". These are bad, uninformative names.
See also [Package Names] and [Style guideline for Go packages].
[package names]: https://blog.golang.org/package-names
[style guideline for go packages]: https://rakyll.org/style-packages/
### Function Names
We follow the Go community's convention of using [MixedCaps for function
names]. An exception is made for test functions, which may contain underscores
for the purpose of grouping related test cases, e.g.,
`TestMyFunction_WhatIsBeingTested`.
[mixedcaps for function names]: https://golang.org/doc/effective_go.html#mixed-caps
### Import Aliasing
Import aliasing must be used if the package name does not match the last
element of the import path.
```go
import (
"net/http"
client "example.com/client-go"
trace "example.com/trace/v2"
)
```
In all other scenarios, import aliases should be avoided unless there is a
direct conflict between imports.
| Bad | Good |
|---|
|
```go
import (
"fmt"
"os"
nettrace "golang.net/x/trace"
)
```
|
```go
import (
"fmt"
"os"
"runtime/trace"
nettrace "golang.net/x/trace"
)
```
|
### Function Grouping and Ordering
- Functions should be sorted in rough call order.
- Functions in a file should be grouped by receiver.
Therefore, exported functions should appear first in a file, after
`struct`, `const`, `var` definitions.
A `newXYZ()`/`NewXYZ()` may appear after the type is defined, but before the
rest of the methods on the receiver.
Since functions are grouped by receiver, plain utility functions should appear
towards the end of the file.
| Bad | Good |
|---|
|
```go
func (s *something) Cost() {
return calcCost(s.weights)
}
type something struct{ ... }
func calcCost(n []int) int {...}
func (s *something) Stop() {...}
func newSomething() *something {
return &something{}
}
```
|
```go
type something struct{ ... }
func newSomething() *something {
return &something{}
}
func (s *something) Cost() {
return calcCost(s.weights)
}
func (s *something) Stop() {...}
func calcCost(n []int) int {...}
```
|
### Reduce Nesting
Code should reduce nesting where possible by handling error cases/special
conditions first and returning early or continuing the loop. Reduce the amount
of code that is nested multiple levels.
| Bad | Good |
|---|
|
```go
for _, v := range data {
if v.F1 == 1 {
v = process(v)
if err := v.Call(); err == nil {
v.Send()
} else {
return err
}
} else {
log.Printf("Invalid v: %v", v)
}
}
```
|
```go
for _, v := range data {
if v.F1 != 1 {
log.Printf("Invalid v: %v", v)
continue
}
v = process(v)
if err := v.Call(); err != nil {
return err
}
v.Send()
}
```
|
### Unnecessary Else
If a variable is set in both branches of an if, it can be replaced with a
single if.
| Bad | Good |
|---|
|
```go
var a int
if b {
a = 100
} else {
a = 10
}
```
|
```go
a := 10
if b {
a = 100
}
```
|
### Top-level Variable Declarations
At the top level, use the standard `var` keyword. Do not specify the type,
unless it is not the same type as the expression.
| Bad | Good |
|---|
|
```go
var _s string = F()
func F() string { return "A" }
```
|
```go
var _s = F()
// Since F already states that it returns a string, we don't need to specify
// the type again.
func F() string { return "A" }
```
|
Specify the type if the type of the expression does not match the desired type
exactly.
```go
type myError struct{}
func (myError) Error() string { return "error" }
func F() myError { return myError{} }
var _e error = F()
// F returns an object of type myError but we want error.
```
### Prefix Unexported Globals with \_
Prefix unexported top-level `var`s and `const`s with `_` to make it clear when
they are used that they are global symbols.
Exception: Unexported error values, which should be prefixed with `err`.
Rationale: Top-level variables and constants have a package scope. Using a
generic name makes it easy to accidentally use the wrong value in a different
file.
| Bad | Good |
|---|
|
```go
// foo.go
const (
defaultPort = 8080
defaultUser = "user"
)
// bar.go
func Bar() {
defaultPort := 9090
...
fmt.Println("Default port", defaultPort)
// We will not see a compile error if the first line of
// Bar() is deleted.
}
```
|
```go
// foo.go
const (
_defaultPort = 8080
_defaultUser = "user"
)
```
|
### Embedding in Structs
Embedded types (such as mutexes) should be at the top of the field list of a
struct, and there must be an empty line separating embedded fields from regular
fields.
| Bad | Good |
|---|
|
```go
type Client struct {
version int
http.Client
}
```
|
```go
type Client struct {
http.Client
version int
}
```
|
Embedding should provide tangible benefit, like adding or augmenting
functionality in a semantically-appropriate way. It should do this with zero
adverse user-facing effects (see also: [Avoid Embedding Types in Public Structs]).
[avoid embedding types in public structs]: #avoid-embedding-types-in-public-structs
Embedding **should not**:
- Be purely cosmetic or convenience-oriented.
- Make outer types more difficult to construct or use.
- Affect outer types' zero values. If the outer type has a useful zero value, it
should still have a useful zero value after embedding the inner type.
- Expose unrelated functions or fields from the outer type as a side-effect of
embedding the inner type.
- Expose unexported types.
- Affect outer types' copy semantics.
- Change the outer type's API or type semantics.
- Embed a non-canonical form of the inner type.
- Expose implementation details of the outer type.
- Allow users to observe or control type internals.
- Change the general behavior of inner functions through wrapping in a way that
would reasonably surprise users.
Simply put, embed consciously and intentionally. A good litmus test is, "would
all of these exported inner methods/fields be added directly to the outer type";
if the answer is "some" or "no", don't embed the inner type - use a field
instead.
| Bad | Good |
|---|
|
```go
type A struct {
// Bad: A.Lock() and A.Unlock() are
// now available, provide no
// functional benefit, and allow
// users to control details about
// the internals of A.
sync.Mutex
}
```
|
```go
type countingWriteCloser struct {
// Good: Write() is provided at this
// outer layer for a specific
// purpose, and delegates work
// to the inner type's Write().
io.WriteCloser
count int
}
func (w *countingWriteCloser) Write(bs []byte) (int, error) {
w.count += len(bs)
return w.WriteCloser.Write(bs)
}
```
|
|
```go
type Book struct {
// Bad: pointer changes zero value usefulness
io.ReadWriter
// other fields
}
// later
var b Book
b.Read(...) // panic: nil pointer
b.String() // panic: nil pointer
b.Write(...) // panic: nil pointer
```
|
```go
type Book struct {
// Good: has useful zero value
bytes.Buffer
// other fields
}
// later
var b Book
b.Read(...) // ok
b.String() // ok
b.Write(...) // ok
```
|
|
```go
type Client struct {
sync.Mutex
sync.WaitGroup
bytes.Buffer
url.URL
}
```
|
```go
type Client struct {
mtx sync.Mutex
wg sync.WaitGroup
buf bytes.Buffer
url url.URL
}
```
|
### Use Field Names to Initialize Structs
You should almost always specify field names when initializing structs. This is
now enforced by [`go vet`].
[`go vet`]: https://golang.org/cmd/vet/
| Bad | Good |
|---|
|
```go
k := User{"John", "Doe", true}
```
|
```go
k := User{
FirstName: "John",
LastName: "Doe",
Admin: true,
}
```
|
Exception: Field names _may_ be omitted in test tables when there are 3 or
fewer fields.
```go
tests := []struct{
op Operation
want string
}{
{Add, "add"},
{Subtract, "subtract"},
}
```
### Local Variable Declarations
Short variable declarations (`:=`) should be used if a variable is being set to
some value explicitly.
| Bad | Good |
|---|
|
```go
var s = "foo"
```
|
```go
s := "foo"
```
|
However, there are cases where the default value is clearer when the `var`
keyword is used. [Declaring Empty Slices], for example.
[declaring empty slices]: https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices
| Bad | Good |
|---|
|
```go
func f(list []int) {
filtered := []int{}
for _, v := range list {
if v > 10 {
filtered = append(filtered, v)
}
}
}
```
|
```go
func f(list []int) {
var filtered []int
for _, v := range list {
if v > 10 {
filtered = append(filtered, v)
}
}
}
```
|
### nil is a valid slice
`nil` is a valid slice of length 0. This means that,
- You should not return a slice of length zero explicitly. Return `nil`
instead.
| Bad | Good |
|---|
|
```go
if x == "" {
return []int{}
}
```
|
```go
if x == "" {
return nil
}
```
|
- To check if a slice is empty, always use `len(s) == 0`. Do not check for
`nil`.
| Bad | Good |
|---|
|
```go
func isEmpty(s []string) bool {
return s == nil
}
```
|
```go
func isEmpty(s []string) bool {
return len(s) == 0
}
```
|
- The zero value (a slice declared with `var`) is usable immediately without
`make()`.
| Bad | Good |
|---|
|
```go
nums := []int{}
// or, nums := make([]int)
if add1 {
nums = append(nums, 1)
}
if add2 {
nums = append(nums, 2)
}
```
|
```go
var nums []int
if add1 {
nums = append(nums, 1)
}
if add2 {
nums = append(nums, 2)
}
```
|
Remember that, while it is a valid slice, a nil slice is not equivalent to an
allocated slice of length 0 - one is nil and the other is not - and the two may
be treated differently in different situations (such as serialization).
### Reduce Scope of Variables
Where possible, reduce scope of variables. Do not reduce the scope if it
conflicts with [Reduce Nesting](#reduce-nesting).
| Bad | Good |
|---|
|
```go
err := ioutil.WriteFile(name, data, 0644)
if err != nil {
return err
}
```
|
```go
if err := ioutil.WriteFile(name, data, 0644); err != nil {
return err
}
```
|
If you need a result of a function call outside of the if, then you should not
try to reduce the scope.
| Bad | Good |
|---|
|
```go
if data, err := ioutil.ReadFile(name); err == nil {
err = cfg.Decode(data)
if err != nil {
return err
}
fmt.Println(cfg)
return nil
} else {
return err
}
```
|
```go
data, err := ioutil.ReadFile(name)
if err != nil {
return err
}
if err := cfg.Decode(data); err != nil {
return err
}
fmt.Println(cfg)
return nil
```
|
### Avoid Naked Parameters
Naked parameters in function calls can hurt readability. Add C-style comments
(`/* ... */`) for parameter names when their meaning is not obvious.
| Bad | Good |
|---|
|
```go
// func printInfo(name string, isLocal, done bool)
printInfo("foo", true, true)
```
|
```go
// func printInfo(name string, isLocal, done bool)
printInfo("foo", true /* isLocal */, true /* done */)
```
|
Better yet, replace naked `bool` types with custom types for more readable and
type-safe code. This allows more than just two states (true/false) for that
parameter in the future.
```go
type Region int
const (
UnknownRegion Region = iota
Local
)
type Status int
const (
StatusReady Status = iota + 1
StatusDone
// Maybe we will have a StatusInProgress in the future.
)
func printInfo(name string, region Region, status Status)
```
### Use Raw String Literals to Avoid Escaping
Go supports [raw string literals](https://golang.org/ref/spec#raw_string_lit),
which can span multiple lines and include quotes. Use these to avoid
hand-escaped strings which are much harder to read.
| Bad | Good |
|---|
|
```go
wantError := "unknown name:\"test\""
```
|
```go
wantError := `unknown error:"test"`
```
|
### Initializing Struct References
Use `&T{}` instead of `new(T)` when initializing struct references so that it
is consistent with the struct initialization.
| Bad | Good |
|---|
|
```go
sval := T{Name: "foo"}
// inconsistent
sptr := new(T)
sptr.Name = "bar"
```
|
```go
sval := T{Name: "foo"}
sptr := &T{Name: "bar"}
```
|
### Initializing Maps
Prefer `make(..)` for empty maps, and maps populated
programmatically. This makes map initialization visually
distinct from declaration, and it makes it easy to add size
hints later if available.
| Bad | Good |
|---|
|
```go
var (
// m1 is safe to read and write;
// m2 will panic on writes.
m1 = map[T1]T2{}
m2 map[T1]T2
)
```
|
```go
var (
// m1 is safe to read and write;
// m2 will panic on writes.
m1 = make(map[T1]T2)
m2 map[T1]T2
)
```
|
|
Declaration and initialization are visually similar.
|
Declaration and initialization are visually distinct.
|
Where possible, provide capacity hints when initializing
maps with `make()`. See
[Specifying Map Capacity Hints](#specifying-map-capacity-hints)
for more information.
On the other hand, if the map holds a fixed list of elements,
use map literals to initialize the map.
| Bad | Good |
|---|
|
```go
m := make(map[T1]T2, 3)
m[k1] = v1
m[k2] = v2
m[k3] = v3
```
|
```go
m := map[T1]T2{
k1: v1,
k2: v2,
k3: v3,
}
```
|
The basic rule of thumb is to use map literals when adding a fixed set of
elements at initialization time, otherwise use `make` (and specify a size hint
if available).
### Format Strings outside Printf
If you declare format strings for `Printf`-style functions outside a string
literal, make them `const` values.
This helps `go vet` perform static analysis of the format string.
| Bad | Good |
|---|
|
```go
msg := "unexpected values %v, %v\n"
fmt.Printf(msg, 1, 2)
```
|
```go
const msg = "unexpected values %v, %v\n"
fmt.Printf(msg, 1, 2)
```
|
### Naming Printf-style Functions
When you declare a `Printf`-style function, make sure that `go vet` can detect
it and check the format string.
This means that you should use predefined `Printf`-style function
names if possible. `go vet` will check these by default. See [Printf family]
for more information.
[printf family]: https://golang.org/cmd/vet/#hdr-Printf_family
If using the predefined names is not an option, end the name you choose with
f: `Wrapf`, not `Wrap`. `go vet` can be asked to check specific `Printf`-style
names but they must end with f.
```shell
$ go vet -printfuncs=wrapf,statusf
```
See also [go vet: Printf family check].
[go vet: printf family check]: https://kuzminva.wordpress.com/2017/11/07/go-vet-printf-family-check/
## Patterns
### Test Tables
Use table-driven tests with [subtests] to avoid duplicating code when the core
test logic is repetitive.
[subtests]: https://blog.golang.org/subtests
| Bad | Good |
|---|
|
```go
// func TestSplitHostPort(t *testing.T)
host, port, err := net.SplitHostPort("192.0.2.0:8000")
require.NoError(t, err)
assert.Equal(t, "192.0.2.0", host)
assert.Equal(t, "8000", port)
host, port, err = net.SplitHostPort("192.0.2.0:http")
require.NoError(t, err)
assert.Equal(t, "192.0.2.0", host)
assert.Equal(t, "http", port)
host, port, err = net.SplitHostPort(":8000")
require.NoError(t, err)
assert.Equal(t, "", host)
assert.Equal(t, "8000", port)
host, port, err = net.SplitHostPort("1:8")
require.NoError(t, err)
assert.Equal(t, "1", host)
assert.Equal(t, "8", port)
```
|
```go
// func TestSplitHostPort(t *testing.T)
tests := []struct{
give string
wantHost string
wantPort string
}{
{
give: "192.0.2.0:8000",
wantHost: "192.0.2.0",
wantPort: "8000",
},
{
give: "192.0.2.0:http",
wantHost: "192.0.2.0",
wantPort: "http",
},
{
give: ":8000",
wantHost: "",
wantPort: "8000",
},
{
give: "1:8",
wantHost: "1",
wantPort: "8",
},
}
for _, tt := range tests {
t.Run(tt.give, func(t *testing.T) {
host, port, err := net.SplitHostPort(tt.give)
require.NoError(t, err)
assert.Equal(t, tt.wantHost, host)
assert.Equal(t, tt.wantPort, port)
})
}
```
|
Test tables make it easier to add context to error messages, reduce duplicate
logic, and add new test cases.
We follow the convention that the slice of structs is referred to as `tests`
and each test case `tt`. Further, we encourage explicating the input and output
values for each test case with `give` and `want` prefixes.
```go
tests := []struct{
give string
wantHost string
wantPort string
}{
// ...
}
for _, tt := range tests {
// ...
}
```
### Functional Options
Functional options is a pattern in which you declare an opaque `Option` type
that records information in some internal struct. You accept a variadic number
of these options and act upon the full information recorded by the options on
the internal struct.
Use this pattern for optional arguments in constructors and other public APIs
that you foresee needing to expand, especially if you already have three or
more arguments on those functions.
| Bad | Good |
|---|
|
```go
// package db
func Open(
addr string,
cache bool,
logger *zap.Logger
) (*Connection, error) {
// ...
}
```
|
```go
// package db
type Option interface {
// ...
}
func WithCache(c bool) Option {
// ...
}
func WithLogger(log *zap.Logger) Option {
// ...
}
// Open creates a connection.
func Open(
addr string,
opts ...Option,
) (*Connection, error) {
// ...
}
```
|
|
The cache and logger parameters must always be provided, even if the user
wants to use the default.
```go
db.Open(addr, db.DefaultCache, zap.NewNop())
db.Open(addr, db.DefaultCache, log)
db.Open(addr, false /* cache */, zap.NewNop())
db.Open(addr, false /* cache */, log)
```
|
Options are provided only if needed.
```go
db.Open(addr)
db.Open(addr, db.WithLogger(log))
db.Open(addr, db.WithCache(false))
db.Open(
addr,
db.WithCache(false),
db.WithLogger(log),
)
```
|
Our suggested way of implementing this pattern is with an `Option` interface
that holds an unexported method, recording options on an unexported `options`
struct.
```go
type options struct {
cache bool
logger *zap.Logger
}
type Option interface {
apply(*options)
}
type cacheOption bool
func (c cacheOption) apply(opts *options) {
opts.cache = bool(c)
}
func WithCache(c bool) Option {
return cacheOption(c)
}
type loggerOption struct {
Log *zap.Logger
}
func (l loggerOption) apply(opts *options) {
opts.logger = l.Log
}
func WithLogger(log *zap.Logger) Option {
return loggerOption{Log: log}
}
// Open creates a connection.
func Open(
addr string,
opts ...Option,
) (*Connection, error) {
options := options{
cache: defaultCache,
logger: zap.NewNop(),
}
for _, o := range opts {
o.apply(&options)
}
// ...
}
```
Note that there's a method of implementing this pattern with closures but we
believe that the pattern above provides more flexibility for authors and is
easier to debug and test for users. In particular, it allows options to be
compared against each other in tests and mocks, versus closures where this is
impossible. Further, it lets options implement other interfaces, including
`fmt.Stringer` which allows for user-readable string representations of the
options.
See also,
- [Self-referential functions and the design of options]
- [Functional options for friendly APIs]
[self-referential functions and the design of options]: https://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html
[functional options for friendly apis]: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis