Is it bad to use interface and then check for implementation type?

Question

Consider the following scenario:

I want to design a discount calculator which gets me the discount that can be applied to an order. There are two types of order: Online and In-Store. Based on type of the order and total amount of the order, a discount calculator calculates the discount.

I programmed to demonstrate the scenario in C# but the problem is language independent. In the below code, DiscountCalculator class calculates the discount by examining the actual type of input parameter.

I feel checking the actual type of IOrder argument in GetDiscount method is code smell; because I hid the implementation details behind the interface IOrder, then I somehow bring out of the box what was meant to be hidden.

    interface IOrder
    {
        int GetTotalPrice();
    }

    class InStoreOrder : IOrder
    {
        public int GetTotalPrice() { // returns the price of order }
    }

    class OnlineOrder : IOrder
    {
        public int GetTotalPrice() { // returns the price of order }
    }

    class DiscountCalculator
    {
        public int GetDiscount(IOrder order)
        {
            Type orderType = order.GetType();
            if (orderType == typeof(OnlineOrder))
            {
                if (order.GetTotalPrice() < 100)
                    return 2;
                else
                    return 5;
            }
            if (orderType == typeof(InStoreOrder))
            {
                if (order.GetTotalPrice() < 100)
                    return 3;
                else
                    return 6;
            }
            else
                throw new Exception("Unknown order type:" + orderType.Name);
        }
    }

Any idea?

Update:

I really appreciate people collaborating on this. All of the solutions were not only enlightening but also brought an elegant way on the table.

BTW, since the time all of the answers assured me that the solution is not good, I was thinking to myself that Abstract Factory may be a good alternative. Why? Because we are dealing with a family of related objects: Order and DiscountCalculator.

Something like this:

Factory f = new FactoryRepo ("Online");
IOrder order = f.CreateItem();
IDiscountCalculator discounter = f.CreateDiscountCalculator();
....

This way, I think for future changes, as @Dhruv Rai Puri pointed out, Decorator pattern may be easily applied.

Any Idea?

Answer 1

The solution of Strategy was already proposed https://stackoverflow.com/a/32798708/1168342, but this answer has some advantages.

Discounts and Orders are common domain problems. This wheel has been reinvented a few times. I'll cite a solution from chapter 26 of Craig Larman's "Applying UML and Patterns" book:

In this solution, a Sale is like your Order and ISalePricingStrategy is like your DiscountCalculator.

ISalePricingStrategy is an application of the Strategy pattern (the name is in the interface), and Strategies are always attached to a context object. In this case, it's the Sale (or in yours, IOrder).

Mapping to your problem

Here's the UML of how I see your problem fitting into Larman's suggested use of pricing strategies:

Both your cases are composite instances of the AbsoluteDiscountOverThresholdPricingStrategy if I understand properly. Let's take the code from your conditional for OnlineOrders:

if (order.GetTotalPrice() < 100)
  return 2;
else
  return 5;

This is like adding to your order two instances of

onlineOrder.addPricingStrategy(new AbsoluteDiscountOverThresholdPricingStrategy(2,0));  // orders under 100
onlineOrder.addPricingStrategy(new AbsoluteDiscountOverThresholdPricingStrategy(5,100));  // orders >= 100

So, Larman goes a step further and explains that you can combine such strategies using the Composite pattern. I'll apply it to your problem (the hint is in the add... method above):

The two classes I put in pink are optional. If you always give the best strategy to the customer (as in the pseudocode of the note I attached to GetTotalPrice) you don't need them. Larman explains you can go a step further and say that if more than one strategy applies, the calculation is either in favor of the store or the customer. Again, it's a question of instantiating the class and attaching it. The code to do this could be run from a "Configuration" menu command in your software.

The code to use this would look something like:

IOrder onlineOrder = new OnlineOrder();  //...
...
CompositePricingStrategy comp = new CompositePricingStrategy();
comp.add(new AbsoluteDiscountOverThresholdPricingStrategy(2,0));  // orders under 100
comp.add(new AbsoluteDiscountOverThresholdPricingStrategy(5,100));  // orders >= 100
onlineOrder.setPricingStrategy(comp);
// repeat as above for instoreOrders ...

There are more flexible ways again to do this, using factories. See Larman's book for very cool examples in Java/.NET.

Advantages

Since this answer is similar to another one, I want to explain some advantages of this method, even though it's more complicated. If you use GetTotal() in the discount logic, it has some advantages over GetDiscount():

• GetTotal() handles (encapsulates) all the logic to calculate the total.
• If multiple strategies apply (e.g., online orders get 5% discount, orders over $200 get 10% discount) you may want to code how this is handled. Larman gives an example using Composite pattern where again GetTotal() works impeccably without the client code having to do anything special.
• You can extend other types of Strategies and instantiate them as needed. For example, for any order over $500 you can make the discount 50. It's a question of instantiating the new class in the code (not coding the logic).