Skip to main content

How To Code Go Part 2: Standard Guidelines

Why guidelines?​

This guideline is inspired by Uber Go Style. this can be valuable for teams seeking a highly consistent and well-defined coding style beyond the basic formatting provided by gofmt. It promotes clarity, reduces code review friction, and encourages best practices. However, teams should carefully consider the overhead and opinionated nature of the guide before adopting it, ensuring it aligns with their project's needs and the team's preferences. Using a consistent style guide, whether it's Uber's or a modified version, is generally preferable to having no clear guidelines.

The single return value form of a type assertion will panic on an incorrect type. Therefore, always use the "comma ok" idiom. It's a suggestion but should to follow if the variable is input from external.

BadGood
t := i.(string)t, ok := i.(string)
.if !ok {
. // handle error
.}

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.

Keep in mind that users can modify a map or slice you received as an argument if you store a reference to it.

BadGood
func (d *Driver) SetTrips(trips []Trip) {
. d.trips = trips
.}
.trips := ...
.d1.SetTrips(trips)
. trips[0] = ...		func (d *Driver) SetTrips(trips []Trip) {
. d.trips = make([]Trip, len(trips))
. copy(d.trips, trips)
.}
.trips := ...
.d1.SetTrips(trips)
. trips[0] = ...

MUST Returning Slices and Maps​

Similarly, be wary of user modifications to maps or slices exposing internal state.

BadGood
type Stats struct {
. mu sync.Mutex
. counters map[string]int
.}
.
.func (s *Stats) Snapshot() map[string]int {
. s.mu.Lock()
. defer s.mu.Unlock()
. return s.counters
.}
.
.snapshot := stats.Snapshot()		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 := stats.Snapshot()

MUST 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:

  1. A pointer to some type-specific information. You can think of this as "type."
  2. 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 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
    BadGood
    type Handler struct {
    .}
    .
    .func (h *Handler) ServeHTTP(
    . w http.ResponseWriter,
    . r *http.Request,
    .) {
    . ...
    .}    		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,
    .) {
    .}    		type LogHandler struct {
    . h http.Handler
    . log *zap.Logger
    .}
    .
    .var _ http.Handler = LogHandler{}
    .
    .func (h LogHandler) ServeHTTP(
    . w http.ResponseWriter,
    . r *http.Request,
    .) {
    .}

MUST 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.

BadGood
mu := new(sync.Mutex)
.mu.Lock()		var mu sync.Mutex
.mu.Lock()
If you use a struct by pointer, then the mutex should be a non-pointer field on it. Do not embed the mutex on the struct, even if the struct is not exported.
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]
.}		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]
.}
The Mutex field, and the Lock and Unlock methods are unintentionally part of the exported API of SMap.The mutex and its methods are implementation details of SMap hidden from its callers.

Use defer to clean up resources such as files and locks.

BadGood
p.Lock()
.if p.count < 10 {
.p.Unlock()
.return p.count
.}
.p.count++
.newCount := p.count
.p.Unlock()
.return newCount		p.Lock()
.defer p.Unlock()
.if p.count < 10 {
.return p.count
.}
.p.count++
.return p.count

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.

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.

BadGood

| c := make(chan int``, 64``)

|

c := make(chan int``, 1``)

c := make(chan int``)

| | | |

MUST 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.

BadGood
func run(args []string) {
.if len(args) == 0 {
.panic("an argument is required")
.}
.}
.
.func main() {
.run(os.Args[1:])
.}		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.

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.

BadGood
f, err := ioutil.TempFile("", "test")
.if err != nil {
.panic("failed to set up test")
.}		f, err := ioutil.TempFile("", "test")
.if err != nil {
.t.Fatal("failed to set up test")
.}

Avoid panics generally, unless in an init  function or in a package that uses a disciplined panic-based error handling protocol (and converts panics to errors).

MUST Recover with goroutine​

As said above, code running in production should avoid panics. So we should create goroutine with recovery. A good practice is use a unify goroutine-create function instead of go  key word.

BadGood
func Go(f func()) {
.go func() {
.defer Recover()
.f()
.}()
.}		func Go(f func()) {
.go func() {
.defer Recover()
.f()
.}()
.}
func Foo(a int, b string) {
....
.}		func Foo(a int, b string) {
....
.}
func Bar() {
.var a int
.var b string
....
.helper.Go(func() {
.Foo(a, b)
.})
.}		func Bar() {
.var a int
.var b string
....
.helper.Go(func() {
.Foo(a, b)
.})
.}

Please note that do not use goroutines on loop iterator variables.

Avoid mutating global variables, instead opting for dependency injection. This applies to function pointers as well as other kinds of values.

BadGood
var _timeNow = time.Now
.func sign(msg string) string {
.now := _timeNow()
.return signWithTime(msg, now)
.}		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)
.}
func TestSign(t *testing.T) {
.oldTimeNow := _timeNow
._timeNow = func() time.Time {
.return someFixedTime
.}
.defer func() { _timeNow = oldTimeNow }()
.assert.Equal(t, want, sign(give))
.}		func TestSigner(t *testing.T) {
.s := newSigner()
.s.now = func() time.Time {
.return someFixedTime
.}
.assert.Equal(t, want, s.Sign(give))
.}

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.

BadGood
type ConcreteList struct {
.*AbstractList
.}		type ConcreteList struct {
.list *AbstractList
.}
func (l *ConcreteList) Add(e Entity) {
.l.Add(e)
.}
.func (l *ConcreteList) Remove(e Entity) {
.l.Remove(e)
.}		func (l *ConcreteList) Add(e Entity) {
.l.list.Add(e)
.}
.func (l *ConcreteList) Remove(e Entity) {
.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.

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.

BadGood
type AbstractList interface {
.Add(Entity)
.Remove(Entity)
.}
.type ConcreteList struct {
.AbstractList
.}		type ConcreteList struct {
.list AbstractList
.}
.
.func (l *ConcreteList) Add(e Entity) {
.l.list.Add(e)
.}
.func (l *ConcreteList) Remove(e Entity) {
.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.

MUST 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.

BadGood
var error string
.func handleErrorMessage(error string) {
.}		var errorMessage string
.func handleErrorMessage(msg string) {
.}
type Foo struct {
.error error
.string string
.}
.func (f Foo) Error() error {
.return f.error
.}
.func (f Foo) String() string {
.return f.string
.}		type Foo struct {
.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() 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 | | --- | ---- | | type Foo struct {
.}
.var _defaultFoo Foo
.func init() {
._defaultFoo = Foo{
.}
.} | var _defaultFoo = Foo{
.}
.var _defaultFoo = defaultFoo()
.func defaultFoo() Foo {
.return Foo{
.}
.} | | type Config struct {
.}
.var _config Config
.func init() {
.cwd, _ := os.Getwd()
.raw, _ := ioutil.ReadFile(``<br />.path.Join(cwd, "config", "config.yaml")<br />.`)`<br />.`yaml.Unmarshal(raw, &_config)`<br />.`}` | `type Config struct {`<br />.`}`<br />.`func loadConfig() Config {`<br />.`cwd, err := os.Getwd()`<br />.`raw, err := ioutil.ReadFile(
.path.Join(cwd, "config", "config.yaml")``<br />.)<br />.var config Config<br />.yaml.Unmarshal(raw, &config)<br />.return config<br />.}` |

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.

If possible, prefer to call os.Exit or log.Fatal at most once in your main(). If there are multiple error scenarios that halt program execution, put that logic under a separate function and return errors from it.

This has the effect of shortening your main() function and putting all key business logic into a separate, testable function.

BadGood
package main
.func main() {
.args := os.Args[1:]
.if len(args) != 1 {
.log.Fatal("missing file")
.}
.name := args[0]
.f, err := os.Open(name)
.if err != nil {
.log.Fatal(err)
.}
.defer f.Close()
.b, err := ioutil.ReadAll(f)
.if err != nil {
.log.Fatal(err)
.}
.}		package main
.func main() {
.if err := run(); err != nil {
.log.Fatal(err)
.}
.}
.func run() error {
.args := os.Args[1:]
.if len(args) != 1 {
.return errors.New("missing file")
.}
.name := args[0]
.f, err := os.Open(name)
.if err != nil {
.return err
.}
.defer f.Close()
.b, err := ioutil.ReadAll(f)
.if err != nil {
.return err
.}
.}