How to Safely Extend your Legacy Code

Introduction

The article assumes basic knowledge of refactoring and unit tests. If you are new to these topics, you are also welcomed, but I recommend reading something about yourself first.

Sobering

Let’s be honest. You have heard about refactoring, unit tests, and how they are important. But you perceived it as additional work. But are they? Aren’t they right the opposite?

Imagine the situation. You were working on a new task. It is an expansion of the existing feature. You just finished it. In the better case, you also test the application from the user's experience. After a while, the tester approves the task's completion but reports another two new bugs in common parts of the expanded feature. How frustrating is it? Does it sound familiar?

Don't you think that time spent fixing bugs can be equal to time spent on writing unit tests? Wouldn't it be better to discover a new bug on your own? The answer is a nice suite of tests.

But how when you are responsible for a large project which runs in production? Well, there is a lot of books covering this topic. Probably the most famous is “Working Effectively with Legacy Code” by Michael C. Feathers. I am reading a named book recently, and I found it cruelly captivating and inspirational.

Motivation

I believe many of us want to refactor but can’t just stop development for nothing. No customer will agree with the half-year postponement because we came up with something called refactoring. In this article, I want to present the techniques you can apply in the mentioned situations.

Sprout method

When you are extending the code of a new sequence of instructions, you should also extract new instructions into the new method. The noticed approach of modification is called the Sprout method.

This technique is compelling when you must expand a large method, but you are afraid to touch a tangled monster. The Sprout method allows you to cover a new sequence of instructions with tests. Let's see an example.

public class TransactionGate{
  ...
  var orderItems = GetOrderItems();
  foreach(var orderItem in orderItems)
  {
    transactionBundle.Add(orderItem);
  }
  ...
}

Now a new task demands to filter out order items that are not already in the TransactionBundle object. It sounds like a pretty easy change. Let’s code.

public class TransactionGate{
  ...
  var orderItems = GetOrderItems();
  foreach(var orderItem in orderItems)
  {
   if(!transactionBundle.Contains(orderItem)){
    transactionBundle.Add(orderItem);
   }
  }
  ...
}

No big deal, right? What do we violate by this procedure?

1. We introduced a new responsibility. The Single Responsibility Principle says, “Method should do one thing and one thing only.” More responsibility means more fragile code.
2. We still don't have any tests because we extend part of the code, which is hard to cover with tests.
3. We made the code more fragile.

This time differently.

public class TransactionGate{
  ...
  var orderItems = GetOrderItems();
  var uniqueOrderItems = GetUniqueOrderItems(orderItems);

  foreach(var orderItem in uniqueOrderItems)
  {
    transactionBundle.Add(orderItem);
  }
  ...    
  public List<OrderItem> GetUniqueOrderItems(List<OrderItem> orderItems){
    ...
  }
}

We created a method GetUniqueOrderItems with one parameter; a list of order items. We changed the existing code. First, we invoked the new method and introduced its result into a variable. Then we changed the variable which is used in the Foreach statement. None of the changes are increasing the complexity of the existing code.

Now we can initiate class and write tests for new code. I recommend using Test Driven Development to develop a Sprout method.

Sprout Method is not always an appropriate approach. Sometimes you may come across to class with a lot of dependencies. Mocking all of them and covering the whole class functionality with tests could be a waste of time. Remember, we are in a rush. It is time to present to you the Sprout Class.

Sprout Class

When you must extend a large method with a sequence of instructions and the class is heavily dependent on different objects, it can be better to extract the sequence of instructions into a new class method. The new class allows you to initialize objects more easily, which means you will cover new functionality with unit tests much more easily. This class is called the Sprout Class. Take a look at the example below.

public class OrdersFacade{
  private IExternalCustomerApiService customerApiService;
  private IOrderRepository orderRepository;
  ...

  public OrdersFacade(
    IExternalCustomerApiService customerApiService,
    IOrderRepository orderRepository,
    ...
    )
    {
    this.customerApiService = customerApiService;
    this.orderRepository = orderRepository;
    ...
    }

  public bool CreateOrder(){
    ...

    var customerInfoInJson = customerApiService.GetCustomerInfo(customerId);
    var customerInfo = JSonConvert.Serialize(customerInfoInJson);
    order.CustomerName = customerInfo.Name;

    orderRepository.Save(order);
    return true;
    }
}

You should see there are minimally three responsibilities, and you can't even see all the code.

Let’s suppose this is a huge class, and it would take about a day to get it covered by tests. We know we are in a hurry. We want to add a VAT to the price of the order, but only if the customer is not the company. It doesn't sound like a long-lasting operation.

Let’s develop a completely separated class, preferably using TDD.

public class OrderVatCalculator{
  public Order AddVatIfNotCompany(Order order, bool isCompany)
  {
    decimal vat = 1.21;
    if(!isCompany){
      order.Price = order.Price * vat;
    }
  }
}

I am sure you are thinking something like: “Is he crazy? Creating a class for such a little amount of code?” Before you hang me, listen.

Why not? Did you ever do multitask? How did it go for you? Do you think you can do a better job if you do one simple thing? With multitasking comes much more responsibility and more responsibility means a greater possibility of an error. The same rule works for code.

The new piece of instructions is covered by tests and has only one responsibility. It is very easy to read and maintain.

Finally, let’s initialize an object of the Sprout class in existing code, pass parameters into its method, and let a method do the required expression.

public class OrdersFacade{
  private IExternalCustomerApiService customerApiService;
  private IOrderRepository orderRepository;
  ...

  public OrdersFacade(
    IExternalCustomerApiService customerApiService,
    IOrderRepository orderRepository,
    ...
    )
    {
    this.customerApiService = customerApiService;
    this.orderRepository = orderRepository;
    ...
    }

  public bool CreateOrder(){
    ...

    var customerInfoInJson = customerApiService.GetCustomerInfo(customerId);
    var customerInfo = JSonConvert.Serialize(customerInfoInJson);
    order.CustomerName = customerInfo.Name;

    var orderVatCalculator = new OrderVatCalculator();
    orderVatCalculator.AddVatIfCompany(customerInfo.IsCompany);

    orderRepository.Save(order);
    return true;
    }
}