refactoring code with too many conditional statements

i was once talking with the IS director of a certain company about the tell tale signs of bad code. i mentioned a few of the items that raise flags for me: code duplication, low cohesion in classes, etc.

the director, which happens to be a really good coder even though he's been in management for several years, mentioned that when he sees code with too many if statements (or any conditional branching for that matter), he knows the code could be cleaned up. after hearing his statement, i started considering on how to clean code that has too many if statements.

last week i had the opportunity to help a coworker refactor some code that looked something like this:

public class CreditCardService()
{
  public void DoWork()
  {
       if(creditCardCode = "VISA")
       {
            //about 10 or so lines of code here
       }
       else if (creditCardCode = "MC")
       {
            //about 10 or so lines of code here
            //the code is very similar in all branches
       }
       //a few more branches for the other credit cards we support
       //...
  }
}

the obvious problem with the code above is that it's not making good use of a basic programming principle: polymorphism.

we could clean up the above code by:

• writing a base class that implements the common functionality across all credit cards
• writing children credit card classes that inherit from the base class and implement what's different in each credit card
• writing a credit card factory that return the right implementation to the CreditCardService class

so, we should end up with code looking something like this:

public class BaseCreditCard
{
     //all common fields go here

     public void DoWork()
     {
          //all common functionality goes here
     }
}

public class VisaCreditCard : BaseCreditCard
{
     //all fields pertaining to VISA go here
     
    public void DoWork()
    {
         base.DoWork();
         //visa specific functionality goes here
    }
}

public class CrediCardFactory()
{
     public static BaseCreditCard GetCard(string cardType)
     {
          //return appropriate credit card child class
          if(cardType.Equals("VISA")
               return new VisaCreditCard();
          //more code like the one above
     }
}
public class CreditCardService()
{
     private CreditCard card;
     public CreditCardService(string cardType)
     {
          card = CreditCardFactory.GetCard(cardType)
     }

     public void DoWork()
     {
          card.DoWork();
     }    
}

ideally, our factory will be really smart about picking the right credit card type, so that all of the if statements necessary to pick the right credit card will all be contained in it.

using polymorphism in this case makes the code much easier to read, and way easier to maintain as we now have all decision logic in one place, all common logic in another place, and type specific logic in its own place.