Go Preferences
These are a few tips on how to make Go code more
1. False
When checking for false, prefer this:
if myVariableName == false {
sendEmail("oh no")
}
to this:
if !myVariableName {
sendEmail("oh no")
}
The reason is that when scanning through code it's easy to miss the !; typing a few extra characters makes it more readable.
2. Switches
When choosing among several possible outcomes, prefer a switch statement to an if statement, like this:
switch {
case f.IsList():
return "it's a list"
case f.IsMap():
return "it's a map"
default:
return "no idea"
}
... not this:
if f.IsList() {
return "it's a list"
else if f.IsMap() {
return "it's a map"
} else {
return "no idea"
}
The switch statement lays out the options more clearly, and your eye doesn't have to follow the logic of "if/else if/else". The switch statment places everything in a vertical line.
3. Early exit
Get negative tests out of the way early in a function, and exit, rather than creating nested if statements. For example, instead of this:
if len(grades) > 0{
var sum float64
for _, g := range grades {
sum += g
}
return sum / len(grades)
}
...do this:
if len(grades) == 0{
return 0
}
var sum float64
for _, g := range grades {
sum += g
}
return sum / len(grades)
This keeps the eye going down in a vertical line, and avoids having to think about the meaning of the nested levels.
4. Call a function instead of commenting
While writing a function, it's common to end up with a few blocks of logic in the middle. They do some kind of processing on the data we received before returning a result. Sometimes an intermediate result is involved, then we do something else to it, etc. If you find yourself starting to write a comment to explain what the block does, refactor it to its own function. The function name should be all the documentation you need.
For example, consider this function that determines if a student is eligible to graduate:
func eligibleForGraduation(s *Student) error {
var unclearedHolds int
for _, hold := range s.Holds {
if hold.Status != cleared {
unclearedHolds++
}
}
if unclearedHolds > 0 {
return fmt.Errorf("student has %d uncleared holds", unclearedHolds)
}
if len(s.Grades) == 0{
return fmt.Errorf("student GPA is less than 2.0")
}
var sum float64
for _, g := range s.Grades {
sum += g
}
gpa := sum/len(s.Grades)
if gpa >= 2.0 {
return nil
} else {
return fmt.Errorf("student GPA is less than 2.0")
}
return false
}
This function is quite short, but imagine if there were five or six more graduation requirements; it would quickly become hard to read. Furthermore, the logic of determining the intermediate values (such as GPA) is inline with the logic that uses the values to implement the process logic.
You might be tempted to do this to help the next reader:
func eligibleForGraduation(s *Student) error {
// check if student has any holds on his account
var unclearedHolds int
for _, hold := range s.Holds {
if hold.Status != cleared {
unclearedHolds++
}
}
if unclearedHolds > 0 {
return fmt.Errorf("student has %d uncleared holds", unclearedHolds)
}
// ensure GPA meets the minimum for graduation
if len(s.Grades) == 0{
return fmt.Errorf("student GPA is less than 2.0")
}
var sum float64
for _, g := range s.Grades {
sum += g
}
gpa := sum/len(s.Grades)
if gpa >= 2.0 {
return nil
} else {
return fmt.Errorf("student GPA is less than 2.0")
}
return false
}
This helps, by separating chunks of code and explaining what they do so the reader doesn't have to review each loop, but it doesn't solve the problem of mixing process logic (is the student eligible to graduate?) and the logic to get the intermediate values.
If we rewrite it like this instead, it is easier to understand exactly what the graduation requirements are:
func eligibleForGraduation(s *Student) error {
if unclearedHolds(s) > 0 {
return fmt.Errorf("student has %d uncleared holds", unclearedHolds)
}
if gpa(s) < 2.0 {
return fmt.Errorf("student GPA is less than 2.0")
}
return nil
}
We can also now document the code func separately where it is defined, test it separately, and re-use it elsewhere, etc.
5. Zoom out
Often when we review a pull request, we're presented with a few changes in a few files, or maybe a function or a type is introduced and used. Without comments it can be very hard to understand why the code is written the way it is. Sure, if you read the code you can figure out what the new function does, but it doesn't tell you why we're doing it in the first place.
Begin by commenting the new function and say what it does and why you had to create it. Then, put it in larger context (zoom out). If we have a function called GetLastCustomerStatusChange, in the pull request explain why we added that function ("Added GetLastCustomerStatusChange to determine if customer lifetime value needs to be updated"). This allows the pull request reviewer to weigh in: is that the best way to determine if the value must be recalculated? What other cases can occur? Do we have another, existing way to determine this?