Duplication in code often arises organically, and can cause problems like missing updates of the duplicated cases, or just obscuring the actual flow through the code due to large amounts of boilerplate code. Refactoring that code to eliminate duplication can take a bit of knowledge on how to break down that code into its similar and differing parts, and help from some of C#’s more interesting features, such as generics.

Recently, my coworker was working on adding features to some very old code and noticed similarities between several classes. She immediately decided a refactor was in order to bring these similar classes together to reduce duplication and make it easier to update. Her plan was to pull all of the common code into the currently-empty base class. However, a bit later, she came over to me: 

“Anne, I know I’d said that I was planning on refactoring these classes, but on a second look, they have some fundamental differences and I’m not really sure what to do with those.”

I told her, “Let’s look at what you’re dealing with—I bet there’s a way to do it, and we can work on it together!” With that, I pulled up my chair and we started working through the problem.

For context, these similar classes were controllers that were more or less a passthrough API to another internally-facing API. We were dealing with code much like the example below, which includes multiple services that provide information on shoppers to an external metrics service using a second, internal API. This external metrics service calls into all of these services to collect specific aspects of a shopper’s data from a purchase along with with their answers to a survey handed to them by a cashier.  There were small differences in behavior and in the wrapping of the data that each controller did, but overall, the controllers had a lot of similarities. When we started the refactor, we looked at two classes: CashierDataController and PurchaseDataController. They looked like this:

These classes are clearly very similar, but they have some important differences:

  • They each call different API methods on the protected API
  • They call the API with different data
  • They receive different data
  • They use a different field for the ID
  • In the case of the Purchase method, the API deals with a collection, not a single item
  • The name of the called method on the two controllers was different

These were the differences that my colleague was having trouble navigating: yet the two methods do exactly the same things.

After determining that all of the logical steps performed were the same between the two classes, we started our refactor by looking at the CashierController.

Step 1: Extract the body of the API method

As each of these classes had a different exposed API name, we decided to start by extracting out the body of the API method into a separate method—then we’d be dealing with a chunk of code that we could eventually use in all of our controllers. We could then try stuffing this new method into our previously-empty base class controller to see where that got us. So, CashierDataController became this:

And BaseController becomes:

Step 2: Generalize the request and response

Given how much CashierData-specific information is in GetMetricsData, the method as-is will clearly not work for PurchaseController, but at least GetMetricsData is in a place where both classes can get to it. GetMetricsData is currently far too specific for the CashierDataController case—it’s got to work for all controllers that inherit from BaseController, meaning it’s got to be generalized.

Let’s look at one of those Cashier-specific specific types in our extracted method: CashierProfileRequest. How might we generalize CashierProfileRequest?  This is a perfect case for generics. Instead of specifying the request will be CashierProfileRequest, we’ll say it’s … something. Same for CashierProfileResponse—it has to be … something.

Let’s extract those out:

Step 3: Generalize setting the Id for our request

We’ve generalized the actual response type, but now that CashierId is a problem—now that we’re not setting the Id on a CashierRequest, our code no longer compiles. The problem is, API method takes a different class, with a different Id field. How might we handle that difference?  Well, we can offload the actual Id setting to the implementing class with an abstract method  on our base class so each controller can set the Id it needs. (We’ll have to make the BaseController abstract too, of course, but we aren’t actually calling that anyway). We’ll make an abstract method that our base can use from GetMetricsData:

protected abstract void SetIdOnData(TRequest request, string id);

Then we can use our new SetIdOnData in our GetMetrics Data:

And add the specific implementation on our CashierController

        protected override void SetIdOnData(CashierProfileRequest 
request, string id) => request.CashierId = id;

Step 4: Generalize pulling the data for our request

We’ve generalized setting the Id, but we still have a similar situation with pulling the data from our internal request—each response has the data contained in a unique structure, and so our earlier refactoring caused an error pulling our data, just like it did with the Id. To fix the problem, we can do a similar refactoring for the response that we did with the Id.

Currently, we pull the response like this, which we can’t have in our base class:

var cashierDto = restResponse.Data.CashierProfileDto;

Instead, we’ll move this to an abstract method and make the inheritors handle this logic:

        protected abstract TResponseData 
GetDtoFromResponse(IRestResponse<TResponse> response);

TResponseData is a new type that we will need to add to our generics list at the top of our class.

Our data retrieval in GetMetricsData turns into

                var restResponse = client.Execute<TResponse>(request);
                var cashierDto = GetDtoFromResponse(restResponse);
                var result = Mapper.Map<TResponseData,
MetricsCashier>(cashierDto);

And finally in our CashierController,

        protected override CashierProfileDto 
GetDtoFromResponse(IRestResponse<CashierProfileResponse> response) =>
response.Data.CashierProfileDto;

Step 5: Genericize the resource

We are now a lot closer, but the resource is still a Cashier-specific string.  Moving the resource string is just a matter of adding on a new string parameter to our GetMetricsData and passing that resource in from our callers.

That change leaves our controllers like this:

Step 6: Let’s try a different controller

We are now really close to this class being usable for both situations. All we need to do is extract out the additional type in the Mapper.Map class, handle the error message, and we’re done.  At this point, I suggested to my colleague, “Okay, you can take the rest of the refactoring on your own, but let’s first quickly sub in the PurchasedItemController class to see what that would look like:”

Implementing for the Purchased Item is almost there!  Of note here is that unlike the Cashier resource, the Purchase resource returns a list of DTOs, so we needed to make our TResponseData a list of DTOs instead of just a single DTO. This works just fine with our generics. 

Step 7: Fix the return, types

One missing piece that is still preventing compilation is that the return type still needs to be updated so we can return a collection if needed. Let’s quickly take care of this now, which will mean another generic type added to the URL:

We’ve extracted out the pieces that need to be different, while consolidating the pieces are really the same. If the core logic of how we wrap and unwrap these calls changes, we will only need to do it in one place. The logic in the base class is still clear to read, and the pieces in the inherited classes only contain the differences between them.

Let's Talk

Have a tech-oriented question? We'd love to talk.