The Dark Side of Switch Statements — Analysis of Object Types

switch statements are considered harmful in OOP. But does that mean that we should always remove them from our code? Are some cases more dangerous than others? Or maybe sneaking a switch into a method can be an appropriate solution?

This article is the first in the series that explores the usage and associated dangers of switch statements.

But first — a few words about naming. Since both switch/case and a series of if/else statements serve the same purpose, I will refer to both of them as explicit case analysis (ECA) — the term I borrowed from Arthur J. Riel¹.

Explicit case analysis of types of objects in object-oriented code

Explicit case analysis of types of objects is the most dangerous kind of ECA as it can give you the worst maintenance problems. It is easy to forget to add an appropriate if or case when you add a new class, and this may lead to incorrect behavior of the system. Luckily, it is easy to spot and to fix. Let's take a look at the example.

Imagine that you are building a website to help people find and adopt cats from various shelters across the country. The website should allow people to specify filter criteria to find cats they would be able to take the best care of.

You start with creating two filters: one for the size of a cat, and another for the distance to a shelter:

public class SizeFilter
{
    public CatSize Size { get; set; }
}

public class DistanceFilter
{
    public Location Location { get; set; }

    public int Distance { get; set; }
}

You model a cat in a basic Cat class:

public class Cat
{
    public Location ShelterLocation { get; set; }

    public CatSize Size { get; set; }

    public bool RequiresSpecialCare { get; set; }
}

Finally, you create CatMatcher class:

public class CatMatcher
{
    public bool MatchesFilters(Cat cat, object[] filters)
    {
        foreach (var filter in filters)
        {
            if (!MatchesFilter(cat, filter))
                return false;
        }

        return true;
    }

    private bool MatchesFilter(Cat cat, object filter)
    {
        switch (filter)
        {
            case SizeFilter sizeCriteria:
                return cat.Size == sizeCriteria.Size;

            case DistanceFilter distanceCriteria:
                var distanceToShelter = cat.ShelterLocation
                        .DistanceTo(distanceCriteria.Location);

                return distanceToShelter <= distanceCriteria.Distance;

            default:
                throw new ArgumentException(
                    $"Unknown cat filter: {filter.GetType()}", nameof(filter));
        }
    }
}

MatchesFilters method accepts a cat and a collection of filters to check the cat against. It then goes through each filter and passes it to MatchesFilter method to check if the cat satisfies the filter. For SizeFilter, the method compares the size of the cat, and for DistanceFilter, it checks the distance between the shelter that houses the cat and the person's location.

The method explicitly checks the type of a filter and performs a corresponding filtering operation. There are two reasons why this approach is wrong:

1. The method treats filters as data structures — objects without behavior. It pulls all the meaningful logic out of the filters and centralizes it. This is not a proper object-oriented design.
2. The method creates a dependency on the types of filters.

The first problem is breaking the vital principle of class design: keep related data and behavior in one place. Should you decide to add a weight criteria to SizeFilter, or, say, measure distance by time traveled rather than kilometers in DistanceFilter, you will need to change both the filter objects and MatchesFilter method. Classes must own both their data and related logic, so when they have to change, they change in only one place.

The second problem is forcing developers to follow an implicit convention when adding new filters. Say, you decide to add SpecialCareFilter to search for cats that need special care. You add a class, but for it to work, you will need to search for all methods that have a switch or an if statements on filter types and update all of them to process SpecialCareFilter properly. You need to know about this convention to make the change. Since there is no compiler support to indicate that you've missed changing all of the switch statements, it can result in incorrect behavior.

Both problems are easy to solve with polymorphism.

You start by creating a method in each of the filter objects and pull the logic contained in each case statement into the according filter. This solves the first problem — your objects now own their meaningful logic.

Then, you make the MatchesFilter method treat all of the filters in the same way, effectively removing the switch statement. You do this by making the method call the filter classes' own methods. This solves the second problem.

Let's refactor the example.

First of all, you need to define a common abstraction for a filter that would allow you to treat all the filters the same. In this case, a simple interface will do the job.

public interface ICatFilter
{
    bool IsMatch(Cat cat);
}

The second step is to change the filters objects to implement the interface and perform the cat matching logic.

public class SizeFilter : ICatFilter
{
    private readonly CatSize _size;

    public SizeFilter(CatSize size)
    {
        _size = size;
    }

    public bool IsMatch(Cat cat)
    {
        return cat.Size == _size;
    }
}

public class DistanceFilter : ICatFilter
{
    private readonly Location _location;
    private readonly int _distance;

    public DistanceFilter(Location location, int distance)
    {
        _location = location;
        _distance = distance;
    }

    public bool IsMatch(Cat cat)
    {
        var distanceToShelter = cat.ShelterLocation.DistanceTo(_location);

        return distanceToShelter <= _distance;
    }
}

Note that each filter now uses its data to decide if the cat is a match. Therefore, the data fields can be made private — it is always a good practice to encapsulate data by default.

Finally, you change the CatMatcher class:

public class CatMatcher
{
    public bool MatchesFilters(Cat cat, ICatFilter[] filters)
    {
        foreach (var filter in filters)
        {
            if (!filter.IsMatch(cat))
                return false;
        }

        return true;
    }
}

The new version of CatMatcher class replaces the old MatchesFilter method that contained the switch statement with the single call to filter.IsMatch(). And what's even better — this call won't change if you decide to add or remove a filter.

Adding a new filter is now easy: you just add a class, and that’s it. The new class performs all the filtering logic itself. Let's create SpecialCareFilter:

public class SpecialCareFilter : ICatFilter
{
    private readonly bool _requiresSpecialCare;

    public SpecialCareFilter(bool requiresSpecialCare)
    {
        _requiresSpecialCare = requiresSpecialCare;
    }

    public bool IsMatch(Cat cat)
    {
        return cat.RequiresSpecialCare == _requiresSpecialCare;
    }
}

When passed as an element of ICatFilter[] filters collection to CatMatcher.MatchesFilters method, SpecialCareFilter will just work without any additional changes.

You have effectively changed explicit case analysis to an implicit one — the one that is being performed by the runtime automatically with the help of polymorphism.

Faking polymorphism is a common misuse of switch statements. Prefer creating a hierarchy of polymorphic classes to doing explicit case analysis of a type.

Explicit case analysis of types of objects in procedural code

Explicit case analysis of types of objects is sometimes preferable to creating a hierarchy of polymorphic classes. As you saw in the previous example, using a switch statement made us pull the logic from objects into a centralized function. It also created a convention for developers to follow. And as you just learned, this approach may seem easier to implement, but it may lead to maintenance issues.

But what if you know that in your domain, the function that performs ECA may change often and the set of objects it operates on won't change ever? If you don't expect the objects to change, the convention ECA creates is not a problem anymore. This is the case when procedural code may be a more appropriate and straightforward solution².

Having procedural code, however, is a rare case. It is likely for an application to get a new type, and using ECA is often a short-sighted, brute-force solution¹. Treat every instance of ECA with the utmost suspicion.

Summary

Having a switch or an if statement that analyzes types of objects is the most dangerous type of explicit case analysis. It is often a poor attempt to simulate polymorphism: it promotes the use of procedural code and forces developers to program by convention, which may lead to maintenance issues. Consider replacing such instances of case analysis with true polymorphism.

On the other hand, this kind of explicit case analysis may be appropriate in procedural code. However, since having procedural code is rarely justified, you should be suspicious of every instance of such ECA.

References

1. Riel, A. J. (1996). Object-Oriented Design Heuristics (1st ed.). Addison-Wesley Professional. Goodreads ↩ ↩²

2. Martin, R. C. (2008). Clean Code: A Handbook of Agile Software Craftsmanship (1st ed.). Pearson. Goodreads ↩