strategy pattern in C#

Question

I've been going through Head First Design Patterns (just came in recently) and I was reading about the strategy pattern, and it occurred to me that it might be a great way to implement a common way of calculating taxes etc. on all of the particular objects I use at work, but I had a question about it.

Here's what I was thinking:

public interface ITax
{
    decimal ProvincialTaxRate { get; set; } // Yes, I'm Canadian :)
    decimal CalculateTax(decimal subtotal);
}

public SaskatchewanTax
{
    public decimal ProvincialTaxRate { get; set; }

    public SaskatchewanTax()
    {
        ProvincialTaxRate = new decimal(0.05f);
    }

    public decimal CalculateTax(subtotal)
    {
        return ProvincialTaxRate * subtotal + FederalTaxRate * subtotal;
    }
}

public OntarioTax
{
    public decimal ProvincialTaxRate { get; set; }

    public OntarioTax()
    {
        ProvincialTaxRate = new decimal(0.08f);
    }

    public decimal CalculateTax(decimal subtotal)
    {
        return ProvincialTaxRate * subtotal + FederalTaxRate * subtotal;
    }
}

You may have noticed that there is no declaration of FederalTaxRate and that's what I wanted to ask. Where should that go?

• Passing it in to the constructor for each concrete ITax seems redundant and would allow for incorrect behaviour (all tax calculators must share the exact same federal tax rate).
• Likewise, creating a member of ITax would allow them to be inconsistent as well.

Should all tax calculators inherit from some other class where it's defined statically as well as ITax?

public class TaxCalculator
{
    public static decimal FederalTaxRate = new decimal(0.05f);
}

Answer 1

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

Answer 2

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.