Adding a second line of business: Refactoring Strategies

In the Lessons Learned case study, we discussed the high level logistical issues driving my team’s decision to add a second line of business to an existing application.

In this article, we’ll go over the code-level refactoring steps involved in this very large and high risk project. If you find yourself in a similar position to the one in the above article, you can use the techniques in this guide to make the process a lot easier.

Safely extracting subclasses from a 10,000 line God Object

For consistency, we’ll continue to use the car insurance & home insurance examples from the case study.

In our example, our application’s main line of business was car insurance, and although there were a lot of car insurance related business concerns scattered throughout, there were also a lot of generic entities that could be shared across the insurance landscape. These are things like insurance agents, insurance companies, insured businesses and people, notices, payments, and most other accounting-related entities.

In this application, we found that there were two root objects that tie everything else together. We found that every customer was managed first through a quoting process via a Quote object, and if the quote was accepted, it became an Account. All business was conducted through these objects. These Quote and Account objects also had database primary key IDs that spread throughout most other objects.

In your case, you likely have at least one object like this.

We found that these root objects were heavily laced with car insurance concerns, so to add a second line of business in a way that would allow us to continue using Quote and Account as the main business vehicle (no pun intended), we decided to convert both Quote and Account into abstract base classes, and have the application work with CarInsuranceQuote, CarInsuranceAccount, HomeInsuranceQuote, and HomeInsuranceAccount subclasses instead.

By extracting subclasses and leaving any shared properties and methods in the base class, we were also priming the system to accept future lines of business as well. Adding boat insurance would simply involve implementing BoatInsuranceQuote and BoatInsuranceAccount subclasses.

The remainder of this article will look at what it took to extract subclasses from just one of the above classes (the Quote class), as you’ll only have to repeat the process for any others.

Step 1: Creating The Line Of Business Distinction

The very first step involved when adding a second line of business to an existing application is to make your application aware of the concept of lines of business. Your app will need a way to distinguish between the two, and since your app has only ever been the original line (car insurance, in our case), it makes sense to initialize all existing Quote objects as belonging to that line.

The easiest way to do this is to add an enum property that represents each line of business, and initialize all existing objects to be of the original line.

public enum LineOfBusiness { CarInsurance = 1, HomeInsurance = 2 }

public class Quote
{
    public LineOfBusiness LineOfBusiness { get; set; }

    //...
}

And the supporting schema changes…

ALTER TABLE Quotes
ADD LineOfBusiness int NOT NULL
DEFAULT (1) --init to existing line of business

You’ll also likely need to modify your ORM or any persistence code for saving/loading this property.

Step 2: Constructor Madness

Now that your app is LOB-aware, it’s time to convert your original class into an abstract base class and bring about the existence of the line of business child classes.

First, make your original class abstract, along with the LOB property we created in the last step.

public abstract class Quote
{
    public abstract LineOfBusiness LineOfBusiness { get; }
}

Then, create a child class for each of your planned lines of business. You can permanently set the line of business since it will never change when using this object.

public class CarInsuranceQuote : Quote
{
    public override LineOfBusiness LineOfBusiness => LineOfBusiness.CarInsurance;
}

public class HomeInsuranceQuote : Quote
{
    public override LineOfBusiness LineOfBusiness => LineOfBusiness.HomeInsurance;
}

//any others?

This step is called Constructor Madness because if your app has been using plain old constructors to create new and/or load existing objects for use, the abstract class conversion will instantly cause a lot of compiler errors.

Being 20-years old, our app was created using the Active Record design pattern, which means that objects are responsible for handling their own persistence (they save and load themselves by interacting directly with the database).

If your app is using an ORM, you may have it slightly easier.

Quote existingQuote = new Quote(existingQuoteID);

var q = new Quote(); //these no longer work...

There are a couple ways to handle this:

1) Create a static factory method on the base class that works out which child class to instantiate based on the line of business. Then replace all usages of new Quote() with the factory method.

public static Quote GetExisting(int quoteId)
{
    var lineOfBusiness = GetLineOfBusiness(quoteId);

    switch (lineOfBusiness)
    {
        case LineOfBusiness.CarInsurance:
            return new CarInsuranceQuote(quoteId);

        case LineOfBusiness.HomeInsurance:
            return new HomeInsuranceQuote(quoteId);
    }
}

Or…

2) Assume most existing constructor usages in your code were designed for the original line of business, so just replace new Quote() with new CarInsuranceQuote().

var existingQuote = new CarInsuranceQuote(existingQuoteID);

var q = new CarInsuranceQuote(); //ahh, all better...

We ended up doing a bit of both.

In our case, it turned out that most of the constructor usages were on user interface pages, and because we decided we would make brand new pages for the new line of business, the reality was that in most cases we could simply replace the old Quote constructor usages with CarInsuranceQuote constructors.

Since the system was built for car insurance for 20+ years, it is safe to assume that using the car insurance subclass throughout the existing code will be appropriate.

If you’re not sure, just use the factory method and you can always change it later.

Plan to have to work through quite a bit of compiler errors. Take your time and commit often.

This refactoring is mostly a hefty exercise in leaning on the compiler. It’s in your best interest to make small changes and re-compile often in order to see what new errors turn up.

If you make too many changes at once, you’ll easily get overwhelmed at the multi-headed hydra of errors that recursively spawn out of each change and subsequent fix.

Change, recompile, fix, commit, repeat.

Active Record pattern woes

In our case, with the Active Record pattern, we also needed to make sure our child classes still made use of the base class’s existing save and load methods.

public abstract class Quote
{
    public void Save()
    {
        SaveInner();
    }

    public virtual void SaveInner()
    {
        //save properties
    }
}

public class CarInsuranceQuote : Quote
{
    public override SaveInner()
    {
        base.SaveInner();

        //save car-insurance-specific properties
    }
}

public class HomeInsuranceQuote : Quote
{
    public override SaveInner()
    {
        base.SaveInner();

        //save home-insurance-specific properties
    }
}

This meant that all our existing usages of .Save() and .Load() would continue to work and would handle all base class properties, as well as any properties specific to a child class.

Again, if you are fortunate enough to not have to use the Active Record pattern, you may not need to go through this much trouble.

Single vs. Class Table Inheritance

Another decision that will need to be made is how you will store the properties that are unique to your new second line of business.

If you’ve only been using one table for Quote, and you’re making use of a save and load strategy like in the last section, your persistence will need to work out which columns (or tables) to include or exclude depending on which object is being stored or retrieved.

Our application was already using single table inheritance in other areas, so we went with that. We also did not have experience with class table inheritance and felt that single table would be the easiest way to continue reusing existing relationships off the primary key.

At this point you will have two or more very thin child classes and one thick base class. It is now safe to begin using your child classes, but obviously they would not be of much use to you, as all the properties and methods are still in the base class and the distinction between the lines of business is no more than a name.

The final step in this process involves three kinds of modifications to the base class. They are:

1) Pushing down all of the properties and methods from the base class that belong exclusively in the original line of business subclass. 2) Converting some properties and methods to be abstract, with custom line-of-business specific implementations.3) Leaving any others behind that can be shared among all lines.

Step 3: Push down methods, properties, and fields (in that order)

After you’ve got your object constructors back in order, and your persistence set up to correctly handle the new hierarchy, the remaining work involves finding the proper home for the properties and methods that are all hanging out on the base class.

In our example scenario, because the application was originally built specifically for car insurance, that meant that a large majority of the properties and methods could be pushed down to live exclusively on the CarInsuranceQuote subclass.

After all is said and done, the base Quote class would be left with the few properties and methods that are shared across all lines of business. For us, that meant metadata properties like when the quote was created (and by whom), when it was last modified, user notes, helper/utility methods and several abstract member definitions.

Methods first!

Depending on how you chose to do the constructor step, this step could be the most time-consuming, since all your existing code is expecting these class members to exist on the Quote class.

Pushing down a single method could result in hundreds or more compiler errors!

To do this step without wanting to pull your hair out, you’ll find it works best to push methods down first, before trying to push properties or fields down. This is because a property or field likely has many more usages than a method, and a method might not depend on any fields, so moving it cause little to no errors. On the other hand, if you move a field or property, many things could depend on it and they will all error.

So methods first, properties second, fields last.

I believe I learned that tip (and the next one) either from Martin Fowler’s Refactoring book or Joshua Kierevsky’s Refactoring To Patterns book. Both are worth a space on your shelf.

Access modifier flipping for error reduction

When you’re pushing things down, you may see many errors related to usages of private members on the base class. An easy, short term fix is to simply make them protected. They will most likely be pushed down eventually, but you can use this access modifier flipping technique to quickly clear up compiler errors so you can keep moving forward with the refactor.

Depending on how large your base class is, this task will surely test your patience. You’ll be pushing down one field, generating a ton of errors, fixing them (which cause several more, and fixing those (which cause several more…(ad-infinitum)).

I recommend you go very slow, do one thing at a time, recompile often, and fix only those errors that come up from your single change. Once it compiles without error, consider doing a commit to lock in that change and begin another. If you get over-eager and try to do more than one at once, you’ll be looking at potentially hundreds or thousands of errors and you’ll get so overwhelmed trying to fix them that you could make a grave mistake.

I had to start over completely so many times because of this.

I’d get ambitious and try to do too many refactors at once and I’d get lost in a whack-a-mole recursive error hell where one fix would generate 50 more errors. Then I’d feel so uneasy about all the changes that I’d just git reset HEAD --hard and start over.

Small changes and commits!

Wrapping Up

Whew! We made it.

Let’s recap over some of the highlights of this refactor.

Check out the case study article to help you decide if doing this kind of refactor is the best decision for your team.

Your application will likely have one or more root objects that orchestrate the majority of the business processes and logic. These will be your splitting points. The goal is to extract a hierarchy of two or more subclasses that will each hold a separate line of business and that can make use of most of the accessory entities in the application (we benefited most from the reuse of the accounting and payment submodules).

Start by creating the knowledge of the line of business via a type enum. In our example, we went with a single-table inheritance model in our database using an LineOfBusiness descriminator column.

Next handle all the constructor issues that turn up once your existing class becomes an abstract base class.

Finally, use the methods-properties-fields order when pushing down members from the base class into the appropriate child class. I found that order to result in the least amount of headaches.

Keep the class refactoring books close by to reference the Extract Hierarchy, Extract Subclass, and Extract Superclass refactorings.

Once you’ve finished, it may be wise to only make use of your new line-of-business subclass within brand new UI pages so that you don’t break existing code.

Good luck!