Question

I have following class design. The complete code is available in " How to achieve this functionality using Generics? ". The code works fine and resolves the casting issue mentioned in " Refactoring Code to avoid Type Casting "

In the RetailInvestmentReturnCalculator class, the GetInvestmentProfit() method utilizes CalculateBaseProfit() method present in InvestmentReturnCalculator abstract base class. This fact is not evident from the class design.

QUESTION

  1. How to refactor this class design to convey the above mentioned fact?
  2. What is the design guideline that will prevent this sort of design mistakes?

Note: Martin Fowler: Is Design Dead? says

What do we mean by a software architecture? To me the term architecture conveys a notion of the core elements of the system, the pieces that are difficult to change. A foundation on which the rest must be built

Class Diagram

enter image description here

Abstract

public abstract class InvestmentReturnCalculator
{
    #region Public

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

    #endregion

    #region Protected

    protected  double CalculateBaseProfit()
    {
        double profit = 0;
        if (ProfitElement < 5)
        {
            profit = ProfitElement * 5 / 100;
        }
        else
        {
            profit = ProfitElement * 10 / 100;
        }
        return profit;
    }

    #endregion
}

public abstract class InvestmentReturnCalculator<T> : InvestmentReturnCalculator where T : IBusiness
{
    public T BusinessType { get; set; }
}

Concrete

public class RetailInvestmentReturnCalculator : InvestmentReturnCalculator<IRetailBusiness>
{
    public RetailInvestmentReturnCalculator(IRetailBusiness retail)
    {
        BusinessType = retail;
        //Business = new BookShop(100);
    }

    public override double GetInvestmentProfit()
    {
        ProfitElement = BusinessType.GrossRevenue;
        return CalculateBaseProfit();
    }
}
Was it helpful?

Solution

The ProfitElement field is rather ugly. I would make it an abstract property on InvestmentReturnCalculator and implement it in base classes (rather than setting the value) - this is called the template method pattern. Then you don't need the GetInvestmentProfit() method.

public abstract class InvestmentReturnCalculator
{
    #region Public

    public abstract double ProfitElement { get; }

    #endregion

    #region Protected

    public double GetInvestmentProfit()
    {
        double profit = 0;
        if (ProfitElement < 5)
        {
            profit = ProfitElement * 5 / 100;
        }
        else
        {
            profit = ProfitElement * 10 / 100;
        }
        return profit;
    }

    #endregion
}

public abstract class InvestmentReturnCalculator<T> : InvestmentReturnCalculator where T : IBusiness
{
    public T BusinessType { get; set; }
}

public class RetailInvestmentReturnCalculator : InvestmentReturnCalculator<IRetailBusiness>
{
    public RetailInvestmentReturnCalculator(IRetailBusiness retail)
    {
        BusinessType = retail;
        //Business = new BookShop(100);
    }

    public override double ProfitElement {get { return BusinessType.GrossRevenue;}}

}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top