Layered example page 40, Price class

StevenGibson · November 2nd, 2010, 01:03 PM

At the top of the Price.cs class on page 40 there is the line:

Code:

private IDiscountStrategy _discountStrategy = new NullDiscountStrategy();

Which defaults the Price to use the null discount strategy. My question is why don't we use this instead:

Code:

private IDiscountStrategy _discountStrategy;
...

public Price( decimal rrp, decimal sellingPrice )
{
... 
  _discountStrategy = DiscountFactory.GetDiscountStrategyFor( CustomerType.Standard );
}

In this way the Price class is not directly dependent on the NullDiscountStrategy class, although is now has dependencies on the factory method and the CustomerType enumeration.
I would like to know whether this would be considered an improvement to the code or does it not really make any difference? I seem to get bogged down in little things like this!

Many thanks

Steven Gibson

elbandit · November 2nd, 2010, 01:25 PM

Hi Steven,

Thanks for buying the book. I think if you were going to use the Factory within the Price class you would probably need to inject it. So maybe for a clearer view to the client of what is going on is to modify the code to:

Code:

public Price(decimal RRP, decimal SellingPrice, IDiscountFactory discountFactory)
{
 _rrp = RRP;
 _sellingPrice = SellingPrice;
 _discountFactory = discountFactory;
} 
public decimal SellingPriceTo(CustomerType customerType)
{
    get { 
              return discountFactory.GetDiscountStrategyFor(customerType)
                                   .ApplyExtraDiscountsTo(_sellingPrice);   
           }
}

What do you think?
Cheers
Scott

StevenGibson · November 3rd, 2010, 01:33 PM

This is an interesting idea, however this would surely break the Discount and Savings properties as they are reliant on SellingPrice.
To make this work would require the CustomerType to be passed in to both these properties (making them methods), also in the class ProductMapperExtensionMethods (page 46)
when the ProductViewModel is created, the CustomerType will now be required here.

Therefore it would seem that to overcome these problems we will effectively be replacing the SetDiscountStrategyTo(IDiscountStrategy) method
with a method SetCustomerTypeForDiscount(CustomerType) - to allow the Price to remember which discount to apply.

This would allow us to remove the CustomerType parameter from the 3 properties (SellingPrice, Discount and Savings).

However this still takes us back to the problem of the 'default' case - i.e. what
should the instance variable that holds the CustomerType initially be set to?

The book text already requires clients to use the CustomerType when using the ProductService classes GetAllProductsFor method.
This then converts the CustomerType to a DiscountStrategy and sets the strategy using the Apply extension method (pages 43-44).
As all this happens within the domain (Model project) I can't see how your change would give a 'clearer view to the client'.

I have assumed that your method SellingPriceTo replaces the property SellingPrice and that the variable _discountStrategy is no longer required.

elbandit · November 3rd, 2010, 04:41 PM

Ooops! Yeah good point, I wrote the snippet of code from memory and had forgottern about the other dependant properties. What would be more clear is something like...

Code:

 
public Price(decimal RRP, decimal SellingPrice, IDiscountStrategy defaultStrategy)
{
_rrp = RRP;
_sellingPrice = SellingPrice;
_discountStrategy = defaultStrategy;
}
public void SetDiscountStrategyTo(IDiscountStrategy DiscountStrategy)
{
_discountStrategy = DiscountStrategy; 
}
public decimal SellingPrice
{
get { return _discountStrategy.ApplyExtraDiscountsTo(_sellingPrice); }
}

Considering that an instance of IDiscountStrategy is required for the Price class to work.