Real-World Refactoring: Available Cars

Are all of those if..then statements really necessary? In today's post, we look at refactoring a long list of if..then statements into an easy two-liner.

Written by Jonathan "JD" Danylko • Last Updated: • Develop •

Cars lined up in a garage.

Refactoring code is something developers should focus on and, as my one friend said, "Relentlessly Refactor."

Refactoring makes code easier to read and easier to maintain so it makes sense to practice (i.e. katas or competitions) every once in a while to keep your skills sharpened.

About a week ago, I was working with a client and came across some code that didn't sit right with me so I decided to ask my newsletter readers if they could refactor the code.

In Friday's newsletter, I added the exercise to the newsletter.

The exercise contained a simple scenario.

There was one method to create a list of strings based on a number of conditions. If a condition was met, the related string was included in the list and returned from the calling method.

The code in question was this:

private string[] GetAvailable(CarRequest request)
{
    var result = new List<string>();

    if (request.State == "CA")
    {
        result.Add("BMW");
    }

    if (request.State == "CA" && request.Imported)
    {
        result.Add("Ferrari");
    }

    if (request.State == "MA")
    {
        result.Add("Jeep Cherokee");
    }

    if (request.State == "PA")
    {
        result.Add("Jeep Cherokee");
    }

    return result.ToArray();
}

Based on the CarRequest, we examine the buyer's state and determine if they want an import. Once we examine the buyer's request, we return a list of cars available for them.

As you can see, this can get a little out of hand. Of course, you could make this data-driven with a set of rules, but for now, let's just look at an initial refactoring for this exercise.

Why Refactor This?

While I've mentioned this before, there are two immediate reasons that come to mind when I see this type of code.

Eliminate Cyclomatic Complexity

One of my friends early in my career said,

"If you were to create a truly object-oriented application, it would have no if..then statements in the code."

I think of his comment every single time I see code like this.

Think about how you refactor this code to not use if..then statements.

The more if..then statements you have in your code, the more the cyclomatic complexity will increase. In other words, "spaghetti code."

Every condition used can send you down a different path making it harder to follow. The less if..then statements in the code, the easier it is to understand and maintain.

Open-Closed Principle

If you're familiar with the SOLID principles, you already know where I'm going with the Open-Closed Principle.

The Open-Closed principle means your code should be "Open for extension, Closed for modification." This gives developers a means to extend the code without copy/pasting.

Your code should be easy enough to extend the code without much effort.

A [Possible] Solution

So let's get back to our exercise.

The pattern I see is a condition (request.State == "CA") and a result ("BMW"). There can be other pairing patterns like a condition/Action pairing. If a condition is met, kick-off this Action.

As a first pass, I see this code refactored into this:

private string[] GetAvailable(CarRequest request)
{
    var conditions = new Dictionary<string, bool>
    {
        { "BMW", request.State == "CA" },
        { "Ferrari", request.State == "CA" && request.Imported },
        { "Jeep Cherokee", request.State == "MA"},
        { "Jeep Cherokee", request.State == "PA"}
    };

   return conditions
        .Where(e=> e.Value)
        .Select(e=> e.Key)
        .ToArray();
}

These two lines of code provide all of the functionality of the previous code listed above, simplifies the code, and returns a list of cars the buyer can purchase based on where they're willing to travel.

How much complexity did this introduce compared to an if..statement or inline-if?

  1. Hiding Cyclomatic Complexity

Conclusion

This simple exercise made me look back over older code and think about how to refactor complex code into newer and simpler approaches. With this "first pass" of refactoring, you can even apply this concept to switch statements.

If you are looking for more ways to refactor code, check out the Real-World Refactoring Collection.

As readers of C#, was this an easier approach or did it seem harder to understand? How did you refactor this out? Post your comments below and let's discuss.

ASP.NET 8 Best Practices on Amazon

ASP.NET 8 Best Practices by Jonathan Danylko


Reviewed as a "comprehensive guide" and a "roadmap to excellence" with over 120 Best Practices for ASP.NET Core 8, Jonathan's first book by Packt Publishing explores proven techniques for every phase of the SDLC.

Learn industry-standard concepts to improve your coding, debugging, and deployment of ASP.NET Core websites.

Order now on Amazon.com button

Picture of Jonathan "JD" Danylko

Jonathan "JD" Danylko is an author, web architect, and entrepreneur who's been programming for over 30 years. He's developed websites for small, medium, and Fortune 500 companies since 1996.

He currently works at Insight Enterprises as an Architect.

When asked what he likes to do in his spare time, he replies, "I like to write and I like to code. I also like to write about code."

comments powered by Disqus