Polymorphism Part 2: Refactoring to Polymorphic Behavior

I spoke at the Houston C# User Group earlier this year.  Before my talk Peter Seale did an introductory presentation on refactoring.  He had sample code to calculate discounts on an order based on the number of items in the shopping cart.  There were several opportunities for refactoring in his sample.  He asked the audience how they thought the code sample could be improved.  He got several responses like making it easier to read and reducing duplicated code.  My response was a bit different; while the code worked just fine and did the job, it was very procedural in nature and did not take advantage of the object-oriented features available in the language.  

One of the most important, but overlooked refactoring strategies is converting logic branches to polymorphic behavior.  Reducing complicated branching can yield significant results in simplifying your code base, making it easier to test and read.

The Evils of the switch Statement

One of my first large applications that I had a substantial influence on the design of the application had some code that looked like this:

private string SetDefaultEditableText()
{
  StringBuilder editableText = new StringBuilder();
    switch ( SurveyManager.CurrentSurvey.TypeID )
    {
        case 1:
        editableText.Append("<p>Text for Survey Type 2 Goes Here</p>");                            
        case 2:
        editableText.Append("<p>Text for Survey Type 2 Goes Here</p>");
        case 3:
        default:
        editableText.Append("<p>Text for Survey Type 3 Goes Here</p>");
    }
    return editableText.ToString();
} 

Now there are a lot of problems with this code (a Singleton, really).  But I want to focus on the use of the switch statement. As a language feature, the switch statement can be very useful. But when designing a large-scale application it can be crippling and using it breaks a lot of OOD principles.  For starters if you use switch statement like this in your code, chances are you are going to need to do it again.  Now you’ve got duplicated logic scattered about your application.  If you ever need to add a new case branch to your switch statement you now have to go through the entire application code base and look where you used these statements and change them.

What is really happening is changing the behavior of our app based on some condition.  We can do the same thing using polymorphism and make our system less complex and easier to maintain.  Suppose you are running a Software as a Service application and you’ve got a couple of different premium services that you charge for.  One of them is a flat fee and the other service fee is calculated by the number of users on the account.  The procedural approach to this might be done by creating an enum for the service type and then use switch statement to branch the logic.

public enum ServiceTypeEnum
{
    ServiceA, ServiceB
} 

public class Account
{
    public int NumOfUsers{get;set;}
    public ServiceTypeEnum[] ServiceEnums { get; set; }
} 

// calculate the service fee
public double CalculateServiceFeeUsingEnum(Account acct)
{
    double totalFee = 0;
    foreach (var service in acct.ServiceEnums) { 

        switch (service)
        {
            case ServiceTypeEnum.ServiceA:
                totalFee += acct.NumOfUsers * 5;
                break;
            case ServiceTypeEnum.ServiceB:
                totalFee += 10;
                break;
        }
    }
    return totalFee;
} 

This has all of the same problems as the code above. As the application gets bigger, the chances of having similar branch statements are going to increase.  Also as you roll out more premium services you’ll have to continually modify this code, which violates the Open-Closed Principle.  There are other problems here too.  The function to calculate service fee should not need to know the actual amounts of each service.  That is information that needs to be encapsulated.

A slight aside: enums are a very limited data structure.  If you are not using an enum for what it really is, a labeled integer, you need a class to truly model the abstraction correctly.  You can use Jimmy’s awesome Enumeration class to use classes to also us them as labels.

Let’s refactor this to use polymorphic behavior.  What we need is abstraction that will allow us to contain the behavior necessary to calculate the fee for a service.

public interface ICalculateServiceFee
{
    double CalculateServiceFee(Account acct);
} 

Several people in my previous post asked me why I started with an interface and if by doing so is it really polymorphism.  My coding style is generally favors composition than inheritance (which I hope to discuss later), so I generally don’t have deep inheritance trees.  Going by the definition of I provided: “Polymorphism lets you have different behavior for sub types, while keeping a consistent contract.”  it really doesn’t matter if it starts with an interface or a base class as you get the same benefits. I would not introduce a base class until I really needed too.

Now we can create our concrete implementations of the interface and attach them the account.

public class Account{
    public int NumOfUsers{get;set;}
    public ICalculateServiceFee[] Services { get; set; }
} 

public class ServiceA : ICalculateServiceFee
{
    double feePerUser = 5; 

    public double CalculateServiceFee(Account acct)
    {
        return acct.NumOfUsers * feePerUser;
    }
} 

public class ServiceB : ICalculateServiceFee
{
    double serviceFee = 10;
    public double CalculateServiceFee(Account acct)
    {
        return serviceFee;
    }
} 

Now we can calculate the total service fee using these abstractions.

public double CalculateServiceFee(Account acct){
    double totalFee = 0;
    foreach (var svc in acct.Services)
    {
        totalFee += svc.CalculateServiceFee(acct);
    }
    return totalFee;
} 

Now we’ve completely abstracted the details of how to calculate service fees into simple, easy to understand classes that are also much easier test. Creating a new service type can be done without changing the code for calculating the total service fee.

public class ServiceC : ICalculateServiceFee
{
    double serviceFee = 15;
    public double CalculateServiceFee(Account acct)
    {
        return serviceFee;
    }
} 

But now we have introduced some duplicated code, since the new service behaves the same as ServiceB.  This is the point where a base class is useful.  We can pull up the duplicated code into base classes.

public abstract class PerUserServiceFee : ICalculateServiceFee
{
    private double feePerUser;
    public PerUserServiceFee(double feePerUser)
    {
        this.feePerUser = feePerUser;
    }
    public double CalculateServiceFee(Account acct){
        return  feePerUser * acct.NumOfUsers; 
   }
}
public abstract class MonthlyServiceFee : ICalculateServiceFee
{
    private  double serviceFee;
    public MonthlyServiceFee(double serviceFee)
    {
        this.serviceFee = serviceFee;
    } 

    public double CalculateServiceFee(Account acct)
    {
        return serviceFee;
    }
} 

Now our concrete classes just need to pass the serviceFee value to their respective base classes by using the base keyword as part of their constructor.

public class ServiceA : PerUserServiceFee
{
    public ServiceA() : base(5) { }
} 

public class ServiceB : MonthlyServiceFee 
{
    public ServiceB() : base(10) { }
} 

public class ServiceC : MonthlyServiceFee
{
    public ServiceC() : base(15) { }
} 

Also, because we started with the interface and our base classes implement it, none of the existing code needs to change because of this refactor.

Next time you catch yourself using a switch statement, or even an if-else statement, consider using the object-oriented features at your disposal first.  By creating abstractions for behavior, your application will be a lot easier to manage.

Related Articles:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://developingux.com Caleb Jenkins

    ” I would not introduce a base class until I really needed too”

    Just like you demonstrated.. as a general rule, if I ever feel that a base class would add value, I would still code against the Interface, and simply have the base class implement that Interface.

    Nice write up, Thanks John!

    • John Teague

      Yeah, it’s an important point.  I’ll edit it to emphasize this.

      Thanks!

  • Marcin

    You forgot to implement CalculateServiceFee() in PerUserServiceFee class. Right? ;)  

    • John Teague

      yeah, I added it.

  • Pingback: The Morning Brew - Chris Alcock » The Morning Brew #1309

  • Aleksandar Dragojević

    Nice article, good job, I personally prefer coding to the interface since it makes integrating DI a breeze later on, among other things. :)

  • http://twitter.com/twit_daffers DG

    Great little article to remind us of the importance of keeping an eye out for this kind of smell. One thing i always want to see when examples like this are shown is how you setup the Account, how do you assign it its calculator. I always feel like you end up with a switch/if to do that.

    • John Teague

      Yeah, good point. There are a couple of ways to do this. In this case, since they are simple objects and all self contained, you could use the Enumeration pattern I linked to in the article. It gives you a way to the list of possible objects as an array of strings that you could use to bind to a user interface. Then once selected it gives you a way to fetch the object from the string. We use this a lot to list our enumerated classes and rehydrate them.

      If however, the objects are more complicated to build and you need more information at initialization time, then use the factory pattern. Inside the factory there will be a switch statement of some kind, but now you’ve isolated that switch to one, *and only one*, location in your application. So if you do need to change it, it only happens in one place.

  • MBR

    Let me offer a different view — that the switch statement isn’t “code smell”, but the “refactoring” often is. Consider what one accomplishes with this sort of re-factoring:

    (1) You still have a switch statement of sorts – however, instead of it being completely under your control, you have now relagated responsibilty to the compiler’s single-inheritence v-table dispatch implementation, which you can not control the behavior of — you are now conflating the semantics of the programming language you are using with the the semantics of your domain.

    (2) You have possibly added very serious costs and impedance-mismatches further down the chain, such as changing how serialization and object-mapping function, changing the signature of proxys/stubs to expose as web-services, etc.

    (3) You have taken a localized desription of rules that are easily viewed all in one place and scattered them among several classes, and perhaps several source files. I would argue that this certainly does not make the code “easier to test and read”. One set of rules in one place is easier to read, and a single funciton that maps an integer to a string is easier to test than a test requiring several sub-classes of mock objects to be created.

    (4) You have taken a step backwards from solving the problem generally – you have taken a “table based” view of your problem, which could easily have been externalized as a workflow in a scripting language or a set of rules in a database, and pushed your logic further into pre-compiled code and a distinct set of classes. Manipulating data is much easier than managing code (meta-programming techniques such as reflection, dynamic code generation, etc.), and offers much more flexibility.

    It could be that in a particular case (especially as the consumer of a large web/GUI/etc. framework that uses OOP formalisms) that this type of refactoring is reducing friction overall , but it certainly isn’t a “gimme” that it’s always the right thing to do. (Using the phrase “code smell” is

    • http://ivonna.biz/blog.aspx Artëm Smirnov

      I guess it depends on the complexity of the original switch statement. Sure in this example it’s better to leave it as it is. However, when it grows up, I’d prefer it scattered among several classes rather than several screens in one file.

    • John Teague

      That’s an interesting opinion. I respect but obviously don’t agree, especially when the code is more difficult than what I can put in a blog post as Artem said. So good luck with that.