Reputation: 5449
Hi I was tasked with refactoring a portion of code in order for it to corespond with the open/closed principle.I managed to do that applying the strategy pattern in two of the methods but in one I do not know how I should proceed.
public ProductPriceCalculator(Product product)
{
this.product = product;
buyerStrategy = new BuyerStrategy();
discountStrategy = new DiscountStrategy();
}
public Price CalculatePrice()
{
price = new Price();
decimal productPrice = product.BasePrice +
(product.BasePrice * product.Addition);
decimal TVA = CalculateTVA();
price.ProductPrice = productPrice*TVA;
decimal discount = CalculateDiscount(price.ProductPrice);
price.Discount = price.ProductPrice * discount;
price.ProductPriceWithDiscount = price.ProductPrice - price.Discount;
price.TransportPrice = product.Transport.GetTransportPrice();
price.TotalPrice = price.ProductPriceWithDiscount + price.TransportPrice;
return price;
}
In this case this method initialezes the object by using the methods in the class.As it stands this method is not closed for modification because if at some point I have to add another property to Price I will have to come back here to initialize it.
How can I structure this code right?
Upvotes: 2
Views: 308
Reputation: 540
One possible solution is following:
public class ProductPriceCalculator
{
private readonly Product _product;
private readonly BuyerStrategy _buyerStrategy;
private readonly DiscountStrategy _discountStrategy;
private readonly Price _price;
public ProductPriceCalculator(Product product,BuyerStrategy buyerStrategy,DiscountStrategy discountStrategy,Price price)
{
_product = product;
_buyerStrategy = buyerStrategy;
_discountStrategy = discountStrategy;
_price = price;
}
public Price CalculatePrice()
{
decimal productPrice = _product.BasePrice + (_product.BasePrice * _product.Addition);
decimal TVA = CalculateTVA();
decimal discount = CalculateDiscount(productPrice * TVA);
decimal transportPrice = _product.Transport.GetTransportPrice();
return _price.CalculatePrice(productPrice*TVA,discount,transportPrice);
}
...
}
public class Price
{
...
public virtual Price CalculatePrice(decimal productPrice, decimal discount, decimal transportPrice)
{
Price price = new Price();
price.ProductPrice = productPrice;
price.Discount = ProductPrice * discount;
price.ProductPriceWithDiscount = ProductPrice - Discount;
price.TransportPrice = transportPrice;
price.TotalPrice = ProductPriceWithDiscount + TransportPrice;
return price;
}
...
}
It is good to put dependencies (such as buyerStrategy
,discountStrategy
,price
) in constructor and than fill them via IoC container or something else rather then creating them in constructor themselves.
After introducing _price
field you can delegate filling price's properties to Price
class itself. Method Price.CalculatePrice
can be called as Fabric method for Price
class.
Upvotes: 5
Reputation: 733
Bit difficult to say without knowing a little more about the surrounding code - but I would have thought that maybe the Decorator Pattern could solve this?! So in your instance perhaps something along these lines :
public abstract class BaseProduct
{
public decimal BasePrice { get; set; }
public decimal Addition { get; set; }
public Transport Transport { get; set; }
public abstract void CalculatePrice(decimal discount);
}
public class Product : BaseProduct
{
public BasePrice Price { get; set; }
public Product(BasePrice price)
{
this.Price = price;
}
public override void CalculatePrice(decimal discount)
{
this.Price.CalculatePrice(this.BasePrice, this.Addition, discount);
}
}
public abstract class BasePrice
{
public abstract void CalculatePrice(decimal basePrice, decimal additional, decimal discount);
}
public class Price : BasePrice
{
public decimal ProductPrice { get; set; }
public decimal Discount { get; set; }
public decimal ProductPriceWithDiscount { get; set; }
public decimal TransportPrice { get; set; }
public decimal TotalPrice { get; set; }
public override void CalculatePrice(decimal basePrice, decimal additional, decimal discount)
{
this.ProductPrice = basePrice + (basePrice * additional);
this.Discount = this.ProductPrice * discount;
this.ProductPriceWithDiscount = this.ProductPrice - this.Discount;
this.TotalPrice = this.ProductPriceWithDiscount + this.TransportPrice;
}
}
Using this method, you can create new concrete implementations of BasePrice as you add new properties and still support the closed principle as you don't need to alter the Calculate logic in the original class, just add new concrete implementations as it changes. Just to clarify - the Product is the Component and the Price is the Decorator.
Hopefully I got that right - open to any feedback! And equally, hope it helps!! :)
Upvotes: 1