Our data package is currently based on functions, and as such, the changes are going to be a little different compared to the previous ones. Here is a typical function from the data package:

// Load will attempt to load and return a person.
// It will return ErrNotFound when the requested person does not exist.
// Any other errors returned are caused by the underlying database 
// or our connection to it.
func Load(ctx context.Context, ID int) (*Person, error) {
   db, err := getDB()
   if err != nil {
      logging.L.Error("failed to get DB connection. err: %s", err)
      return nil, err
   }

   // set latency budget for the database call
   subCtx, cancel := context.WithTimeout(ctx, 1*time.Second)
   defer cancel()

   // perform DB select
   row := db.QueryRowContext(subCtx, sqlLoadByID, ID)

   // retrieve columns and populate the person object
   out, err := populatePerson(row.Scan)
   if err != nil {
      if err == sql.ErrNoRows {
         logging.L.Warn("failed to load requested person '%d'. err: %s", ID, err)
         return nil, ErrNotFound
      }

      logging.L.Error("failed to convert query result. err: %s", err)
      return nil, err
   }
   return out, nil
}

In this function, we have references to the logger which we want to remove, and one configuration that we really need to extract. The config is required by the first line of the function from the previous code. Here is the getDB() function:

var getDB = func() (*sql.DB, error) {
   if db == nil {
      if config.App == nil {
         return nil, errors.New("config is not initialized")
      }

      var err error
      db, err = sql.Open("mysql", config.App.DSN)
      if err != nil {
         // if the DB cannot be accessed we are dead
         panic(err.Error())
      }
   }

   return db, nil
}

We have a reference to the DSN to create the database pool. So, what do you think our first step should be?

As with the previous change, let's first define an interface that includes all of the dependencies and configuration that we want to inject:

// Config is the configuration for the data package
type Config interface {
   // Logger returns a reference to the logger
   Logger() logging.Logger

   // DataDSN returns the data source name
   DataDSN() string
}

Now, let's update our functions to inject the config interface:

// Load will attempt to load and return a person.
// It will return ErrNotFound when the requested person does not exist.
// Any other errors returned are caused by the underlying database 
// or our connection to it.
func Load(ctx context.Context, cfg Config, ID int) (*Person, error) {
   db, err := getDB(cfg)
   if err != nil {
      cfg.Logger().Error("failed to get DB connection. err: %s", err)
      return nil, err
   }

   // set latency budget for the database call
   subCtx, cancel := context.WithTimeout(ctx, 1*time.Second)
   defer cancel()

   // perform DB select
   row := db.QueryRowContext(subCtx, sqlLoadByID, ID)

   // retrieve columns and populate the person object
   out, err := populatePerson(row.Scan)
   if err != nil {
      if err == sql.ErrNoRows {
         cfg.Logger().Warn("failed to load requested person '%d'. err: %s", ID, err)
         return nil, ErrNotFound
      }

      cfg.Logger().Error("failed to convert query result. err: %s", err)
      return nil, err
   }
   return out, nil
}

var getDB = func(cfg Config) (*sql.DB, error) {
   if db == nil {
      var err error
      db, err = sql.Open("mysql", cfg.DataDSN())
      if err != nil {
         // if the DB cannot be accessed we are dead
         panic(err.Error())
      }
   }

   return db, nil
}

Unfortunately, this change is going to break a lot of things as getDB() is called by all of the public functions in the data package, which are in turn called by the model layer packages. Thankfully, we have enough unit tests to help prevent regression while working through the changes.

I'd like to ask you to stop for a moment and consider this: we are attempting to make what should be an insignificant change, but it's causing a mass of small changes. Additionally, we are being forced to add one parameter to every public function in this package. How does this make you feel about the decision to build this package based on functions? Refactoring away from functions would be no small task, but do you think it would be worth it?

The changes to the model layer are small, but interesting, thanks to the fact that we have already updated the model layer with config injection.

There are only have two small changes to make:

• We will add the DataDSN() method to our config
• We need to pass the config down to data package via the loader() call

Here is the code with the changes applied:

// Config is the configuration for Getter
type Config interface {
   Logger() logging.Logger
   DataDSN() string
}

// Getter will attempt to load a person.
// It can return an error caused by the data layer or when the 
// requested person is not found
type Getter struct {
   cfg Config
}

// Do will perform the get
func (g *Getter) Do(ID int) (*data.Person, error) {
   // load person from the data layer
   person, err := loader(context.TODO(), g.cfg, ID)
   if err != nil {
      if err == data.ErrNotFound {
         // By converting the error we are hiding the implementation 
         // details from our users.
         return nil, errPersonNotFound
      }
      return nil, err
   }

   return person, err
}

// this function as a variable allows us to Monkey Patch during testing
var loader = data.Load

Sadly, we need to make these small changes in all of our model layer packages. After that is done, our dependency graph now looks as shown in the following diagram:

Fantastic. There is only one unnecessary connection to the config package left, and it comes from the exchange package.