Go-advice

(Some of advices are implemented in go-critic)

Contents

Go Proverbs

  • Don't communicate by sharing memory, share memory by communicating.
  • Concurrency is not parallelism.
  • Channels orchestrate; mutexes serialize.
  • The bigger the interface, the weaker the abstraction.
  • Make the zero value useful.
  • interface{} says nothing.
  • Gofmt's style is no one's favorite, yet gofmt is everyone's favorite.
  • A little copying is better than a little dependency.
  • Syscall must always be guarded with build tags.
  • Cgo must always be guarded with build tags.
  • Cgo is not Go.
  • With the unsafe package there are no guarantees.
  • Clear is better than clever.
  • Reflection is never clear.
  • Errors are values.
  • Don't just check errors, handle them gracefully.
  • Design the architecture, name the components, document the details.
  • Documentation is for users.
  • Don't panic.

Author: Rob Pike See more: https://go-proverbs.github.io/

The Zen of Go

  • Each package fulfils a single purpose
  • Handle errors explicitly
  • Return early rather than nesting deeply
  • Leave concurrency to the caller
  • Before you launch a goroutine, know when it will stop
  • Avoid package level state
  • Simplicity matters
  • Write tests to lock in the behaviour of your package’s API
  • If you think it’s slow, first prove it with a benchmark
  • Moderation is a virtue
  • Maintainability counts

Author: Dave Cheney See more: https://the-zen-of-go.netlify.com/

Code

Always go fmt your code.

Community uses the official Go format, do not reinvent the wheel.

Try to reduce code entropy. This will help everyone to make code easy to read.

Multiple if-else statements can be collapsed into a switch

// NOT BAD
if foo() {
	// ...
} else if bar == baz {
	// ...
} else {
	// ...
}

// BETTER
switch {
case foo():
	// ...
case bar == baz:
	// ...
default:
	// ...
}

To pass a signal prefer chan struct{} instead of chan bool.

When you see a definition of chan bool in a structure, sometimes it's not that easy to understand how this value will be used, example:

type Service struct {
	deleteCh chan bool // what does this bool mean? 
}

But we can make it more clear by changing it to chan struct{} which explicitly says: we do not care about value (it's always a struct{}), we care about an event that might occur, example:

type Service struct {
	deleteCh chan struct{} // ok, if event than delete something.
}

Prefer 30 * time.Second instead of time.Duration(30) * time.Second

You don't need to wrap untyped const in a type, compiler will figure it out. Also prefer to move const to the first place:

// BAD
delay := time.Second * 60 * 24 * 60

// VERY BAD
delay := 60 * time.Second * 60 * 24

// GOOD
delay := 24 * 60 * 60 * time.Second

// EVEN BETTER
delay := 24 * time.Hour

Use time.Duration instead of int64 + variable name

// BAD
var delayMillis int64 = 15000

// GOOD
var delay time.Duration = 15 * time.Second

Group const declarations by type and var by logic and/or type

// BAD
const (
	foo     = 1
	bar     = 2
	message = "warn message"
)

// MOSTLY BAD
const foo = 1
const bar = 2
const message = "warn message"

// GOOD
const (
	foo = 1
	bar = 2
)

const message = "warn message"

This pattern works for var too.

defer func() {
	err := ocp.Close()
	if err != nil {
		rerr = err
	}
}()
  • don't use checkErr function which panics or does os.Exit
  • use panic only in very specific situations, you have to handle error
  • don't use alias for enums 'cause this breaks type safety 
package main

type Status = int
type Format = int // remove `=` to have type safety

const A Status = 1
const B Format = 1

func main() {
	println(A == B)
}
  • if you're going to omit returning params, do it explicitly
    • so prefer this _ = f() to this f()
  • the short form for slice initialization is a := []T{}
  • iterate over array or slice using range loop
    • instead of for i := 3; i < 7; i++ {...} prefer for _, c := range a[3:7] {...}
  • use backquote(`) for multiline strings
  • skip unused param with _
func f(a int, _ string) {}
  • If you are comparing timestamps, use time.Before or time.After. Don't use time.Sub to get a duration and then check its value.
  • always pass context as a first param to a func with a ctx name
  • few params of the same type can be defined in a short way
func f(a int, b int, s string, p string)
func f(a, b int, s, p string)
var s []int
fmt.Println(s, len(s), cap(s))
if s == nil {
	fmt.Println("nil!")
}
// Output:
// [] 0 0
// nil!
var a []string
b := []string{}

fmt.Println(reflect.DeepEqual(a, []string{}))
fmt.Println(reflect.DeepEqual(b, []string{}))
// Output:
// false
// true
  • do not compare enum types with <, >, <= and >=
    • use explicit values, don't do this:
value := reflect.ValueOf(object)
kind := value.Kind()
if kind >= reflect.Chan && kind <= reflect.Slice {
	// ...
}
func f1() {
	var a, b struct{}
	print(&a, "\n", &b, "\n") // Prints same address
	fmt.Println(&a == &b)     // Comparison returns false
}

func f2() {
	var a, b struct{}
	fmt.Printf("%p\n%p\n", &a, &b) // Again, same address
	fmt.Println(&a == &b)          // ...but the comparison returns true
}
  • wrap errors with fmt.Errorf
    • so: fmt.Errorf("additional message to a given error: %w", err)
  • be careful with range in Go:
  • reading nonexistent key from map will not panic
    • value := map["no_key"] will be zero value
    • value, ok := map["no_key"] is much better
  • do not use raw params for file operation
    • instead of an octal parameter like os.MkdirAll(root, 0700)
    • use predefined constants of this type os.FileMode
  • don't forget to specify a type for iota 
const (
	_ = iota
	testvar // will be int
)

vs

type myType int
const (
	_ myType = iota
	testvar // will be myType
)

Don’t use encoding/gob on structs you don’t own.

At some point structure may change and you might miss this. As a result this might cause a hard to find bug.

Don't depend on the evaluation order, especially in a return statement.

// BAD
return res, json.Unmarshal(b, &res)

// GOOD
err := json.Unmarshal(b, &res)
return res, err

To prevent unkeyed literals add _ struct{} field:

type Point struct {
	X, Y float64
	_    struct{} // to prevent unkeyed literals
}

For Point{X: 1, Y: 1} everything will be fine, but for Point{1,1} you will get a compile error:

./file.go:1:11: too few values in Point literal

There is a check in go vet command for this, there is no enough arguments to add _ struct{} in all your structs.

To prevent structs comparison add an empty field of func type

type Point struct {
	_ [0]func()	// unexported, zero-width non-comparable field
	X, Y float64
}

Prefer http.HandlerFunc over http.Handler

To use the 1st one you just need a func, for the 2nd you need a type.

Move defer to the top

This improves code readability and makes clear what will be invoked at the end of a function.

JavaScript parses integers as floats and your int64 might overflow.

Use json:"id,string" instead.

type Request struct {
	ID int64 `json:"id,string"`
}

Concurrency

  • best candidate to make something once in a thread-safe way is sync.Once
    • don't use flags, mutexes, channels or atomics
  • to block forever use select{}, omit channels, waiting for a signal
  • don't close in-channel, this is a responsibility of it's creator
    • writing to a closed channel will cause a panic
  • func NewSource(seed int64) Source in math/rand is not concurrency-safe. The default lockedSource is concurrency-safe, see issue: golang/go#3611
  • when you need an atomic value of a custom type use atomic.Value

Performance

  • do not omit defer
    • 200ns speedup is negligible in most cases
  • always close http body aka defer r.Body.Close()
    • unless you need leaked goroutine
  • filtering without allocating
b := a[:0]
for _, x := range a {
	if f(x) {
		b = append(b, x)
	}
}

To help compiler to remove bound checks see this pattern _ = b[7]

  • time.Time has pointer field time.Location and this is bad for go GC
    • it's relevant only for big number of time.Time, use timestamp instead
  • prefer regexp.MustCompile instead of regexp.Compile
    • in most cases your regex is immutable, so init it in func init
  • do not overuse fmt.Sprintf in your hot path. It is costly due to maintaining the buffer pool and dynamic dispatches for interfaces.
    • if you are doing fmt.Sprintf("%s%s", var1, var2), consider simple string concatenation.
    • if you are doing fmt.Sprintf("%x", var), consider using hex.EncodeToString or strconv.FormatInt(var, 16)
  • always discard body e.g. io.Copy(ioutil.Discard, resp.Body) if you don't use it
    • HTTP client's Transport will not reuse connections unless the body is read to completion and closed
res, _ := client.Do(req)
io.Copy(ioutil.Discard, res.Body)
defer res.Body.Close()
  • don't use defer in a loop or you'll get a small memory leak
    • 'cause defers will grow your stack without the reason
  • don't forget to stop ticker, unless you need a leaked channel
ticker := time.NewTicker(1 * time.Second)
defer ticker.Stop()
func (entry Entry) MarshalJSON() ([]byte, error) {
	buffer := bytes.NewBufferString("{")
	first := true
	for key, value := range entry {
		jsonValue, err := json.Marshal(value)
		if err != nil {
			return nil, err
		}
		if !first {
			buffer.WriteString(",")
		}
		first = false
		buffer.WriteString(key + ":" + string(jsonValue))
	}
	buffer.WriteString("}")
	return buffer.Bytes(), nil
}
// noescape hides a pointer from escape analysis.  noescape is
// the identity function but escape analysis doesn't think the
// output depends on the input. noescape is inlined and currently
// compiles down to zero instructions.
func noescape(p unsafe.Pointer) unsafe.Pointer {
	x := uintptr(p)
	return unsafe.Pointer(x ^ 0)
}
  • for fastest atomic swap you might use this m := (*map[int]int)(atomic.LoadPointer(&ptr))
  • use buffered I/O if you do many sequential reads or writes
    • to reduce number of syscalls
  • there are 2 ways to clear a map:
    • reuse map memory
for k := range m {
	delete(m, k)
}
  • allocate new
m = make(map[int]int)

Modules

Build

Testing

  • prefer package_test name for tests, rather than package
  • go test -short allows to reduce set of tests to be runned
func TestSomething(t *testing.T) {
	if testing.Short() {
		t.Skip("skipping test in short mode.")
	}
}
  • skip test depending on architecture
if runtime.GOARM == "arm" {
	t.Skip("this doesn't work under ARM")
}

Tools

  • quick replace gofmt -w -l -r "panic(err) -> log.Error(err)" .
  • go list allows to find all direct and transitive dependencies
    • go list -f '{{ .Imports }}' package
    • go list -f '{{ .Deps }}' package
  • for fast benchmark comparison we've a benchstat tool
  • go-critic linter enforces several advices from this document
  • go mod why -m <module> tells us why a particular module is in the go.mod file
  • GOGC=off go build ... should speed up your builds source
  • The memory profiler records one allocation every 512Kbytes. You can increase the rate via the GODEBUG environment variable to see more details in your profile.

Misc

go func() {
	sigs := make(chan os.Signal, 1)
	signal.Notify(sigs, syscall.SIGQUIT)
	buf := make([]byte, 1<<20)
	for {
		<-sigs
		stacklen := runtime.Stack(buf, true)
		log.Printf("=== received SIGQUIT ===\n*** goroutine dump...\n%s\n*** end\n", buf[:stacklen])
	}
}()
  • check interface implementation during compilation
var _ io.Reader = (*MyFastReader)(nil)
var hits struct {
	sync.Mutex
	n int
}
hits.Lock()
hits.n++
hits.Unlock()
  • httputil.DumpRequest is very useful thing, don't create your own
  • to get call stack we've runtime.Caller https://golang.org/pkg/runtime/#Caller
  • to marshal arbitrary JSON you can marshal to map[string]interface{}{}
  • configure your CDPATH so you can do cd github.com/golang/go from any directore
    • add this line to your bashrc(or analogue) export CDPATH=$CDPATH:$GOPATH/src
  • simple random element from a slice
    • []string{"one", "two", "three"}[rand.Intn(3)]