strategy pattern in C#

I think this is a common case of pattern abuse.

If you check your two "strategies", they do EXACTLY the same thing. The only thing that changes is the ProvincialTaxRate.

I'd keep things DRY and don't overuse this pattern (or any other), here you gain a little bit of flexibility, but then you also have 2 classes that don't pull their weights, and probably You Ain't Gonna Need that flexibility.

This is common when you learn a new technology or insight, you want to apply it everywhere (it happens to every one of us), even if doing it harms the code readability and maintainability.

My opinion: keep it simple

Regards

EDIT (In response to the author comment on my answer)

I did not try to make fun of you, or anyone. This is a common mistake, I did it MANY times, and learned it the hard way, not only with patterns but also with fancy frameworks, servers, new buzzword technologies, you name it.

The authors of the book themselves warn the readers not to overuse patterns, and the upvotes in this answer clearly indicate something too.

But if for some reason you still want to implement the pattern, here's my humble opinion:

  • Make a superclass for both strategies, this superclass would be abstract and should contain the shared rate value of their child strategies (FederalTaxRate)

  • Inherit and implement the abstract method "Calculate" in each subclass (here you'll see that both methods are the same, but let's continue)

  • Try to make each concrete strategy immutable, always favor immutability as Joshua Bloch says. For that, remove the setter of ProvincialTaxRate and specify the value on it's constructor or directly in its declaration.

  • Lastly, I'd create some static factory methods in the StrategySuperclass so that you decouple your clients from the implementations or concrete strategies (that can very well be protected classes now)

Edit II: Here's a pastie with some (pseudo) code to make the solution a bit more clear

http://pastie.org/441068

Hope it helps

Regards


In my opinion, you have the right solution - create a base class that contains the Canadian federal fax rate from which all of your derived classes can inherit. Statically defining it is a perfectly fine idea. You could also make the FederalTaxRate define only an accessor function for the tax rate, so that you could presumably define it at runtime from a file or something else.

I don't think that this is uniquely the best solution, but it will work perfectly well. Design patterns shouldn't get in the way of your common sense, and I think that common sense will solve this problem just fine.


You might want to start with this code, and move on from there:

public interface ITax
{
    decimal CalculateTax(decimal subtotal);
}

public class SaskatchewanTax : ITax
{
    private readonly decimal provincialTaxRate;
    private readonly decimal federalTaxRate;

    public SaskatchewanTax(decimal federalTaxRate)
    {
        provincialTaxRate = 0.05m;
        this.federalTaxRate = federalTaxRate;
    }

    public decimal CalculateTax(decimal subtotal)
    {
        return provincialTaxRate * subtotal + federalTaxRate * subtotal;
    }
}

public class OntarioTax : ITax
{
    private readonly decimal provincialTaxRate;
    private readonly decimal federalTaxRate;

    public OntarioTax(decimal federalTaxRate)
    {
        provincialTaxRate = 0.08m;
        this.federalTaxRate = federalTaxRate;
    }

    public decimal CalculateTax(decimal subtotal)
    {
        return provincialTaxRate * subtotal + federalTaxRate * subtotal;
    }
}

At this point there may not be much point to have two different strategy objects representing the tax calculation, but with a more realistic implementation (I am assuming tax calculation is more complicated and varies more by province), it might make sense.

However, you should consider applying the "simplest thing that could possibly work" principle, and only use the strategy pattern when you feel that it is needed.

Tags:

C#

Design Patterns

Strategy Pattern

Recent Posts