Is there a good way to avoid unused method parameter in some of the subclasses while applying strategy pattern?

Question

I have the following scenario where I have different kinds of sales algorithms to calculate the sales price. FixedSaleStrategy does not need basePrice parameter while all the other strategy implementations need it. Is there a good way to avoid this redundant parameter?

public abstract class SalesStrategy
{
    public abstract double GetPrice(double basePrice, double saleAmount);
}
public class AmountOffSale : SalesStrategy
{
    public override double GetPrice(double basePrice, double salesAmount)
    {
        return basePrice - salesAmount;
    }
}
public class FixedPriceSale : SalesStrategy
{
    public override double GetPrice(double basePrice, double salesAmount)
    {
        return salesAmount;
    }
}

First things first... change your parameters to `decimal` instead of `double`. **Never** use binary floating point numbers for dealing with currency because they cannot exactly represent currency values, even simple values like `1.1`, and rounding errors are very important when handing peoples money. See: http://en.wikipedia.org/wiki/Floating_point#Representable_numbers.2C_conversion_and_rounding — Greg Beech, Jul 30 '10 at 23:44

score 6 · Accepted Answer · edited Nov 17 '17 at 11:22

6

At the core of the strategy pattern is the idea that the calling code does not know the implementation being called.

If you were to change the parameters used per implementation you would find that you are not getting the full benefit of this pattern: a caller would need to know which implementation was going to be used and how to call it.

What I tend to do is pass a class that contains a super-set of information (something like PricingInfo), which is always populated the same way (Ideally centralized in the code) and the only difference is the implementations of the strategy.

One of the benefits is that I can add a property to my PricingInfo class that was not relevant in the past (like say, systemDiscount), and the impact to the system as a whole is not too large.

edited Nov 17 '17 at 11:22

Robbie Dee

1,939
16
43

answered Jul 31 '10 at 01:21

brian chandley

1,276
1
10
20

Good comment on PricingInfo. Wrapping the underlying entity info (eg from 'Product' or 'OrderItem') into a PricingInfo, also allows more kinds of entities (shipping charges?) to be brought into the system later.. – Thomas W May 04 '13 at 12:16
And a PricingInfo or similar 'answer' interface, also helps focus your API design better; onto 'what does the algorithm need to know', rather than 'how can I hack this up from the fields in my current entity'. I find this helps clean & clarify my code, very effectively. – Thomas W May 04 '13 at 12:17

score 5 · Answer 2 · answered Jul 30 '10 at 23:43

5

No. It's not a redundant parameter; the code that utilizes a SalesStrategy should not know which concrete class it is using, so the method signature must be identical in all derived classes.

answered Jul 30 '10 at 23:43

Jamie Ide

48,427
16
81
117

robyaw · Answer 3 · 2010-07-30T23:48:53.023

If you're using c# 4.0, you could reverse the parameters and make basePrice optional like so:

public abstract class SalesStrategy
{
    public abstract double GetPrice(double saleAmount, double basePrice = 0d);
}

public class AmountOffSale : SalesStrategy
{
    public override double GetPrice(double salesAmount, double basePrice)
    {
        return basePrice - salesAmount;
    }
}

public class FixedPriceSale : SalesStrategy
{
    public override double GetPrice(double salesAmount, double basePrice = 0d)
    {
        return salesAmount;
    }
}

Meaning the following can be done...

FixedPriceSale fixedPrice = new FixedPriceSale();
...
fixedPrice.GetPrice(salesAmount);

Note that AmountOffSale's basePrice parameter is not optional, meaning that the following will not compile:

AmountOffSale amountOffSale = new AmountOffSale();
...
// No overload for method 'GetPrice' takes 1 arguments
amountOffSale.GetPrice(salesAmount);

I just typed in this same response; then noticed your reply as I was verifying the code :-). — Jess, Aug 05 '10 at 15:12

score 0 · Answer 4 · answered Jul 30 '10 at 23:39

0

Not a good one, in my opinion. I would keep it as is. There are various tricks you could use like params (have a single params double[] priceData) or IDynamicObject. But the cleanest is just to have some strategies ignore the extra parameter.

answered Jul 30 '10 at 23:39

Matthew Flaschen

278,309
50
514
539

score 0 · Answer 5 · answered Jul 31 '10 at 01:06

A good way to remove irrelevant parameters from an interface is by passing those parameters in constructors from subclasses . So, an alternative for your design would be:

public interface SalesStrategy
    {
        double CalculatePrice(double basePrice);
    }

public class FixedPriceSale : SalesStrategy
    {
        public double CalculatePrice(double basePrice)
        {
            return basePrice;
        }
    }

public class AmountOffSale : SalesStrategy
    {
        public double SalesAmount { get; set; }

        public AmountOffSale(double salesAmount)
        {
            this.SalesAmount = salesAmount;
        }

        public double CalculatePrice(double basePrice)
        {
            return basePrice - SalesAmount;
        }
    }

In that construction you do not pollute your interface with specific data from subclasses.

I think the problem with this approach is that at some point, something needs to create a `SalesStrategy` sub-class and **that** thing will need to know the basePrice. So effectively this moves the extra parameter from the method call to the factory method. There still would be a place where this parameter is not being used. — Igor Zevaka, Jul 31 '10 at 01:31

score 0 · Answer 6 · answered Jul 31 '10 at 01:16

0

Another alternative is to use a parameters object or Dictionary<string, object>. This way you can consolidate the number of parameters on each method and leave room for additional parameters should there be a change in requirements in the future.

The one drawback is that a Dictionary<string, object> can make tracking the parameters harder in your code, where as a parameters object will simply have all the properties that you can view in your code.

answered Jul 31 '10 at 01:16

David Robbins

9,996
7
51
82

Dictionaries are too unstructured to be good design in a case with simple & well-defined properties, such as pricing -- and not very efficient either. They do have applications such as configuration or optioning for complex submodules, or very open extensibility -- but are best to be used at initialization/ configuration time, rather than every calculation. – Thomas W May 04 '13 at 12:22
I agree - it's about the trade off between making method signatures complex, efficient code, readability, etc. – David Robbins May 05 '13 at 17:36

Is there a good way to avoid unused method parameter in some of the subclasses while applying strategy pattern?

6 Answers6