2182 lines
42 KiB
Markdown
2182 lines
42 KiB
Markdown
# uber-go-style-guide-kr
|
|
|
|
Translated in Korean
|
|
|
|
Currently WIP, but translation will done by 20th of Oct, 2019
|
|
|
|
---
|
|
|
|
<!--
|
|
|
|
Editing this document:
|
|
|
|
- Discuss all changes in GitHub issues first.
|
|
- Update the table of contents as new sections are added or removed.
|
|
- Use tables for side-by-side code samples. See below.
|
|
|
|
Code Samples:
|
|
|
|
Use 2 spaces to indent. Horizontal real estate is important in side-by-side
|
|
samples.
|
|
|
|
For side-by-side code samples, use the following snippet.
|
|
|
|
~~~
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
BAD CODE GOES HERE
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
GOOD CODE GOES HERE
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
~~~
|
|
|
|
(You need the empty lines between the <td> and code samples for it to be
|
|
treated as Markdown.)
|
|
|
|
If you need to add labels or descriptions below the code samples, add another
|
|
row before the </tbody></table> line.
|
|
|
|
~~~
|
|
<tr>
|
|
<td>DESCRIBE BAD CODE</td>
|
|
<td>DESCRIBE GOOD CODE</td>
|
|
</tr>
|
|
~~~
|
|
|
|
-->
|
|
|
|
# Uber의 Go언어 스타일 가이드 (Uber's Go Style Guide)
|
|
|
|
- [uber-go-style-guide-kr](#uber-go-style-guide-kr)
|
|
- [Uber의 Go언어 스타일 가이드 (Uber's Go Style Guide)](#uber%ec%9d%98-go%ec%96%b8%ec%96%b4-%ec%8a%a4%ed%83%80%ec%9d%bc-%ea%b0%80%ec%9d%b4%eb%93%9c-ubers-go-style-guide)
|
|
- [소개 (Introduction)](#%ec%86%8c%ea%b0%9c-introduction)
|
|
- [가이드라인 (Guidelines)](#%ea%b0%80%ec%9d%b4%eb%93%9c%eb%9d%bc%ec%9d%b8-guidelines)
|
|
- [인터페이스에 대한 포인터 (Pointers to Interfaces)](#%ec%9d%b8%ed%84%b0%ed%8e%98%ec%9d%b4%ec%8a%a4%ec%97%90-%eb%8c%80%ed%95%9c-%ed%8f%ac%ec%9d%b8%ed%84%b0-pointers-to-interfaces)
|
|
- [수신자(Receivers)와 인터페이스(Interfaces)](#%ec%88%98%ec%8b%a0%ec%9e%90receivers%ec%99%80-%ec%9d%b8%ed%84%b0%ed%8e%98%ec%9d%b4%ec%8a%a4interfaces)
|
|
- [제로 값 뮤텍스(Zero-value Mutexes)는 유효하다](#%ec%a0%9c%eb%a1%9c-%ea%b0%92-%eb%ae%a4%ed%85%8d%ec%8a%a4zero-value-mutexes%eb%8a%94-%ec%9c%a0%ed%9a%a8%ed%95%98%eb%8b%a4)
|
|
- [Copy Slices and Maps at Boundaries](#copy-slices-and-maps-at-boundaries)
|
|
- [Receiving Slices and Maps](#receiving-slices-and-maps)
|
|
- [Returning Slices and Maps](#returning-slices-and-maps)
|
|
- [Defer to Clean Up](#defer-to-clean-up)
|
|
- [Channel Size is One or None](#channel-size-is-one-or-none)
|
|
- [Start Enums at One](#start-enums-at-one)
|
|
- [Error Types](#error-types)
|
|
- [Error Wrapping](#error-wrapping)
|
|
- [Handle Type Assertion Failures](#handle-type-assertion-failures)
|
|
- [Don't Panic](#dont-panic)
|
|
- [Use go.uber.org/atomic](#use-gouberorgatomic)
|
|
- [Performance](#performance)
|
|
- [Prefer strconv over fmt](#prefer-strconv-over-fmt)
|
|
- [Avoid string-to-byte conversion](#avoid-string-to-byte-conversion)
|
|
- [Style](#style)
|
|
- [Group Similar Declarations](#group-similar-declarations)
|
|
- [Import Group Ordering](#import-group-ordering)
|
|
- [Package Names](#package-names)
|
|
- [Function Names](#function-names)
|
|
- [Import Aliasing](#import-aliasing)
|
|
- [Function Grouping and Ordering](#function-grouping-and-ordering)
|
|
- [Reduce Nesting](#reduce-nesting)
|
|
- [Unnecessary Else](#unnecessary-else)
|
|
- [Top-level Variable Declarations](#top-level-variable-declarations)
|
|
- [Prefix Unexported Globals with _](#prefix-unexported-globals-with)
|
|
- [Embedding in Structs](#embedding-in-structs)
|
|
- [Use Field Names to initialize Structs](#use-field-names-to-initialize-structs)
|
|
- [Local Variable Declarations](#local-variable-declarations)
|
|
- [nil is a valid slice](#nil-is-a-valid-slice)
|
|
- [Reduce Scope of Variables](#reduce-scope-of-variables)
|
|
- [Avoid Naked Parameters](#avoid-naked-parameters)
|
|
- [Use Raw String Literals to Avoid Escaping](#use-raw-string-literals-to-avoid-escaping)
|
|
- [Initializing Struct References](#initializing-struct-references)
|
|
- [Format Strings outside Printf](#format-strings-outside-printf)
|
|
- [Naming Printf-style Functions](#naming-printf-style-functions)
|
|
- [Patterns](#patterns)
|
|
- [Test Tables](#test-tables)
|
|
- [Functional Options](#functional-options)
|
|
|
|
## 소개 (Introduction)
|
|
|
|
스타일은 코드를 통제하는(govern) 관습이다. 이러한 관습(convention)은 소스파일 포맷팅 (e.g. gofmt)보다 더 많은 영역을 다루기(cover) 때문에, "스타일" 이라는 단어 자체가 약간 부적절 할 수 있다.
|
|
|
|
본 가이드의 목표는 Uber에서 Go 코드를 작성할 때 해야 할 것과 하지 말아야 할 것 (Dos and Don'ts)에 대하여 자세하게 설명하여 이러한 복잡성을 관리하는 것이다. 이런 규칙들은 엔지니어들이 Go 언어의 특성을(feature) 생산적으로개계속 사용할 수 있도록 코드 베이스를 관리가능하게 유지하기위해 존재한다.
|
|
|
|
이 가이드는 원래 [Prashant Varanasi]와 [Simon Newton]이 동료들에게 Go를 사용하면서 개발속도 향상을 도모하기 위해 소개되었다. 또한, 수 년에 거쳐서 다른 사람들로부터의 피드백을 통해서 개정되 오고 있다.
|
|
|
|
[Prashant Varanasi]: https://github.com/prashantv
|
|
[Simon Newton]: https://github.com/nomis52
|
|
|
|
이 문서는 Uber에서의 엔지니어들이 지향하는 Go언어 코드의 관용적 규칙을 설명한다. 상당 수의 규칙들은 Go언어에 대한 일반적인 가이드라인이며, 다른 부분에 대해서는 외부 레퍼런스에 의해 확장된다 (아래 참고)
|
|
|
|
1. [Effective Go](https://golang.org/doc/effective_go.html)
|
|
2. [The Go common mistakes guide](https://github.com/golang/go/wiki/CodeReviewComments)
|
|
|
|
모든 코드는 `golint` 와 `go vet`를 실행할 때 에러가 없어야 한다. 또한 우리는 여러분들의 에디터를 아래와 같이 설정하기를 권고한다:
|
|
|
|
- Run `goimports` on save
|
|
- Run `golint` and `go vet` to check for errors
|
|
|
|
아래의 링크를 통해서 Go 툴을 지원하는 에디터에 대한 정보를 얻을 수 있다:
|
|
<https://github.com/golang/go/wiki/IDEsAndTextEditorPlugins>
|
|
|
|
## 가이드라인 (Guidelines)
|
|
|
|
### 인터페이스에 대한 포인터 (Pointers to Interfaces)
|
|
|
|
일반적으로 인터페이스에 대한 포인터는 거의 필요하지 않을 것이다. 여러분들은 인터페이스를 값(value)으로서 전달(passing)해야 할 것이며, 인터페이스에 대한 기본 데이터(underlying data)는 여전히 포인터가 될 수 있다.
|
|
|
|
한 인터페이스는 두 가지 필드이다:
|
|
|
|
1. 타입-특정 정보(type-specific information)에 대한 포인터. 여러분들을 이것을 "타입"으로 간주할 수 있다.
|
|
2. 데이터 포인터. 저장된 데이터가 포인터일 경우, 이것은 직접적으로 저장될 수 있다. 만약, 저장된 데이터가 값(value)인 경우, 값에 대한 포인터가 저장된다.
|
|
|
|
만약 여러분들이 기본 데이터(underlying data) 수정하기 위한 인터페이스 메서드 (interface methods)를 원한다면, 여러분들은 반드시 포인터를 사용해야 한다.
|
|
|
|
### 수신자(Receivers)와 인터페이스(Interfaces)
|
|
|
|
값 수신자 (value receivers)와 메서드(Methods)는 포인터 혹은 값에 의해서 호출 될 수 있다.
|
|
|
|
예를 들면,
|
|
|
|
```go
|
|
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"}}
|
|
|
|
// 오직 값만 사용하여 Read를 호출 할 수 있다.
|
|
sVals[1].Read()
|
|
|
|
// 아래 코드는 컴파일 되지 않을 것:
|
|
// sVals[1].Write("test")
|
|
|
|
sPtrs := map[int]*S{1: {"A"}}
|
|
|
|
// 포인터를 사용하여 Read와 Write 모두 호출 할 수 있다.
|
|
sPtrs[1].Read()
|
|
sPtrs[1].Write("test")
|
|
```
|
|
|
|
마찬가지로, 메서드가 값 수신자(value receiver)를 가지고 있다고 하더라도 포인터가 인터페이스를 충족시킬 수 있다.
|
|
|
|
```go
|
|
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
|
|
|
|
// s2Val이 값이고 f에 대한 수신자가 없기 때문에, 아래의 코드는 컴파일 되지 않는다.
|
|
// i = s2Val
|
|
```
|
|
|
|
Effective Go에 [Pointers vs. Values]에 대한 좋은 글이 있으니 참고하기 바란다.
|
|
|
|
[Pointers vs. Values]: https://golang.org/doc/effective_go.html#pointers_vs_values
|
|
|
|
### 제로 값 뮤텍스(Zero-value Mutexes)는 유효하다
|
|
|
|
`sync.Mutex`와 `sync.RWMutex` 의 제로 값은 유효하므로, 거의 대부분의 경우 뮤텍스에 대한 포인터는 필요로 하지 않는다.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
mu := new(sync.Mutex)
|
|
mu.Lock()
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
var mu sync.Mutex
|
|
mu.Lock()
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
포인터로 구조체를 사용할 경우, 뮤텍스는 포인터가 아닌 필드(non-pointer field)가 될 수 있다.
|
|
|
|
구조체의 필드를 보호하기 위해 뮤텍스를 사용한 수출되지 않는 구조체(unexported structs)는 뮤텍스를 포함(embed) 할 수 있다.
|
|
|
|
<table>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
type smap struct {
|
|
sync.Mutex // 오직 수출되지 않은 타입을 위해서 사용
|
|
|
|
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]
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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]
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
|
|
</tr>
|
|
<tr>
|
|
<td>뮤텍스 인터페이스를 구현해야 하는 전용 타입(private type) 혹은 타입에 포함됨. </td>
|
|
<td>수출되는 타입(exported type)에 대해서는 전용 필드 (private field)를 사용함.</td>
|
|
</tr>
|
|
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th> <th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr>
|
|
<td>
|
|
|
|
```go
|
|
func (d *Driver) SetTrips(trips []Trip) {
|
|
d.trips = trips
|
|
}
|
|
|
|
trips := ...
|
|
d1.SetTrips(trips)
|
|
|
|
// Did you mean to modify d1.trips?
|
|
trips[0] = ...
|
|
```
|
|
|
|
</td>
|
|
<td>
|
|
|
|
```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] = ...
|
|
```
|
|
|
|
</td>
|
|
</tr>
|
|
|
|
</tbody>
|
|
</table>
|
|
|
|
#### Returning Slices and Maps
|
|
|
|
Similarly, be wary of user modifications to maps or slices exposing internal
|
|
state.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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 racy.
|
|
snapshot := stats.Snapshot()
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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()
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Defer to Clean Up
|
|
|
|
Use defer to clean up resources such as files and locks.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
p.Lock()
|
|
defer p.Unlock()
|
|
|
|
if p.count < 10 {
|
|
return p.count
|
|
}
|
|
|
|
p.count++
|
|
return p.count
|
|
|
|
// more readable
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
// Ought to be enough for anybody!
|
|
c := make(chan int, 64)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
// Size of one
|
|
c := make(chan int, 1) // or
|
|
// Unbuffered channel, size of zero
|
|
c := make(chan int)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
type Operation int
|
|
|
|
const (
|
|
Add Operation = iota
|
|
Subtract
|
|
Multiply
|
|
)
|
|
|
|
// Add=0, Subtract=1, Multiply=2
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
type Operation int
|
|
|
|
const (
|
|
Add Operation = iota + 1
|
|
Subtract
|
|
Multiply
|
|
)
|
|
|
|
// Add=1, Subtract=2, Multiply=3
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
There are cases where using the zero value makes sense, for example when the
|
|
zero value case is the desirable default behavior.
|
|
|
|
```go
|
|
type LogOutput int
|
|
|
|
const (
|
|
LogToStdout LogOutput = iota
|
|
LogToFile
|
|
LogToRemote
|
|
)
|
|
|
|
// LogToStdout=0, LogToFile=1, LogToRemote=2
|
|
```
|
|
|
|
<!-- TODO: section on String methods for enums -->
|
|
|
|
### Error Types
|
|
|
|
There are various options for declaring errors:
|
|
|
|
- [`errors.New`] for errors with simple static strings
|
|
- [`fmt.Errorf`] for formatted error strings
|
|
- Custom types that implement an `Error()` method
|
|
- Wrapped errors using [`"pkg/errors".Wrap`]
|
|
|
|
When returning errors, consider the following to determine the best choice:
|
|
|
|
- Is this a simple error that needs no extra information? If so, [`errors.New`]
|
|
should suffice.
|
|
- Do the clients need to detect and handle this error? If so, you should use a
|
|
custom type, and implement the `Error()` method.
|
|
- Are you propagating an error returned by a downstream function? If so, check
|
|
the [section on error wrapping](#error-wrapping).
|
|
- Otherwise, [`fmt.Errorf`] is okay.
|
|
|
|
[`errors.New`]: https://golang.org/pkg/errors/#New
|
|
[`fmt.Errorf`]: https://golang.org/pkg/fmt/#Errorf
|
|
[`"pkg/errors".Wrap`]: https://godoc.org/github.com/pkg/errors#Wrap
|
|
|
|
If the client needs to detect the error, and you have created a simple error
|
|
using [`errors.New`], use a var for the error.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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")
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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")
|
|
}
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
func open(file string) error {
|
|
return fmt.Errorf("file %q not found", file)
|
|
}
|
|
|
|
func use() {
|
|
if err := open(); err != nil {
|
|
if strings.Contains(err.Error(), "not found") {
|
|
// handle
|
|
} else {
|
|
panic("unknown error")
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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(); err != nil {
|
|
if _, ok := err.(errNotFound); ok {
|
|
// handle
|
|
} else {
|
|
panic("unknown error")
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
```go
|
|
// 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")
|
|
}
|
|
}
|
|
```
|
|
|
|
<!-- TODO: Exposing the information to callers with accessor functions. -->
|
|
|
|
### 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:
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
s, err := store.New()
|
|
if err != nil {
|
|
return fmt.Errorf(
|
|
"failed to create new store: %s", err)
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
s, err := store.New()
|
|
if err != nil {
|
|
return fmt.Errorf(
|
|
"new store: %s", err)
|
|
}
|
|
```
|
|
|
|
<tr><td>
|
|
|
|
```
|
|
failed to x: failed to y: failed to create new store: the error
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```
|
|
x: y: new store: the error
|
|
```
|
|
|
|
</td></tr>
|
|
</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
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
t := i.(string)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
t, ok := i.(string)
|
|
if !ok {
|
|
// handle the error gracefully
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
<!-- TODO: There are a few situations where the single assignment form is
|
|
fine. -->
|
|
|
|
### 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
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
func foo(bar string) {
|
|
if len(bar) == 0 {
|
|
panic("bar must not be empty")
|
|
}
|
|
// ...
|
|
}
|
|
|
|
func main() {
|
|
if len(os.Args) != 2 {
|
|
fmt.Println("USAGE: foo <bar>")
|
|
os.Exit(1)
|
|
}
|
|
foo(os.Args[1])
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
func foo(bar string) error {
|
|
if len(bar) == 0 {
|
|
return errors.New("bar must not be empty")
|
|
}
|
|
// ...
|
|
return nil
|
|
}
|
|
|
|
func main() {
|
|
if len(os.Args) != 2 {
|
|
fmt.Println("USAGE: foo <bar>")
|
|
os.Exit(1)
|
|
}
|
|
if err := foo(os.Args[1]); err != nil {
|
|
panic(err)
|
|
}
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
// func TestFoo(t *testing.T)
|
|
|
|
f, err := ioutil.TempFile("", "test")
|
|
if err != nil {
|
|
panic("failed to set up test")
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
// func TestFoo(t *testing.T)
|
|
|
|
f, err := ioutil.TempFile("", "test")
|
|
if err != nil {
|
|
t.Fatal("failed to set up test")
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
<!-- TODO: Explain how to use _test packages. -->
|
|
|
|
### 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/
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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!
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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()
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
## 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`.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
for i := 0; i < b.N; i++ {
|
|
s := fmt.Sprint(rand.Int())
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
for i := 0; i < b.N; i++ {
|
|
s := strconv.Itoa(rand.Int())
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
<tr><td>
|
|
|
|
```
|
|
BenchmarkFmtSprint-4 143 ns/op 2 allocs/op
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```
|
|
BenchmarkStrconv-4 64.2 ns/op 1 allocs/op
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Avoid string-to-byte conversion
|
|
|
|
Do not create byte slices from a fixed string repeatedly. Instead, perform the
|
|
conversion once and capture the result.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
for i := 0; i < b.N; i++ {
|
|
w.Write([]byte("Hello world"))
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
data := []byte("Hello world")
|
|
for i := 0; i < b.N; i++ {
|
|
w.Write(data)
|
|
}
|
|
```
|
|
|
|
</tr>
|
|
<tr><td>
|
|
|
|
```
|
|
BenchmarkBad-4 50000000 22.2 ns/op
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```
|
|
BenchmarkGood-4 500000000 3.25 ns/op
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
## Style
|
|
|
|
### Group Similar Declarations
|
|
|
|
Go supports grouping similar declarations.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
import "a"
|
|
import "b"
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
import (
|
|
"a"
|
|
"b"
|
|
)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
This also applies to constants, variables, and type declarations.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
|
|
const a = 1
|
|
const b = 2
|
|
|
|
|
|
|
|
var a = 1
|
|
var b = 2
|
|
|
|
|
|
|
|
type Area float64
|
|
type Volume float64
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
const (
|
|
a = 1
|
|
b = 2
|
|
)
|
|
|
|
var (
|
|
a = 1
|
|
b = 2
|
|
)
|
|
|
|
type (
|
|
Area float64
|
|
Volume float64
|
|
)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
Only group related declarations. Do not group declarations that are unrelated.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
type Operation int
|
|
|
|
const (
|
|
Add Operation = iota + 1
|
|
Subtract
|
|
Multiply
|
|
ENV_VAR = "MY_ENV"
|
|
)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
type Operation int
|
|
|
|
const (
|
|
Add Operation = iota + 1
|
|
Subtract
|
|
Multiply
|
|
)
|
|
|
|
const ENV_VAR = "MY_ENV"
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
Groups are not limited in where they can be used. For example, you can use them
|
|
inside of functions.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
func f() string {
|
|
var red = color.New(0xff0000)
|
|
var green = color.New(0x00ff00)
|
|
var blue = color.New(0x0000ff)
|
|
|
|
...
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
func f() string {
|
|
var (
|
|
red = color.New(0xff0000)
|
|
green = color.New(0x00ff00)
|
|
blue = color.New(0x0000ff)
|
|
)
|
|
|
|
...
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Import Group Ordering
|
|
|
|
There should be two import groups:
|
|
|
|
- Standard library
|
|
- Everything else
|
|
|
|
This is the grouping applied by goimports by default.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"go.uber.org/atomic"
|
|
"golang.org/x/sync/errgroup"
|
|
)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
|
|
"go.uber.org/atomic"
|
|
"golang.org/x/sync/errgroup"
|
|
)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
|
|
|
|
nettrace "golang.net/x/trace"
|
|
)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"runtime/trace"
|
|
|
|
nettrace "golang.net/x/trace"
|
|
)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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{}
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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 {...}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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)
|
|
}
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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()
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Unnecessary Else
|
|
|
|
If a variable is set in both branches of an if, it can be replaced with a
|
|
single if.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
var a int
|
|
if b {
|
|
a = 100
|
|
} else {
|
|
a = 10
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
a := 10
|
|
if b {
|
|
a = 100
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
var _s string = F()
|
|
|
|
func F() string { return "A" }
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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" }
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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.
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
// foo.go
|
|
|
|
const (
|
|
_defaultPort = 8080
|
|
_defaultUser = "user"
|
|
)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
type Client struct {
|
|
version int
|
|
http.Client
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
type Client struct {
|
|
http.Client
|
|
|
|
version int
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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/
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
k := User{"John", "Doe", true}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
k := User{
|
|
FirstName: "John",
|
|
LastName: "Doe",
|
|
Admin: true,
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
var s = "foo"
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
s := "foo"
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
However, there are cases where the default value is clearer when the `var`
|
|
keyword is use. [Declaring Empty Slices], for example.
|
|
|
|
[Declaring Empty Slices]: https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
func f(list []int) {
|
|
filtered := []int{}
|
|
for _, v := range list {
|
|
if v > 10 {
|
|
filtered = append(filtered, v)
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
func f(list []int) {
|
|
var filtered []int
|
|
for _, v := range list {
|
|
if v > 10 {
|
|
filtered = append(filtered, v)
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
if x == "" {
|
|
return []int{}
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
if x == "" {
|
|
return nil
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
- To check if a slice is empty, always use `len(s) == 0`. Do not check for
|
|
`nil`.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
func isEmpty(s []string) bool {
|
|
return s == nil
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
func isEmpty(s []string) bool {
|
|
return len(s) == 0
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
- The zero value (a slice declared with `var`) is usable immediately without
|
|
`make()`.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
nums := []int{}
|
|
// or, nums := make([]int)
|
|
|
|
if add1 {
|
|
nums = append(nums, 1)
|
|
}
|
|
|
|
if add2 {
|
|
nums = append(nums, 2)
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
var nums []int
|
|
|
|
if add1 {
|
|
nums = append(nums, 1)
|
|
}
|
|
|
|
if add2 {
|
|
nums = append(nums, 2)
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Reduce Scope of Variables
|
|
|
|
Where possible, reduce scope of variables. Do not reduce the scope if it
|
|
conflicts with [Reduce Nesting](#reduce-nesting).
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
err := ioutil.WriteFile(name, data, 0644)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
if err := ioutil.WriteFile(name, data, 0644); err != nil {
|
|
return err
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
If you need a result of a function call outside of the if, then you should not
|
|
try to reduce the scope.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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
|
|
}
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Avoid Naked Parameters
|
|
|
|
Naked parameters in function calls can hurt readability. Add C-style comments
|
|
(`/* ... */`) for parameter names when their meaning is not obvious.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
// func printInfo(name string, isLocal, done bool)
|
|
|
|
printInfo("foo", true, true)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
// func printInfo(name string, isLocal, done bool)
|
|
|
|
printInfo("foo", true /* isLocal */, true /* done */)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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 = 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
wantError := "unknown name:\"test\""
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
wantError := `unknown error:"test"`
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### Initializing Struct References
|
|
|
|
Use `&T{}` instead of `new(T)` when initializing struct references so that it
|
|
is consistent with the struct initialization.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
sval := T{Name: "foo"}
|
|
|
|
// inconsistent
|
|
sptr := new(T)
|
|
sptr.Name = "bar"
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
sval := T{Name: "foo"}
|
|
|
|
sptr := &T{Name: "bar"}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
msg := "unexpected values %v, %v\n"
|
|
fmt.Printf(msg, 1, 2)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
const msg = "unexpected values %v, %v\n"
|
|
fmt.Printf(msg, 1, 2)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
### 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 pre-defined `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 pre-defined 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
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```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)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```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)
|
|
})
|
|
}
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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.
|
|
|
|
<table>
|
|
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
|
|
<tbody>
|
|
<tr><td>
|
|
|
|
```go
|
|
// package db
|
|
|
|
func Connect(
|
|
addr string,
|
|
timeout time.Duration,
|
|
caching bool,
|
|
) (*Connection, error) {
|
|
// ...
|
|
}
|
|
|
|
// Timeout and caching must always be provided,
|
|
// even if the user wants to use the default.
|
|
|
|
db.Connect(addr, db.DefaultTimeout, db.DefaultCaching)
|
|
db.Connect(addr, newTimeout, db.DefaultCaching)
|
|
db.Connect(addr, db.DefaultTimeout, false /* caching */)
|
|
db.Connect(addr, newTimeout, false /* caching */)
|
|
```
|
|
|
|
</td><td>
|
|
|
|
```go
|
|
type options struct {
|
|
timeout time.Duration
|
|
caching bool
|
|
}
|
|
|
|
// Option overrides behavior of Connect.
|
|
type Option interface {
|
|
apply(*options)
|
|
}
|
|
|
|
type optionFunc func(*options)
|
|
|
|
func (f optionFunc) apply(o *options) {
|
|
f(o)
|
|
}
|
|
|
|
func WithTimeout(t time.Duration) Option {
|
|
return optionFunc(func(o *options) {
|
|
o.timeout = t
|
|
})
|
|
}
|
|
|
|
func WithCaching(cache bool) Option {
|
|
return optionFunc(func(o *options) {
|
|
o.caching = cache
|
|
})
|
|
}
|
|
|
|
// Connect creates a connection.
|
|
func Connect(
|
|
addr string,
|
|
opts ...Option,
|
|
) (*Connection, error) {
|
|
options := options{
|
|
timeout: defaultTimeout,
|
|
caching: defaultCaching,
|
|
}
|
|
|
|
for _, o := range opts {
|
|
o.apply(&options)
|
|
}
|
|
|
|
// ...
|
|
}
|
|
|
|
// Options must be provided only if needed.
|
|
|
|
db.Connect(addr)
|
|
db.Connect(addr, db.WithTimeout(newTimeout))
|
|
db.Connect(addr, db.WithCaching(false))
|
|
db.Connect(
|
|
addr,
|
|
db.WithCaching(false),
|
|
db.WithTimeout(newTimeout),
|
|
)
|
|
```
|
|
|
|
</td></tr>
|
|
</tbody></table>
|
|
|
|
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
|
|
|
|
<!-- TODO: replace this with parameter structs and functional options, when to
|
|
use one vs other -->
|