// before
func EnrichReading(r *Reading) { ... }

// after
func (r *Reading) Enrich() { ... }

Scenario #

A few days ago, I worked on transforming a YAML file that represents a small web application into a CloudFormation (CFN) template.

This is roughly the code that I started with in service/web.go:

package service

type Web struct {
	Name      string `yaml:"name"`
	ImageURL  string `yaml:"imageUrl"`
	CPU       int    `yaml:"cpu"`
	Memory    int    `yaml:"memory"`
}

func CFNTemplate(w *Web) string {
  return `
AWSTemplateFormatVersion: '2010-09-09'
Parameters:
  ServiceName:
    Type: String
    Default: ` + w.Name + `
  ImageUrl:
    Type: String
    Default: ` + w.ImageURL + `
  ContainerCpu:
    Type: Number
    Default: ` + w.CPU + `
  ContainerMemory:
    Type: Number
    Default: ` + w.Memory 
    // the rest of the template
}

What are some problems with this code?

  1. It’s not extensible. I know that later on I’ll have to parse other types of services that will be transformed to a CFN template. Since service.CFNTemplate only accepts a *service.Web, it can’t work with other types. The impact here is within the service package.
  2. It locks any customer of service.CFNTemplate to service.Web. This is related to the previous point, but this time the impact is upstream. The chain of functions that end up invoking service.CFNTemplate will need to be updated once we allow it to accept a more generic type.

Refactoring #

To deal with the extensibility problem, we can instead transform the function to a method:

func (w *Web) CFNTemplate() string { ... }

Now, each new service type can implement its own CFNTemplate method. It won’t need to be tied down to the Web type.

To deal with the lock in problem, the caller can instead accept an interface. Here is an example:

package main

type CFNTemplater interface {
  CFNTemplate() string
}

func deployStack(client *cloudformation.Cloudformation, srv CFNTemplater) {
  ...
  tpl := srv.CFNTemplate()
  ...
}

This is pretty powerful and it reminds me of the general security advice of granting least privilege. The function went from accepting a particular struct, *service.Web, that has a bunch of other methods that deployStack doesn’t care about to only accepting what it needs: the CFNTemplate method.

Mechanics #

The steps for this refactoring is similar to Combine Functions into Class from Martin Fowler’s refactoring book.

  1. Create a new struct for the data that’s common between functions.
  2. Transform the function to a method by adding the new struct as a receiver.

Conclusion #

Here are some rules of thumb when you hesitate between creating a function or a method:

  1. If your function is accessing fields in your struct, instead consider transforming to a method.
  2. Replace any upstream function that accepted the struct as a parameter with an interface of the method needed for flexibility.

Categories refactoring