Golang best practices

This document is a compliment of 2 official sources, please refer to those as part of the Golang best practices journey:

• https://github.com/golang/go/wiki/CodeReviewComments
• https://golang.org/doc/effective_go.html

Code principles

Don’t write code that only works. Aim to write code that can be maintained — not only by yourself but by anyone else who may end up working on the software at some point in the future.

80 percent of the time a developer is reading code and 20 percent writing and testing the code. So please focus on writing readable code!

your code should not need comments to understand what it is doing!

To help us to develop good code, there are many programming principles that we can use as the guidelines. Below we will list the most important ones:

• KISS - It stands for “Keep It Simple, Stupid”. You may notice that developers at the beginning of their journey try to implement the complicated, ambiguous design
• DRY - “Don’t Repeat Yourself”. Try to avoid any duplicates, instead, you put them into a single part of the system or a method.
• YAGNI - “You Ain’t Gonna Need It”. If you run into a situation where you are asking yourself, “What about adding extra (feature, code, …etc.) ?”, you probably need to re-think about it.
• Clean code over clever code - Speaking of clean code, leave your ego at the door and forget about writing clever code.
• Avoid premature optimization - The problem with premature optimization is that you can never really know where a program’s bottlenecks will be until after the fact.
• Single responsibility - Every class/struct, package/module or function/method in a program should only concern itself with providing one bit of specific functionality.
• Fail fast, fail hard - The fail-fast principle stands for stopping the current operation as soon as any unexpected error occurs. Adhering to this principle generally results in a more stable solution

Packages

Organize by responsibility

Favor structuring packages by domain concerns rather than technical layers. A common practice from other languages is to organize types together in a package called models or types. In Go, we organize code by their functional responsibilities.

package models// DON'T DO IT!!!

type User struct {...}

Rather than creating a models package and declare all entity types there, a User type should live in a service-layer package.

Even though, the Go language doesn't restrict where you define types, it is often a good practice to keep the core types grouped at the top of a file.

Avoid very long files

The net/http package from the standard library contains 15734 lines in 47 files.

Don't forget that the package name will appear before the identifier you chose.

• In package encoding/json we find the type Encoder, not JSONEncoder.
• It is referred as json.Encoder.

Avoid package names like base, common, or util

In the case where utility functions are used in many places prefer multiple packages, each focused on a single aspect, to a single monolithic package. eg. dateutil, textutil, stringutil

Keep package main small as small as possible

Your main function, and main package should do as little as possible. This is because main packages are not importable and things there can not be reused.

func main() should parse flags, open connections to databases, loggers, and such, then hand off execution to a high-level object

Concurrency

TLDR

1. It is really hard to do it correctly. Try your best to not use it at all.
2. It is really hard to test. Try your best to not use it at all.

Avoid concurrency in your API

Let the caller be responsible for the async call. It is a good practice to know when your goroutine will stop, this way your consumer will be concerned with all the goroutines it produced are finished

func serveApp() {  
    mux := http.NewServeMux()  
    mux.HandleFunc("/", func(resp http.ResponseWriter, req *http.Request) {  
    fmt.Fprintln(resp, "Hello, QCon!")  
    })  

    if err := http.ListenAndServe("0.0.0.0:8080", mux); err != nil {  
    log.Fatal(err)  
    }  
}  

func serveDebug() {  
    if err := http.ListenAndServe("127.0.0.1:8001", http.DefaultServeMux); err != nil {  
      log.Fatal(err)   
    }   
}  

func main() {  
    go serveDebug() // The caller is responsible for the async call  
    go serveApp()  
    select {}  
}

Thread Safe

As Java when developing asynchronous code with Golang, we need to make sure our code is Thread-safe and it is done using sync.RWMutex.

Check out this in-memory cache project and how Thread-safe is done.

One more recommendation to achieve Thread-safe is to avoid pass pointer to a goroutine.

go myFunc(&myParam) // DON'T DO IT!!!

Alternatively, you can use channels to pass values between goroutines. Channels work for many situations and encouraged. You can read more about concurrency in Effective Go. Channels don't always fit every situation though, so it depends on the situation.

Further reading

Miscellaneous

Return early rather than nesting deeply

Go code is written in a style where the success path continues down the screen as the function progresses. This simple approach will reduce a lot the Cognitive complexity of your code.

func (b *Buffer) UnreadRune() error {  
    if b.lastRead <= opInvalid {  
        return  errors.New("bytes.Buffer: UnreadRune: previous operation was not a successful ReadRune")  
    }  

    if b.off >= int(b.lastRead) {  
        b.off -= int(b.lastRead)  
    }  

    b.lastRead = opInvalid  
    return  nil  
}

Errors Handling

An error should be handled only once. Logging an error is handling an error. So an error should either be logged or propagated, and logging should be the least preferred way to handle an error.

1. When propagating an error, preferred way is to wrap it using %w with fmt.Errorf() (and not log it)
2. When logging an error use %v for default presentation as given by error.Error() string (error interface)

See also fmt package documentation https://golang.org/pkg/fmt/

Let’s see an example of what we would expect with a REST call leading to a DB issue:

unable to serve HTTP POST request for customer customer_test: unable to insert customer contract customer_test: unable to commit transaction

We could do it this way:

func postHandler(customer string) bool {  
  err := insert(customer)  
  if err != nil {  
    logrus.Errorf("unable to serve HTTP POST request for customer %s: %v", customer, err)  
    return  false  
  }  
  return  true  
}  

func insert(contract string) error {  
  err := dbQuery(contract)  
  if err != nil {  
    return  fmt.Errorf("unable to insert customer contract %s: %w", contract, err)  
  }  
  return  nil  
}  

func dbQuery(contract string) error {  
  // Do something then fail  
 return fmt.Errorf("unable to commit transaction")  
}

More here

HTTP/GRPC Timeouts

Always set timeouts to your requests(GRPC, HTTP, DB)

//HTTP call

c := &http.Client{
Timeout: 15 * time.Second,
}

resp, err := c.Get(``"[https://deem.com/"](https://deem.com/%22)``)
//DB call
newCtx, cancel := context.WithTimeout(ctx, time.Second)
row := c.db.QueryRowContext(newCtx,` `"SELECT name FROM items WHERE id = ?"``, id)

Panic or log.Fatalf

• The log message goes to the configured log output, while panic is only going to write to stderr.

• Panic will print a stack trace, which may not be relevant to the error at all.

• Defers will be executed when a program panics, but calling os.Exit exits immediately, and deferred functions can't be run.

In general, only use panic for programming errors, where the stack trace is important to the context of the error. If the message isn't targeted at the programmer use log.Fatalf

More here

Use Enums values instead of a list of constants

Don't do  this!!!  

const (  
StatusOpen = 0  
StatusClosed = 1  
StatusUnknown = 2  
)  

Instead, use Enum  

type Status uint32  

const (  
StatusOpen Status = iota  
StatusClosed  
StatusUnknown  
)

Pointers! Pointers Everywhere!

Passing a variable by value will create a copy of this variable. Whereas passing it by pointer will just copy the memory address.

Hence, passing a pointer will always be faster, isn’t it?

Actually, that is not true, In some benchmarks, passing by value is more than 4 times faster than passing by pointer. This might a bit counterintuitive, right?

The explanation of this result is related to how the memory is managed in Go. More here

Tests

Use tesdata folder to keep test data

Go build ignores the directory named testdata and it will not be part of the final binary.

It is also ignored by the go tool, Directory and file names that begin with "." or "_" . More here

Use testing folder for test related files

We recommend placing all required objects/configs/data in the testing directory. Be aware that testdata inside the testing folder will be ignored by the go build

Prefer internal tests to external tests (packagename_test or just packagename)

Prefer using internal tests when writing unit tests for your package(without _test). This allows you to test each function or method directly, avoiding the bureaucracy of external testing.

More

We like testify

Simple comparisons are good enough to test with. However, it can get tedious and inconsistent to write our own failure messages. assert and require reduce the noise in a test and provide nicely formatted default failure messages. Plus it works very well with the standard libraries.

Mocking

We are using tesfify mock package for easily writing mock objects that can be used in place of real objects when writing test code.

Referencies:

https://golang.org/doc/effective_go.html

https://dave.cheney.net/practical-go/presentations/qcon-china.html#_avoid_package_names_like_base_common_or_util

https://rakyll.org/style-packages/

https://github.com/codeship/go-best-practices/tree/master/concurrency

https://itnext.io/the-top-10-most-common-mistakes-ive-seen-in-go-projects-4b79d4f6cd65

https://github.com/codeship/go-best-practices