Design pattern for cost calculator app?

Question

Hi,

I have a problem that I’ve tried to get help for before, but I wasn’t able to solve it then, so I’m trying to simplify the problem now to see if I can get some more concrete help with this because it is driving me crazy…

Basically, I have a working (more complex) version of this application, which is a project cost calculator. But because I am at the same time trying to learn to design my applications better, I would like some input on how I could improve this design. Basically the main thing I want is input on the conditionals that (here) appear repeated in two places. The suggestions I got before was to use the strategy pattern or factory pattern. I also know about the Martin Fowler book with the suggestion to Refactor conditional with polymorphism. I understand that principle in his simpler example. But how can I do either of these things here (if any would be suitable)? The way I see it, the calculation is dependent on a couple of conditions: 1. What kind of service is it, writing or analysis? 2. Is the project small, medium or large? (Please note that there may be other parameters as well, equally different, such as “are the products new or previously existing?” So such parameters should be possible to add, but I tried to keep the example simple with only two parameters to be able to get concrete help)

So refactoring with polymorphism would imply creating a number of subclasses, which I already have for the first condition (type of service), and should I really create more subclasses for the second condition as well (size)? What would that become, AnalysisSmall, AnalysisMedium, AnalysisLarge, WritingSmall, etc…??? No, I know that’s not good, I just don’t see how to work with that pattern anyway else?

I see the same problem basically for the suggestions of using the strategy pattern (and the factory pattern as I see it would just be a helper to achieve the polymorphism above). So please, if anyone has concrete suggestions as to how to design these classes the best way I would be really grateful! Please also consider whether I have chosen the objects correctly too, or if they need to be redesigned. (Responses like "you should consider the factory pattern" will obviously not be helpful... I've already been down that road and I'm stumped at precisely how in this case)

Regards,

Anders

The code (very simplified, don’t mind the fact that I’m using strings instead of enums, not using a config file for data etc, that will be done as necessary in the real application once I get the hang of these design problems):

public abstract class Service
{
    protected Dictionary<string, int> _hours;
    protected const int SMALL = 2;
    protected const int MEDIUM = 8;

    public int NumberOfProducts { get; set; }
    public abstract int GetHours();
}

public class Writing : Service
{
    public Writing(int numberOfProducts)
    {
        NumberOfProducts = numberOfProducts;
        _hours = new Dictionary<string, int> { { "small", 125 }, { "medium", 100 }, { "large", 60 } };
    }

    public override int GetHours()
    {
        if (NumberOfProducts <= SMALL)
            return _hours["small"] * NumberOfProducts;
        if (NumberOfProducts <= MEDIUM)
            return (_hours["small"] * SMALL) + (_hours["medium"] * (NumberOfProducts - SMALL));
        return (_hours["small"] * SMALL) + (_hours["medium"] * (MEDIUM - SMALL))
            + (_hours["large"] * (NumberOfProducts - MEDIUM));
    }
}

public class Analysis : Service
{
    public Analysis(int numberOfProducts)
    {
        NumberOfProducts = numberOfProducts;
        _hours = new Dictionary<string, int> { { "small", 56 }, { "medium", 104 }, { "large", 200 } };
    }

    public override int GetHours()
    {
        if (NumberOfProducts <= SMALL)
            return _hours["small"];
        if (NumberOfProducts <= MEDIUM)
            return _hours["medium"];
        return _hours["large"];
    }
}

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        List<int> quantities = new List<int>();

        for (int i = 0; i < 100; i++)
        {
            quantities.Add(i);
        }
        comboBoxNumberOfProducts.DataSource = quantities;
    }

    private void comboBoxNumberOfProducts_SelectedIndexChanged(object sender, EventArgs e)
    {
        Service writing = new Writing((int) comboBoxNumberOfProducts.SelectedItem);
        Service analysis = new Analysis((int) comboBoxNumberOfProducts.SelectedItem);

        labelWriterHours.Text = writing.GetHours().ToString();
        labelAnalysisHours.Text = analysis.GetHours().ToString();
    }
}

Answer 1

You could combine the factory and the strategy pattern. Your factory would then create a concrete service and pass it a strategy to handle the different sizes (small, medium or large).

This would give you 8 classes: Service, Analysis, Writing, MediumStrategy, SmallStrategy, LargeStrategy and ServiceFactory + the interface for the strategies.

The ServiceFactory would then contain the code to decide which strategy would be used. Something like:

Analysis createAnalysis(int numberOfProducts) {
    SizeStrategy strategy;
    if (numberOfProducts <= SMALL) {
        strategy = new SmallStrategy();
    } else if (numberOfProducts <= MEDIUM) {
        strategy = new MediumStrategy();
    } else {
        strategy = new LargeStrategy();
    }
    return new Analysis(numberOfProducts, strategy);
}

In this case you only save very little code though. As an exercise this doesn't matter of course, but I don't think I would waste my time refactoring this in practice.

EDIT: On second thought, assuming that the rules are likely to change, it seems to me that a control table is probably more appropriate than the OOP patterns.

Answer 2

I'd move the logic for choosing which value to calculate into the Service base class and delegate the actual calculations to each subclass:

public abstract class Service
{
    private readonly int numberOfProducts;
    private readonly IDictionary<string, int> hours;
    protected const int SMALL = 2; 
    protected const int MEDIUM = 8;

    public Service(int numberOfProducts, IDictionary<string, int> hours)
    {
        this.numberOfProducts = numberOfProducts;
        this.hours = hours;
    }

    public int GetHours()
    {
        if(this.numberOfProducts <= SMALL)
            return this.CalculateSmallHours(this.hours["small"], this.numberOfProducts);
        else if(this.numberOfProducts <= MEDIUM)
            return this.CalculateMediumHours(this.hours["medium"], this.numberOfProducts);
        else
            return this.CalculateLargeHours(this.hours["large"], this.numberOfProducts);
    }

    protected abstract int CalculateSmallHours(int hours, int numberOfProducts);
    protected abstract int CalculateMediumHours(int hours, int numberOfProducts);
    protected abstract int CalculateLargeHours(int hours, int numberOfProducts);
}

Then if any calculation is particularly complicated you could extract it into a strategy object and use it just for that specific subclass.

EDIT: If you want to support an arbitrary number of calculations, you could create a class to manage the mappings between hours 'categories' and a calculation for each one. Then each subclass (or some factory) can provide the relevant calculations for each category:

public class HoursCalculationStrategyCollection
{
    private readonly Dictionary<string, int> hours;

    private readonly Dictionary<string, Func<int, int, int>> strategies;

    public HoursCalculationStrategyCollection(IDictionary<string, int> hours)
    {
        this.hours = hours;
        this.strategies = new Dictionary<string, Func<int, int, int>();
    }

    public void AddCalculationStrategy(string hours, Func<int, int, int> strategy)
    {
        this.strategies[hours] = strategy;
    }

    public int CalculateHours(int numberOfProducts)
    {
        string hoursKey = null;

        if(numberOfProducts <= SMALL)
            hoursKey = small;
        else if(...)
            ...

        Func<int, int, int> strategy = this.strategies[hoursKey];
        return strategy(this.hours[hoursKey], numberOfProducts);
    }
}

Answer 3

In your calculation there is a tight coupling between service type, service size and the number of products, it's very hard as it stands to separate them out into modular chunks to apply the strategy pattern.

If the calculation system is fixed, then it seems that the strategy pattern is not appropriate. If it isn't... Well, why not simplify the system?

For example, pull the base number of hours from the service size, and apply various discounts or increases depending on your other settings.

public class Service
{
    public IServiceSize serviceSize { internal get; set; }
    public IServiceBulkRate serviceBulkRate { internal get; set; }
    public IServiceType serviceType { internal get; set; }
    public int numberOfProducts { get; set; }

    /// <summary>
    /// Initializes a new instance of the <see cref="Service"/> class with default values
    /// </summary>
    public Service()
    {
        serviceSize = new SmallSize();
        serviceBulkRate = new FlatBulkRate();
        serviceType = new WritingService();
        numberOfProducts = 1;
    }

    public decimal CalculateHours()
    {
        decimal hours = serviceSize.GetBaseHours();
        hours = hours * serviceBulkRate.GetMultiplier(numberOfProducts);
        hours = hours * serviceType.GetMultiplier();

        return hours;
    }
}

public interface IServiceSize
{
    int GetBaseHours();
}

public class SmallSize : IServiceSize
{
    public int GetBaseHours()
    {
        return 125;
    }
}

public interface IServiceBulkRate
{
    decimal GetMultiplier(int numberOfProducts);
}

public class FlatBulkRate : IServiceBulkRate
{
    public decimal GetMultiplier(int numberOfProducts)
    {
        return numberOfProducts;
    }
}

public class StaggeredBulkRate : IServiceBulkRate
{
    public decimal GetMultiplier(int numberOfProducts)
    {
        if (numberOfProducts < 2)
            return numberOfProducts;
        else if (numberOfProducts >= 2 & numberOfProducts < 8)
            return numberOfProducts * 0.85m;
        else
            return numberOfProducts * 0.8m;
    }
}

public interface IServiceType
{
    decimal GetMultiplier();
}

public class WritingService : IServiceType
{
    public decimal GetMultiplier()
    {
        return 1.15m;
    }
}