puzzle pieces

Refactoring C# Data Access Code

Don’t Repeat Yourself is a well-known Computer Science adage. Sometimes, however, it’s too convenient to just repeat some code now and deal with it later. But eventually that practice will cost you. Maintaining multiple copies of code becomes a nightmare when a bug needs to be fixed or new features need to be added. It’s especially bad when many people are working on the code and don’t realize other copies exist elsewhere. Ugh.

For Whom The Bell Tolls

I’ve been working on a suite of programs for the past three years. Most of the programs share a common data access library. And, every so often, a new program requires a new data accessor. The library had been accumulating nearly-identical methods for almost three years! Look at this ugly code snippet (and imagine a dozen or so similar blocks).

public IEnumerable<JobMaterial> GetJobMaterials()
{
  ensureConnection();

  Timer timer = new Timer().Start();

  var jobMaterials = new LinkedList<JobMaterial>();
  try
  {
    using (var command = new SqlCommand(getJobMaterialsSql, connection))
    {
      int rowCount = command.TryExecute(reader =>
        jobMaterials.AddFirst(readJobMaterial(reader))
      );

      log.DebugFormat("Read {0} job materials in {1}", rowCount, timer);
    }
  }
  catch (SqlException e)
  {
    log.Error("Failed to look up job materials", e);
    throw new ApplicationException("Failed to look up job materials", e);
  }

  return jobMaterials;
}

There’s a lot of noise (i.e. boilerplate code) and not much code that is directly relevant to reading job materials. Specifically, this method shouldn’t care about testing the connection, timing the length of the operation, logging, exception trapping, etc. How did I let things get so bad?

Functional Programming to the Rescue

I’m finding that the two-plus years I spent writing Scala code has heavily influenced my C# code to be more functional-programming-esque. Take a look at the new and improved accessor function that injects the job-materials-specific code into a generic accessor method.

public IEnumerable<JobMaterial> GetJobMaterials()
{
  return getList<JobMaterial>(
    new SqlCommand(getJobMaterialsSql, connection),
    readJobMaterial,
    "job materials"
  );
}

private IEnumerable<JobMaterial> readJobMaterial(IDataReader reader)
{
  // Shown here so you can see the function signature
}

All the watery redundancy has been boiled off and I’m left with sweet, sweet code syrup.

The new generic helper method, getList, is basically all the boilerplate from the original method. But now it injects the function parameter in order to read the appropriate data:

protected IEnumerable getList<T>(
  IDbCommand command,
  Func<IDataReader, T> fnRead,
  String what)
{
  ensureConnection();

  Timer timer = new Timer().Start();

  var list = new LinkedList<T>();
  try
  {
    using (command)
    {
      int rowCount = command.TryExecute(reader =>
        list.AddFirst(fnRead(reader))
      );

      log.DebugFormat("Read {0} {1} in {2}", rowCount, what, timer);
    }
  }
  catch (DbException e)
  {
    log.ErrorFormat("Failed to look up {0}", what);
    throw new ApplicationException("Failed to look up " + what, e);
  }

  return list;
}

In Summary…

Spend the time to refactor as soon as you’ve got the same pattern showing up in multiple places.

I’ve barely scratched the surface of Functional Programming here. I highly recommend you learn about FP because, even in small doses, it can be used to improve your procedural code.