Unit Testing Legacy Code, Part 1: Creating Maintainable Applications

If you’re tired of reading articles about how to apply unit testing to new applications when most of your life is extending and enhancing legacy code, here’s a plan for (finally) exploiting automated testing when working with existing applications. It’s easier than you think, especially if you let Visual Studio and JustMock do the heavy lifting.

I love reading articles about getting started on automated unit testing because those articles are almost completely unrealistic. All these articles assume you’re building some greenfield application from scratch, which—let’s face it—practically never happens. We all know that 70% to 90% of a developer’s time is spent on enhancing, extending, modifying and (sometimes) fixing applications already in production. And I’m here to tell you that no one is willing to pay for you to wrap one of those existing/“legacy” applications in unit tests.

But, because you spend most of your time making changes to those legacy applications, you can do two things:

1. Apply unit testing to those areas you change
2. Use a mocking tool (Telerik JustMock, for example) to fill in the places you’ve left alone

And that strategy makes perfect sense because, after all, the part of the application you haven’t touched still works (presumably). The danger point—the part worth unit testing—is where you’re making changes.

Spoiler alert: Some refactoring is required.

A Legacy Application Case Study

As a case study, I’ll use an application with a page that calculates shipping costs for a product. I’m assuming an ASP.NET MVC application but what I’m going to do will work equally well with WebForms or a desktop application. (Here is the starting code for this project, before we make changes.)

Here’s a unit testing strategy that shows what it means to “unit test” a Web Service API: API Testing—Strategy and Tools.

On the page, the user picks the product to be shipped, selects a quantity to ship, and selects an urgency (High, Medium, Low). The user then clicks the page’s Submit button to send the data for processing.

Therefore, somewhere in the bowels of this ShippingManager application, there exists a method that processes the user’s data: the Product object (which has a weight, height, width and special shipping instructions like “Fragile”), the quantity being shipped and the urgency. That method then calls a CalcShipCost method, which calculates the shipping cost using all that information. Needless to say, the CalcShipCost method also uses several global variables declared at the class level (fields).

That method looks something like this:

[HttpPost]
public ActionResult ShipProduct(IProduct prod, int qty, ShippingUrgency urg)
{
   //…some code…
   **decimal shipCost = CalcShipCost(prod, qty, urg);**
   //…more code…

   ShipAcceptModel model = new ShipAcceptModel();
   model.ShipCost = shipCost;
   return View(model);
}

Here’s the problem: The company wrote CalcShippingCost when they shipped everything one way. The company now wants to keep using that shipping method but they want to “extend” it so the user can pick from several different shipping methods (FedEx, UPS, USPS, whatever).

I’ve got at least three jobs (two of which I’ve created for myself):

1. Enhance the application to support the new shipping methods
2. Do it in a way that reduces maintenance costs
3. Do it in a way that supports automated testing so I don’t have to do manual regression tests to prove it works every time I make a change

Refactoring for Maintainability (and Testing)

You might think that, at this point, the simplest way to extend the application is to put a switch statement in CalcShipCosts that tests for each shipping method and then does the right thing. If so, then you’re right: You probably won’t be able to do unit testing.

Of course, you’ll also have an application that incurs some significant costs every time you need to add or modify a shipping method. With every change you’ll have to:

• Rewrite the CalcShipCosts method
• Do manual regression testing for every shipping scenario to ensure they all still work

If you implement the Strategy pattern, then you’ll get a cheaper application to maintain. In the Strategy pattern, you separate out the various shipping calculation processes so they can be modified independently both of each other and of the application (this pattern will also let you add new shipping methods without having to rewrite either the existing shipping methods or the application).

If you take advantage of some Visual Studio tricks I’ll show you along the way, the refactoring won’t take long (perhaps an hour) and you can do it without disturbing the rest of the application. That you’ll also get an application where you can eliminate manual regression testing is just icing on the cake.

Refactoring for the Strategy pattern means that my revised ShipProduct method will, initially, look something like this (it’s going to get a lot simpler):

[HttpPost]
public ActionResult ShipProduct(IProduct prod, int qty, ShippingUrgency urg, ShippingMethod meth)
{
   IShippingStrategy sStrat = new OriginalShip();
   switch(meth)
   {
       case ShippingMethod.USPS:
                    sStrat = new USPSShip();
                     break;
       case ShippingMethod.FedEx:
                    sStrat = new FedexShip();
                     break;
      //…more shipping methods…
   }
   decimal shipCost = CalcShipCost(prod, qty, urg, sStrat);

   ShipAcceptModel model = new ShipAcceptModel();
   model.ShipCost = shipCost;
   return View(model);
}

And I just typed in the code as you see it, ignoring all the red wavy lines that Visual Studio generates because it’s never heard of these new classes. Each of these new classes (FedExShip, UPSShip, etc.) will hold the code that’s unique to a shipping method (I’ll call them “strategy classes”). To ensure that all my strategy classes will be interchangeable, I also invent an IShipping interface for all my strategy classes to implement.

My next step is to get Visual Studio to generate my interfaces and classes for me. I hover my mouse over the reference to IShippingStrategy, click on the smart tab that appears at the front of the class name, and select “Generate Interface ‘IShippingStrategy’ in a new file” from the resulting menu. I repeat that for all my strategy classes and, suddenly, I have several new files.

Creating my ShippingMethod enum is almost as slick: I click on ShippingMethod in my code and select “Generate new type.”

That pops up a dialog box where I specify that I want this new type to be a public enum created in a new file. I click OK and, once again, I get a new file.

Sadly, Visual Studio has put these new files in the Controllers folder. In Solution Explorer, I drag these new files to where I want them in the Models folder. While I do that, I change each class’s scope from internal to public and update their namespaces for their new location. I also fill in my enum with its values (USPS, FedEx, etc.).

Finally, my CalcShipCost method isn’t set up to accept my new IShippingStrategy parameter. To fix that, I hover my mouse over my call to CalcShipCost, click on the smart tag, and select “Add parameter to …”. Visual Studio fixes up my CalcShipCost method to accept the new parameter.

Generating My First Strategy Object

I don’t throw away that CalcShipCost method for two reasons. First, the method’s got some code that’s independent of the shipping method. Second, the company is going to continue to use the code in the method that is dependent on the current shipping method. That means I’m going cut out the part of the existing CalcShipCost that’s dependent on the current shipping method and move it to my new OriginalShip class (I’ll put it inside a method I’ll call CalcMethodCost).

More precisely: I’m going to get Visual Studio to do all that.

The original CalcShipCost method looks something like this (perhaps, after moving some code around—which I’d have to do even with the “just put in a switch block” solution):

private decimal CalcShipCost(IProduct prod, int qty, ShippingUrgency urg, IShippingStrategy sStrat)
{
   decimal extraCharges = 0;
   bool areExtraCharges = false;

   //code that I realize will be different if we ship using some other method
  decimal shippingCost = 0;
  if (prod.weight > 100)
  {
     shippingCost += 100 * SalesTax;
  }
  if (qty < 50) { ….more code …}
 switch (urg)
 {
     …various case statements
 }
  //…lots more of that code

  //code that I realize is independent of how we ship the product
  if (areExtraCharges)
  {
     shippingCost += extraCharges;
  }
  //…lots more of that code…

  return shippingCost;
}

By the way, if you’ve looked at this code closely enough to wonder where SalesTax came from, it’s one of those fields I mentioned earlier, declared at the class level and used everywhere in the application.

Creating my first strategy object is easy if I leverage Visual Studio and do it in two steps. I first select the code in my original CalcShipCost method that I want to move into my new method, right-click on the selection and pick “Quick Actions and Refactorings” from the pop-up menu.

That displays another menu where I select “Extract Method.”

When you click on the Extract Method menu choice, Visual Studio displays a dialog box, extracts the code I selected and replaces it with a call to a new method named, cleverly, NewMethod. I rename NewMethod to CalcMethodCost in my original code and click the Apply button on the dialog box. Visual Studio magically creates my new method.

I click on my new method call and press F12 to go to the method. Once there, I cut the method out of the ShippingManager application and paste it into my OriginalShip class: I have my first strategy class. I remove the static modifier added to the method and change the method’s scope from private to public.

Finally, back in CalcShipCost, I call my new method from the IShippingStrategy parameter:

private decimal CalcShipCost(IProduct prod, int qty, ShippingUrgency urg, IShippingStrategy sStrat)
{
   decimal extraCharges = 0;
   bool areExtraCharges = false;

   **shippingCost = sStrat.CalcMethodCost(prod, qty, urg);**

   if (areExtraCharges)
   {
      shippingCost += extraCharges;
   }
   //…more code…

   return shippingCost;
}

And here’s my first strategy object:

public class OriginalShip: IShippingStrategy
{
   public decimal CalcMethodCost(IProduct prod, int qty, ShippingUrgency urg)
   {
      //…shipping cost code extracted from original method…
   }
}

I can now build out my interface: I copy my CalcMethodCost method’s first line, paste it into IShippingStrategy, delete the public scope, and put a semicolon at the end:

//Strategy object interface
public interface IShippingStrategy
{
   decimal CalcMethodCost(IProduct prod, int qty, ShippingUrgency urg);
}

I then have Visual Studio do a build and discover that my new OriginalShip class doesn’t know where SalesTax comes from (or, for that matter, any other field from ShippingManager). The easiest solution is just to add SalesTax as another value passed to the call to CalcMethodCost and use “Add parameter to …” again. Sadly, this just updates my interface, so I have to add the parameter to CalcMethodCost in OriginalShip myself.

With my IShippingStrategy interface now defined, I’ll have Visual Studio implement that interface in my other strategy classes: I click on the interface name in each strategy class, click on the smart tab that appears and select “Implement Interface.” My code now compiles, all my strategy objects have a CalcMethodCost method, and I could be done with my refactoring.

Refactoring for Factories

However, because that went so fast (probably less than an hour), I decide that I’ll make one more change to ShipProduct method—having the code that creates the strategy object sit in my original method limits my ability to add in new shipping methods. I decide to implement the Factory Method pattern by putting the switch block that picks the right shipping strategy object into a method in a class of its own.

It’s now a “same old, same old” process: I add a line of code to ShippingManager to instantiate my factory class and then have Visual Studio create the class. I select the switch block and use Extract method to put the block in a method. After pressing F12 to switch to the method, I cut it from ShippingManager and paste it into my new class. Once the method has been relocated, I remove its static modifier and change the method’s private scope to public. I also make the class public, change its namespace and drag the class to the Models folder. Finally, I have the original code in ShippingManager use the new class when it calls the method.

That means my original method now looks like this:

[HttpPost]
public ActionResult ShipProduct(IProduct prod, int qty, ShippingUrgency urg, ShippingMethod meth)
{
   //…more code
   ShippingMethodFactory smf = new ShippingMethodFactory();
   IShippingStrategy sStrat = smf.GetShippingMethod(meth);
   decimal shipCost = CalcShipCost(prod, qty, urg, meth, sStrat);
   //…more code
   return View(model);
}

My ShippingMethodFactory class is pretty simple and looks like this:

public class ShippingMethodFactory
{
   public IShippingStrategy GetShippingMethod(ShippingMethod meth)
   {
      IShippingStrategy sStrat = new OriginalShip();
      switch (meth)
      {
        //…case methods to return the right strategy object…
      }
      return sStrat;
   }
}

That additional factoring takes about five more minutes. Now, if the company adds another shipping method, I can rewrite my factory method to return the related strategy object and leave my CalcShipCost method alone.

There are other benefits: If it turns out that a strategy object requires some configuration code to work correctly, I can put that in my factory method also. If I do that, then, when any developer needs a shipping strategy object, they can just call GetShippingMethod and be confident that they’re getting an object that’s ready to be used.

The Final Refactoring

At this point I realize that, if I do one more refactoring and move the call to CalcShipCost into its own class, I’ll have a standalone shipping calculator. That will also position me to do unit testing at the level of this new calculator—a component-level test. That seems worth doing to me.

Again, I leverage Visual Studio to rewrite my original ShipProduct method from this:

ShippingMethodFactory smf = new ShippingMethodFactory();
IShippingStrategy sStrat = smf.GetShippingMethod(meth);
decimal shipCost = CalcShipCost(prod, qty, urg, meth, sStrat);

To this:

ShippingCostCalculator scc = new ShippingCostCalculator();
decimal shipCost = scc.CalcShipping(prod, qty, urg, meth, SalesTax);

My new class looks like this:

public class ShippingCostCalculator
{
  public decimal CalcShipping(IProduct prod, int qty, ShippingUrgency urg, ShippingMethod meth, decimal salesTax)
  {
      ShippingMethodFactory smf = new ShippingMethodFactory();
      IShippingStrategy sStrat = smf.GetShippingMethod(meth);
      return CalcShipCost(prod, qty, urg, sStrat, salesTax);
  }
  private decimal CalcShipCost(IProduct prod, int qty, ShippingUrgency urg, decimal salesTax )
  {
     //…copied from the application…
}

This positions me to write component-level tests for my new calculator that will prove all my code works together to deliver the answer the business expects.

The code after we’ve done all our refactoring can be found here.

Looking Back

You could do less than I’ve done here. You might decide that you don’t need the factory class. You could decide that if your unit-level tests work, you can go straight to integration and not worry about positioning yourself for component-level tests. (I’ll just point out that if any other application ever needs to calculate product shipping costs, at the cost of updating some namespaces, I could move ShippingCostCalculator to its own class library and make it generally available.)

But, regardless of how much you do, you have a better design. We’ve gone from a monolithic shipping application to a shipping application with a set of Single Responsibility Principle classes: a costing class (ShippingCostCalculator), a factory class (ShippingMethodFactory) and one shipping strategy class for every shipping method the business supports (FedExShip, UPSShip, etc.).

So, after about an hour to an hour and a half of refactoring, I have a highly maintainable design and—a happy accident—one that I can unit test. Which is a good thing because, after all this hacking and slashing, I don’t know if the code actually works anymore. That’s my next post.