5 Common mistakes in Go

Bug-risks are issues in code that can cause errors and breakages in production. A bug is a flaw in the code that produces undesired or incorrect results. Code often has bug-risks due to poor coding practices, lack of version control best practices, miscommunication of requirements, and unrealistic time schedules for development. In this post, let's take a look at a few commonly seen bug-risks in Go we've seen so far.

1. Infinite recursive call

A function that calls itself recursively needs to have an exit condition. Otherwise, it will recurse forever until the system runs out of memory.

This issue can be caused by common mistakes such as forgetting to add an exit condition. It can also happen "on purpose." Some languages have tail-call optimization, which makes certain infinite recursive calls safe to use. Tail-call optimization allows you to avoid allocating a new stack frame for a function because the calling function will return the value that it gets from the called function. The most common use is tail-recursion, where a recursive function written to take advantage of tail-call optimization can use constant stack space. Go, however, does not implement tail-call optimization, and you will eventually run out of memory. However, this problem doesn't apply to spawning new goroutines.

Recommended reading: Why is a Goroutine’s stack infinite ?

2. Assignment to nil map

A map needs to be initialized using the make function (or a map literal) before you can add any elements. A new, empty map value is made using the built-in function make, which takes the map type and an optional capacity hint as arguments:

make(map[string]int)
make(map[string]int, 100)

The initial capacity does not bound its size: maps grow to accommodate the number of items stored in them, with the exception of nil maps. A nil map is equivalent to an empty map except that no elements may be added.

Bad pattern:

var countedData map[string][]ChartElement

Good pattern:

countedData := make(map[string][]ChartElement)

Recommended reading: Go: assignment to entry in nil map

3. Method modifies receiver

A method that modifies a non-pointer receiver value may have unwanted consequences. This is a bug risk because the method may change the value of the receiver inside the method, but it won't reflect in the original value. To propagate the change, the receiver must be a pointer.

For example:

type data struct {
    num   int
    key   *string
    items map[string]bool
}

func (d data) vmethod() {
    d.num = 8
}

func (d data) run() {
    d.vmethod()
    fmt.Printf("%+v", d) // Output: {num:1 key:0xc0000961e0 items:map[1:true]}
}

If num must be modified:

type data struct {
    num   int
    key   *string
    items map[string]bool
}

func (d *data) vmethod() {
    d.num = 8
}

func (d *data) run() {
    d.vmethod()
    fmt.Printf("%+v", d) // Output: &{num:8 key:0xc00010a040 items:map[1:true]}
}

4. Possibly undesired value being used in goroutine

Range variables in a loop are reused at each iteration; therefore, a goroutine created in a loop will point to the range variable from the upper scope. This way, the goroutine could use the variable with an undesired value.

In the example below, the value of index and value used in the goroutine are from the outer scope. Because the goroutines run asynchronously, the value of index and value could be (and usually are) different from the intended value.

mySlice := []string{"A", "B", "C"}
for index, value := range mySlice {
    go func() {
        fmt.Printf("Index: %d\n", index)
        fmt.Printf("Value: %s\n", value)
    }()
}

To overcome this problem, a local scope must be created, like in the example below.

mySlice := []string{"A", "B", "C"}
for index, value := range mySlice {
    index := index
    value := value
    go func() {
        fmt.Printf("Index: %d\n", index)
        fmt.Printf("Value: %s\n", value)
    }()
}

Another way to handle this could be by passing the values as args to the goroutines.

mySlice := []string{"A", "B", "C"}
for index, value := range mySlice {
    go func(index int, value string) {
        fmt.Printf("Index: %d\n", index)
        fmt.Printf("Value: %s\n", value)
    }(index, value)
}

Recommended reading: What happens with closures running as goroutines?

5. Deferring Close before checking for a possible error

It's a common pattern amongst Go developers, to defer the Close() method for a value that implements the io.Closer interface. For example, when opening a file:

f, err := os.Open("/tmp/file.md")
if err != nil {
    return err
}
defer f.Close()

But this pattern is harmful for writable files because deferring a function call ignores its return value, and the Close() method can return errors. For instance, if you wrote data to the file, it might have been cached in memory and not flushed to disk by the time you called Close. This error should be explicitly handled.

While you could go ahead without using defer at all, you would need to remember to close the file everytime their job is done. A better way would be to defer a wrapper function, like in the example below.

f, err := os.Open("/tmp/file.md")
if err != nil {
    return err
}

defer func() {
    closeErr := f.Close()
    if closeErr != nil {
        if err == nil {
            err = closeErr
        } else {
            log.Println("Error occurred while closing the file :", closeErr)
        }
    }
}()
return err

Recommended reading: Do not defer Close() on writable files

When it comes to working in a team, reviewing other people's code becomes important. DeepSource is an automated code review tool that manages the end-to-end code scanning process and automatically makes pull requests with fixes whenever new commits are pushed or new pull requests.

Setting up DeepSource for Go is extremely easy. As soon as you have it set up, an initial scan will be performed on your entire codebase, find scope for improvements, fix them, and open PRs for those changes.

go build!