Refactoring Code to avoid Type Casting

Question

I have following C# code in .Net 4.0. It requires a type casting of IBusiness to IRetailBusiness.

//Type checking
if (bus is IRetailBusiness)
{
       //Type casting
       investmentReturns.Add(new RetailInvestmentReturn((IRetailBusiness)bus));
}

if (bus is IIntellectualRights)
{
       investmentReturns.Add(new IntellectualRightsInvestmentReturn((IIntellectualRights)bus));
}

Business Scenario:

I am designing a software system for and Investment Holding Company. The company has Retail business and IntellectualRights business. BookShop and AudioCDShop are examples of Retail business. EngineDesignPatent and BenzolMedicinePatent are examples of IntellectualRights business. These two business types are totally unrelated.

The investment company has a concept called InvestmentReturn (But each individual business is totally ignorant about this concept). InvestmentReturn is the profit gained from each business and it is calulated using ProfitElement. For each “Business Type” (Retail, IntellectualRights ), the ProfitElement used is different.

QUESTION

How to refactor this class design to avoid this type casting and type checking?

Abstract Investment

public abstract class InvestmentReturn
{
    public double ProfitElement { get; set; }
    public IBusiness Business{ get;  set; }

    public abstract double GetInvestmentProfit();

    public double CalculateBaseProfit()
    {
       double profit = 0;

       if (ProfitElement < 5)
       {
           profit = ProfitElement * 5 / 100;
       }
       else if (ProfitElement < 20)
       {
           profit = ProfitElement * 7 / 100;
       }
       else
       {
           profit = ProfitElement * 10 / 100;
       }

       return profit;
    }
}

Extensions

public class RetailInvestmentReturn : InvestmentReturn
{
    public RetailInvestmentReturn(IRetailBusiness retail)
    {
        Business = retail;
    }

    public override  double GetInvestmentProfit()
    {
        //GrossRevenue is the ProfitElement for RetailBusiness
        ProfitElement = ((IRetailBusiness)Business).GrossRevenue;
        return base.CalculateBaseProfit();
    }  
}

public class IntellectualRightsInvestmentReturn : InvestmentReturn
{

    public IntellectualRightsInvestmentReturn(IIntellectualRights intellectual)
    {
        Business = intellectual;
    }

    public override double GetInvestmentProfit()
    {
        //Royalty is the ProfitElement for IntellectualRights Business
        ProfitElement = ((IIntellectualRights)Business).Royalty;
        return base.CalculateBaseProfit();
    }
}

Client

class Program
{

    static void Main(string[] args)
    {

        #region MyBusines

        List<IBusiness> allMyProfitableBusiness = new List<IBusiness>();

        BookShop bookShop1 = new BookShop(75);
        AudioCDShop cd1Shop = new AudioCDShop(80);
        EngineDesignPatent enginePatent = new EngineDesignPatent(1200);
        BenzolMedicinePatent medicinePatent = new BenzolMedicinePatent(1450);

        allMyProfitableBusiness.Add(bookShop1);
        allMyProfitableBusiness.Add(cd1Shop);
        allMyProfitableBusiness.Add(enginePatent);
        allMyProfitableBusiness.Add(medicinePatent);

        #endregion

        List<InvestmentReturn> investmentReturns = new List<InvestmentReturn>();

        foreach (IBusiness bus in allMyProfitableBusiness)
        {
            //Type checking
            if (bus is IRetailBusiness)
            {
                //Type casting
                investmentReturns.Add(new RetailInvestmentReturn((IRetailBusiness)bus));
            }

            if (bus is IIntellectualRights)
            {
                investmentReturns.Add(new IntellectualRightsInvestmentReturn((IIntellectualRights)bus));
            }
        }

        double totalProfit = 0;
        foreach (var profitelement in investmentReturns)
        {
            totalProfit = totalProfit + profitelement.GetInvestmentProfit();
            Console.WriteLine("Profit: {0:c}", profitelement.GetInvestmentProfit());
        }

        Console.ReadKey();
    }
}

Business Domain Entities

public interface IBusiness
{

}

public abstract class EntityBaseClass
{

}

public interface IRetailBusiness : IBusiness
{
    double GrossRevenue { get; set; }
}

public interface IIntellectualRights : IBusiness
{
    double Royalty { get; set; }
}



#region Intellectuals
public class EngineDesignPatent : EntityBaseClass, IIntellectualRights
{
    public double Royalty { get; set; }
    public EngineDesignPatent(double royalty)
    {
        Royalty = royalty;
    }
}

public class BenzolMedicinePatent : EntityBaseClass, IIntellectualRights
{
    public double Royalty { get; set; }
    public BenzolMedicinePatent(double royalty)
    {
        Royalty = royalty;
    }
}
#endregion

#region Retails
public class BookShop : EntityBaseClass, IRetailBusiness
{
    public double GrossRevenue { get; set; }
    public BookShop(double grossRevenue)
    {
        GrossRevenue = grossRevenue;
    }
}

public class AudioCDShop : EntityBaseClass, IRetailBusiness
{
    public double GrossRevenue { get; set; }
    public AudioCDShop(double grossRevenue)
    {
        GrossRevenue = grossRevenue;
    }
}
#endregion

REFERENCES

1. Refactor my code : Avoiding casting in derived class
2. Cast to generic type in C#
3. How a Visitor implementation can handle unknown nodes
4. Open Closed Principle and Visitor pattern implementation in C#

Answer 1

This solution uses the notions that business interfaces know they must create a return and that their concrete implementations know what kind of concrete return to create.

Step 1 Split InvestmentReturn into two interfaces; the original minus the Business property and a new generic subclass:

public abstract class InvestmentReturn
{
    public double ProfitElement { get; set; }
    public abstract double GetInvestmentProfit();

    public double CalculateBaseProfit()
    {
        // ...
    }
}

public abstract class InvestmentReturn<T>: InvestmentReturn where T : IBusiness
{
    public T Business { get; set; }        
}

Step 2 Inherit from the generic one so you can use Business without casting:

public class RetailInvestmentReturn : InvestmentReturn<IRetailBusiness>
{
    // this won't compile; see **Variation** below for resolution to this problem...
    public RetailInvestmentReturn(IRetailBusiness retail)
    {
        Business = retail;
    }

    public override double GetInvestmentProfit()
    {
        ProfitElement = Business.GrossRevenue;
        return CalculateBaseProfit();
    }
}

Step 3 Add a method to IBusiness that returns an InvestmentReturn:

public interface IBusiness
{
    InvestmentReturn GetReturn();
}

Step 4 Introduce a generic sublcass of EntityBaseClass to provide the default implementation of the above method. If you don't do this you'll have to implement it for all the businesses. If you do do this it means all of your classes where you don't want to repeat the GetReturn() implementation must inherit from the class below, which in turn means they must inherit from EntityBaseClass.

public abstract class BusinessBaseClass<T> : EntityBaseClass, IBusiness where T : InvestmentReturn, new()
{
    public virtual InvestmentReturn GetReturn()
    {
        return new T();
    }
}

Step 5 Implement that method for each of your subclasses if necessary. Below is an example for the BookShop:

public class BookShop : BusinessBaseClass<RetailInvestment>, IRetailBusiness
{
    public double GrossRevenue { get; set; }
    public BookShop(double grossRevenue)
    {
        GrossRevenue = grossRevenue;
    }

    // commented because not inheriting from EntityBaseClass directly
    // public InvestmentReturn GetReturn()
    // {
    //     return new RetailInvestmentReturn(this);
    // }
}

Step 6 Modify your Main to add the instances of InvestmentReturn. You don't have to typecast or type-check because that's already been done earlier in a type safe way:

    static void Main(string[] args)
    {
        var allMyProfitableBusiness = new List<IBusiness>();
        // ...
        var investmentReturns = allMyProfitableBusiness.Select(bus => bus.GetReturn()).ToList();
        // ...
    }

If you don't want your concrete businesses to know anything about creating an InvestmentReturn—only knowing that they must create one when asked—then you'll probably want to modify this pattern to incorporate a factory that creates returns given input (e.g. a map between IBusiness implementations and InvestmentReturn subtypes).

Variation

All of the above works fine and will compile if you remove the investment return constructors that set the Business property. Doing this means setting Business elsewhere. That might not be desirable.

An alternative to that would be to set the Business property inside GetReturn. I found a way to do that, but it really starts to make the classes look messy. It's here for your evaluation as to whether its worth it.

Remove the non-default constructor from RetailInvestmentReturn:

public class RetailInvestmentReturn : InvestmentReturn<IRetailBusiness>
{
   public override double GetInvestmentProfit()
   {
       ProfitElement = Business.GrossRevenue;
       return CalculateBaseProfit();
   }
}

Change BusinessBaseClass. This is where it gets messy with a double-cast, but at least it's limited to one place.

public abstract class BusinessBaseClass<T, U> : EntityBaseClass, IBusiness
    where T : InvestmentReturn<U>, new()
    where U : IBusiness
{
    public double GrossRevenue { get; set; }

    public virtual InvestmentReturn GetReturn()
    {
        return new T { Business = (U)(object)this };
    }
}

Finally change your businesses. Here's an example for BookShop:

public class BookShop : BusinessBaseClass<RetailInvestmentReturn, IRetailBusiness>, IRetailBusiness
{
    // ...
}