REFACTORING MESSY CODE

Creating Highly Configurable Code in Three Simple Steps

Hard-coded, unconfigurable code is headache-inducing

It’s honestly not very difficult to write great, configurable classes. But, it might be if you’ve grown used to doing things in certain ways.

From my experience, the best way to turn an existing class into a configurable one is first to move logic into separate classes and then the moving hardcoded part into constructor or method arguments.

It’s that easy. Let me show you how.

But first, what’s so great about a configurable class?

Before you spend any time refactoring your existing classes into configurable ones, I’ll let you know why it’s so great.

A configurable class is damn easy to use. It’s extensible. It doesn’t do too much. It’s stupid simple, which is awesome.

The biggest selling point, it’s easy to test.

Meet the classes

For good measure, here’s a quick overview of the classes you’re dealing with.

You have an OrderProcessor class with a single method ProcessOrder(Order).

Next, there’s a base class Order and three subclasses VirtualOrder, ServiceOrder, ShippingOrder.

Understanding the Order classes is completely irrelevant. Don’t get distracted by thinking of how they’re implemented. If you really must know, they just have two properties, ‘Name’ and ‘Price’… Okay, let’s carry on.

You’ve implemented the order processor the following way.

It took 1 minute to implement. It works. You’re happy. Your manager is happy. Your customer is happy.

But it’s a mess.

This could be a switch. But if that’s your first thought, you’re focusing on entirely irrelevant aspects of software development.

Let me show you how to make this better

Better is of course subjective. If you’re certain (you’re not) this class will never change (it will), this might be a great implementation (it’s not).

You and I are refactoring this hot mess into a sensible, configurable class — without much effort whatsoever.

We’ll make this class better regarding three internal software qualities.

Readability. Currently, this class’ method reads terribly. I got an easy fix up my sleeve for this.

Maintainability. If you need to modify the processing of a ‘ServiceOrder’, you’ll do so at a place surrounded by unrelated code. Also, if other developers are working on the same class, you probably get merge conflicts. Annoying and potentially introducing unexpected bugs.

Flexibility. Adding a new order? You’ll have to add an extra else-if. I’m sure you already know that’s bad, and a clear violation of Open/Closed.

1 Improving readability by turning IFs into maps

It usually takes a few iterations to refactor a messy class. First, we’re improving the readability. It’s difficult to achieve a highly maintainable and flexible class if it’s unreadable.

Entirely getting rid of if-else is a great first step.

The ProcessOrder(Order) method just got immensely easier to read, and is now doing only one, single job.

If you’re not familiar with Action<T>, it’s just a lambda taking an argument, or, anonymous function if you’d like.

We’ve taken the nasty if-else and made it into a dictionary instead.

Also, notice the added benefit in regards to the property OrderTypes. If a new order type comes along, we don’t need to manually add it in two places.

2 Creating separate classes improves maintainability

We could easily stop at this point and call it a day. You’ve already made the class so much simpler. But you haven’t exactly reached ‘stupid simple’ yet. That’s our end goal.

Currently, with our first refactoring iteration, our class still knows too much about how each type is processed.

In this case, achieving maintainability is about refactoring the action implementations out of the processor class and into their own classes.

In code, this looks like the following — notice the new IOrderProcessing interface. Each lambda action from the dictionary is now its own, separate class.

This incredibly simple refactoring allows us to granularly test each processing. We no longer need to ‘new’ up the whole OrderProcessor just to see if e.g. a virtual order is processed correctly.

Your order processor is starting to look really stupid. Just how we like it.

3 One final refactoring for maximum flexibility

Sure, unit testing is easy at this point. But, you’d still be required to test the ProcessOrder(Order) method three times, just to make sure it’s working properly. Even worse, this is not obvious. You’d need to be aware of the inner workings to know this.

For every new order type, you’d need to test the same method with an additional case. This is surely a code smell.

Your last refactoring is going to make the processor class amazingly simple. You simply turn the dictionary instantiation into an explicit dependency.

So light. So easy. So simple to test.

When your code suddenly becomes stupidly simple, you know you’ve hit the right level of abstraction. This is one of those times. Congrats.

Refactoring to more classes

Computer science majors may shriek in horror as each new class is added, calling out potential performance issues (which never manifests).

Well, there’s a trade-off to everything. Nicely composed code requires more components as opposed to an entangled, unreadable, lightning-fast mess. Pick your poison.

But, again, for good measure, I did a simple performance test. 1 million iterations of instantiating the non-refactored class vs the flexible class, calling the process order method for each order type.

You decide yourself how terrible those numbers are.

Resources for the curious
-------------------------

Making Your C# Code More Object Oriented by Zoran Horvat

Clean Code Principles in C# by Cory House

Code Complete 2 by Steve McConnell

Nicklas Millard is a software development engineer in one of the fastest-growing banks, building mission-critical financial services infrastructure.

Previously, he was a Big4 Senior Tech Consultant developing software for commercial clients and government institutions.

New YouTube Channel (@Nicklas Millard)

Connect on LinkedIn