Question

so I am trying to write a discounts method that will apply discount(s) on a product. The current vanilla code goes like so:

void ApplyDiscount(List<DiscountRule> discountRules, Product objProduct)
    {
        foreach (var discountRule in discountRules)
        {
            // this is a very simple way of deciding on the available discounts
            if (discountRule.Type==DiscountType.Percent)
            {
                // Process for percentage discount
            }

            if (discountRule.Type==DiscountType.Free)
            {
                // Process for flat discount
            }

            // and so on , there are like 5 more types, 
            // not mentioned here for the case of brevity.   

        }
    }

What this method does is take a list of discount rules and apply on the product. The discount rules are fetched by executing a SP @ the server and that returns the available discounts for that product.

The code review for this resulted in the following comment:

Please use an interface based approach and try to get rid of the IFs!

I can get rid of the IFs but they will be replaced by SWITCH. How do I go about using an interface?

May be this question is not constructive, but I would want to know if some OOPs gurus here can guide me in writing this better.

Regards.

Was it helpful?

Solution

An interface / virtual-dispatch approach might look something like this:

// First we loosely define how a "discount" can be used.
// This could also be an abstract class, if common base-class
// functionality is desired.
public interface IDiscount
{
    // This is called to apply this discount to a particular product
    void ApplyDiscount(Product product);
}

// Here's one implementation that applies a percentage discount
public class PercentDiscount : IDiscount
{
    private decimal m_percent;

    public PercentDiscount(decimal percent) {
        m_percent = percent;
    }

    #region IDiscount implementation

    public void ApplyDiscount(Product product) {
        product.Price -= product.Price * m_discount;
    }

    #endregion
}

// Here's another implementation that makes a product free
public class FreeDiscount : IDiscount
{
    public FreeDiscount() {
    }

    #region IDiscount implementation

    public void ApplyDiscount(Product product) {
        product.Price = 0;
    }

    #endregion
}


public class SomeClass {

    // Now applying the discounts becomes much simpler! Note that this function
    // takes a collection of IDiscounts, and applies them in a consistent way,
    // by just calling IDiscount.ApplyDiscount()
    void ApplyDiscounts(IEnumerable<IDiscount> discounts, Product product) {
        foreach (var discount in discounts) {
            discount.ApplyDiscount(product);
        }
    }

}

Note that I also changed ApplyDiscounts to take an IEnumerable<T> instead of List<T>. This allows any arbitrary collection type to be passed, and also doesn't allow the function to inadvertently modify the collection.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top