Simple Go
These are a few rules I try to follow to keep my Go code simple and maintainable. This is meant as a supplement to Go Code Review Comments.
Don't Assign Variables in an If Condition
This is easy to read:
func shippingRate(postalCode, serviceName string, weight int) (Rate, bool) {
region, ok := regions[postalCode]
if !ok {
return Rate{}, false
}
service, ok := region.Services[serviceName]
if !ok {
return Rate{}, false
}
rate, ok := service.Rates[weightClass(weight)]
if !ok {
return Rate{}, false
}
return rate, true
}
This is shorter but less readable:
func shippingRate(postalCode, serviceName string, weight int) (Rate, bool) {
if region, ok := regions[postalCode]; ok {
if service, ok := region.Services[serviceName]; ok {
if rate, ok := service.Rates[weightClass(weight)]; ok {
return rate, true
}
}
}
return Rate{}, false
}
But what about limiting scope? That usually doesn't matter. Typically you'll be doing an early return on the negative case.
Avoid else
There are very few cases in Go where else is needed, and where it's used it is
usually a smell indicating your func is too complex.
Here the func is doing too much:
func newClient(cfg clientConfig) (*Client, error) {
var transport http.RoundTripper
if cfg.transportType == "tls" {
transport, err = newTLSTransport(cfg)
if err != nil {
return nil, err
}
} else if cfg.transportType == "proxy" {
transport, err = newProxyTransport(cfg)
if err != nil {
return nil, err
}
} else {
panic("invalid transport type")
}
return &Client{Transport: transport}, nil
}
Pulling the transport creation into a separate func makes it simpler and avoids
the else:
func newClient(cfg clientConfig) (*Client, error) {
transport, err := newTransport(cfg)
if err != nil {
return nil, err
}
return &Client{Transport: transport}, nil
}
func newTransport(cfg clientConfig) (http.RoundTripper, error) {
switch cfg.transportType {
case "tls":
return newTLSTransport(cfg)
case "proxy":
return newProxyTransport(cfg)
default:
panic("invalid transport type")
}
}
Another simpler example is assigning a value:
// bad
var a string
if x > 10 {
a = "big"
} else {
a = "small"
}
// good
a := "small"
if x > 10 {
a = "big"
}
You Don't Need Functional Options
Functional options are slick, but they are usually overkill. It's simpler to directly pass a pointer to an options struct. The options struct is also better for discoverability because all the fields are defined in the same place.
With an options struct:
type ClientOptions struct {
Timeout time.Duration
Retry int
}
func NewClient(baseURL string, opts *ClientOptions) *Client {
if opts == nil {
opts = &ClientOptions{}
}
return &Client{
baseURL: baseURL,
timeout: opts.Timeout,
retry: opts.Retry,
}
}
With functional options:
type ClientOption func(*Client)
func WithTimeout(timeout time.Duration) ClientOption {
return func(c *Client) {
c.timeout = timeout
}
}
func WithRetry(retry int) ClientOption {
return func(c *Client) {
c.retry = retry
}
}
func NewClient(baseURL string, opts ...ClientOption) *Client {
c := &Client{
baseURL: baseURL,
}
for _, opt := range opts {
opt(c)
}
return c
}
Don't Not Panic
"Don't panic" is one of the better known Go Proverbs. Jonathan Hall wrote a good post about it. Definitely worth reading if you aren't familiar.
Some people read that as a rule that you should literally never use the panic
builtin, but that's not what it means. It means your code shouldn't panic in
normal operation when called with valid input. It's actually quite useful to use
panic in cases that won't be except for a bug in the calling code.
For example if you are using an int-derived type as an enum, and your func gets
called with an invalid value. Consider the Color type below.
ParseColorName returns an error when the input isn't a supported color name
because it's expected that the caller might be using user input for the name.
But by the time you're dealing with a Color value, it should be valid.
type Color int
const (
ColorRed Color = iota + 1
ColorGreen
ColorBlue
)
func ParseColorName(name string) (Color, error) {
switch name {
case "red":
return ColorRed, nil
case "green":
return ColorGreen, nil
case "blue":
return ColorBlue, nil
default:
return 0, fmt.Errorf("invalid color name: %s", name)
}
}
func (c Color) Hex() string {
switch c {
case ColorRed:
return "#FF0000"
case ColorGreen:
return "#00FF00"
case ColorBlue:
return "#0000FF"
default:
panic("invalid color")
}
}
If you're trying to avoid calling panic you might be tempted to return
"invalid color" instead of panicking, but that would just move the problem to
the caller who will end writing invalid css without realizing it. Instead you
might change Hex to return an error. That would make every caller handle an
error that won't happen in normal operation.
A rule of thumb is that you should always return an error for an operation error but panic for a programming error.
Read more about this in Alex Edwards' blog post "When Is It OK to Panic in Go?".