Common anti-patterns in Go
It has been widely acknowledged that coding is an art, and like every artisan who crafts wonderful art and is proud of them, we as developers are also really proud of the code we write. In order to achieve the best results, artists constantly keep searching for ways and tools to improve their craft. Similarly, we as developers keep levelling up our skills and remain curious to know the answer to the single most important question -“How to write good code”. 🤯
Frederick P. Brooks in his book “The Mythical Man Month: Essays on Software Engineering ” wrote :
“The programmer, like the poet, works only slightly removed from pure thought-stuff. He builds his castles in the air, from air, creating by exertion of the imagination. Few media of creation are so flexible, so easy to polish and rework, so readily capable of realizing grand conceptual structures.
Image source:https://xkcd.com/844/
This post tries to explore answers to the big question mark in the comic above. The simplest way to write good code is to abstain from includinganti-patterns in the code we write.😇
What are anti-patterns? 🤔
Anti-patterns occur when code is written without taking future considerations into account. Anti-patterns might initially appear to be an appropriate solution to the problem, but, in reality, as the codebase scales, these come out to be obscure and add ‘technical debt’ to our codebase.
A simple example of an anti-pattern is to write an API without considering how the consumers of the API might use it, as explained in example 1 below. Being aware of anti-patterns and consciously avoid using them while programming is surely a major step towards a more readable and maintainable codebase. In this post, let’s take a look at a few commonly seen anti-patterns in Go.
1. Returning value of unexported type from an exported function
In Go, toexport
anyfield
orvariable
we need to make sure that its name starts with an uppercase letter. The motivation behind exporting them is to make them visible to other packages. For example, if we want to use thePi
function frommath
package, we should address it asmath.Pi
. Usingmath.pi
won’t work and will error out.
Names (struct fields, functions, variables) that start with a lowercase letter are unexported, and are only visible inside the package they are defined in.
An exported function or method returning a value of an unexported type may be frustrating to use since callers of that function from other packages will have to define a type again to use it.
// Bad practicetypeunexportedTypestringfuncExportedFunc()unexportedType{returnunexportedType("some string")}// RecommendedtypeExportedTypestringfuncExportedFunc()ExportedType{returnExportedType("some string")}
2. Unnecessary use of blank identifier
In various cases, assigning value to a blank identifier is not needed and is unnecessary. In case of using the blank identifier infor
loop, the Go specification mentions :
If the last iteration variable is the blank identifier, the range clause is equivalent to the same clause without that identifier.
// Bad practicefor_=rangesequence{run()}x,_:=someMap[key]_=<-ch// Recommendedforrangesomething{run()}x:=someMap[key]<-ch
3. Using loop/multipleappend
s to concatenate two slices
When appending multiple slices into one, there is no need to iterate over the slice and append each element one by one. Rather, it is much better and efficient to do that in a singleappend
statement.
As an example, the below snippet does concatenation by appending elements one by one through iterating oversliceTwo
.
for_,v:=rangesliceTwo{sliceOne=append(sliceOne,v)}
But, since we know thatappend
is avariadic
function and thus, it can be invoked with zero or more arguments. Therefore, the above example can be re-written in a much simpler way by using only oneappend
function call like this:
sliceOne=append(sliceOne,sliceTwo…)
4. Redundant arguments inmake
calls
Themake
function is a special built-in function used to allocate and initialize an object of type map, slice, or chan. For initializing a slice usingmake
, we have to supply the type of slice, the length of the slice, and the capacity of the slice as arguments. In the case of initializing amap
usingmake
, we need to pass the size of themap
as an argument.
make
, however, already has default values for those arguments:
- For channels, the buffer capacity defaults to zero (unbuffered).
- For maps, the size allocated defaults to a small starting size.
- For slices, the capacity defaults to the length if capacity is omitted.
Therefore,
ch=make(chanint,0)sl=make([]int,1,1)
can be rewritten as:
ch=make(chanint)sl=make([]int,1)
However, using named constants with channels is not considered as an anti-pattern, for the purposes of debugging, or accommodating math, or platform-specific code.
constc=0ch=make(chanint,c)// Not an anti-pattern
5. Uselessreturn
in functions
It is not considered good practice to put areturn
statement as the final statement in functions that do not have a value to return.
// Useless return, not recommendedfuncalwaysPrintFoofoo(){fmt.Println("foofoo")return}// RecommendedfuncalwaysPrintFoo(){fmt.Println("foofoo")}
Named returns should not be confused with useless returns, however. The return statement below really returns a value.
funcprintAndReturnFoofoo()(foofoostring){foofoo:="foofoo"fmt.Println(foofoo)return}
6. Uselessbreak
statements inswitch
In Go,switch
statements do not have automaticfallthrough
. In programming languages like C, the execution falls into the next case if the previous case lacks thebreak
statement. But, it is commonly found thatfallthrough
inswitch
-case is used very rarely and mostly causes bugs. Thus, many modern programming languages, including Go, changed this logic to neverfallthrough
the cases by default.
Therefore, it is not required to have abreak
statement as the final statement in a case block ofswitch
statements. Both the examples below act the same.
Bad pattern:
switchs{case1:fmt.Println("case one")breakcase2:fmt.Println("case two")}
Good pattern:
switchs{case1:fmt.Println("case one")case2:fmt.Println("case two")}
However, for implementingfallthrough
inswitch
statements in Go, we can use thefallthrough
statement. As an example, the code snippet given below will print23
.
switch2{case1:fmt.Print("1")fallthroughcase2:fmt.Print("2")fallthroughcase3:fmt.Print("3")}
7. Not using helper functions for common tasks
Certain functions, for a particular set of arguments, have shorthands that can be used instead to improve efficiency and better understanding/readability.
For example, in Go, to wait for multiple goroutines to finish, we can use async.WaitGroup
. Instead of incrementing async.WaitGroup
counter by1
and then adding-1
to it in order to bring the value of the counter to0
and in order to signify that all the goroutines have been executed :
wg.Add(1)// ...some codewg.Add(-1)
It is easier and more understandable to usewg.Done()
helper function which itself notifies thesync.WaitGroup
about the completion of all goroutines without our need to manually bring the counter to0
.
wg.Add(1)// ...some codewg.Done()
8. Redundantnil
checks on slices
The length of anil
slice evaluates to zero. Hence, there is no need to check whether a slice isnil
or not, before calculating its length.
For example, thenil
check below is not necessary.
ifx!=nil&&len(x)!=0{// do something}
The above code could omit thenil
check as shown below:
iflen(x)!=0{// do something}
9. Too complex function literals
Function literals that only call a single function can be removed without making any other changes to the value of the inner function, as they are redundant. Instead, the inner function that is being called inside the outer function should be called.
For example:
fn:=func(xint,yint)int{returnadd(x,y)}
Can be simplified as:
10. Usingselect
statement with a single case
Theselect
statement lets a goroutine wait on multiple communication operations. But, if there is only a single operation/case, we don’t actually requireselect
statement for that. A simplesend
orreceive
operation will help in that case. If we intend to handle the case to try a send or receive without blocking, it is recommended to add adefault
case to make theselect
statement non-blocking.
// Bad patternselect{casex:=<-ch:fmt.Println(x)}// Recommendedx:=<-chfmt.Println(x)
Usingdefault
:
select{casex:=<-ch:fmt.Println(x)default:fmt.Println("default")}
11. context.Context should be the first param of the function
The context.Context should be the first parameter, typically named ctx. ctx should be a (very) common argument for many functions in a Go code, and since it’s logically better to put the common arguments at the first or the last of the arguments list. Why? It helps us to remember to include that argument due to a uniform pattern of its usage. In Go, as the variadic variables may only be the last in the list of arguments, it is hence advised to keep context.Context as the first parameter. Various projects like even Node.js have some conventions like error first callback. Thus, it’s a convention that context.Context should always be the first parameter of a function.
// Bad practicefuncbadPatternFunc(kfavContextKey,ctxcontext.Context){// do something}// RecommendedfuncgoodPatternFunc(ctxcontext.Context,kfavContextKey){// do something}
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 pull requests for those changes.
go build
Top comments(0)
For further actions, you may consider blocking this person and/orreporting abuse