Introduction
This is part 2 of an article that started here. You’ll probably want to read it first or you wont' know what I'm talking about . I’m going to continue with the same code base while re factoring and demonstrating some more coding principles.
- Part 1
- Part 2
- Part 3
More Re Factoring, Patterns and Interfaces Hooray
Below is a code snippet from the order class we made previously. Do you see anything odd about it?
public class Order
{
ITaxFactory _taxFactory;
public Order(Customer c)
: this(new DateBasedTaxFactory(c, new CustomerBasedTaxFactory(c)))
{
}
public Order(ITaxFactory taxFactory)
{
_taxFactory = taxFactory;
}
public List<OrderItem> _orderItems = new List<OrderItem>();
public decimal CalculateTotal(Customer customer)
{
decimal total = _orderItems.Sum((item) =>
{
return item.Cost * item.Quantity;
});
total = total + _taxFactory.GetTaxObject().CalculateTax(customer, total);
return total;
}
public interface ITax
{
decimal CalculateTax(Customer customer, decimal total);
}
public class TXTax:ITax
{
public decimal CalculateTax(Customer customer, decimal total)
{
return total * .08m;
}
}
}
Perhaps you notice something off in the CalculateTotal method. We have 2 separate references to a Customer object, one in the constructor and one as a parameter in CalculateTotal. Besides it being awkward, it is possible that Order could be working with 2 different customer instances which would cause some unstable behavior. And look at the code in the TXTax class. See something off? The CalculateTax method is requiring a customer parameter but it isn't using it. I might make a claim that this is violating the interface segregation principle. This basically says that classes shouldn't have to implement a bunch of stuff they don’t need from an interface. If they aren't going to use it then it shouldn't be there. Usually this applies to interface methods, but perhaps it applies to parameters too. What do you think? Anyways, if you think about it logically, it’s perfectly reasonable to have a Tax object that doesn't need customer, such as our NoTax class. So this should not be required for every ITax implementation.
So, to explain, in the first version of the code we needed the customer in order to implement the tax logic directly in this method for one specific case (based on state). Well we don't need this anymore because the tax object handles the logic itself. So let’s make some dramatic code changes to accommodate this. This may seem like a lot but remember we are still designing and coding for learning sake, not changing production code
public class Order
{
ITaxFactory _taxFactory;
public Order(Customer c)
: this( new DateBasedTaxFactory(c, new CustomerBasedTaxFactory(c)))
{
}
public Order(ITaxFactory taxFactory)
{
_taxFactory = taxFactory;
}
public List<OrderItem> _orderItems = new List<OrderItem>();
public decimal CalculateTotal()
{
decimal total = _orderItems.Sum((item) =>
{
return item.Cost * item.Quantity;
});
total = total + _taxFactory.GetTaxObject().CalculateTax( total);
return total;
}
public interface ITax
{
decimal CalculateTax( decimal total);
}
public class TXTax:ITax
{
public decimal CalculateTax( decimal total)
{
return total * .08m;
}
}
}
Ok, done, let's move on to something else. Let me add some code that will very simply demonstrate actually using the Order object. Usually in e-commerce we have the concept of a Basket so we'll use this and stick with what we've learned in Article I concerning coding to abstractions and factories and such
public interface IOrderFactory
{
Order GetOrder(int orderId);
}
public class OrderFactory : IOrderFactory
{
public Order GetOrder(int orderId)
{
Order order = null;
return order;
}
}
public class Basket
{
Order_order;
public Basket(int sessionId,IOrderFactory orderFactory)
{
int orderId = 3333;
_order = orderFactory.GetOrder(orderId);
}
public Basket(int sessionId):this(sessionId,new OrderFactory())
{
}
}
Super, now we have everything we need for the UI to get a basket and it's orders.
What if I were to say that we should create an interface called IOrder that looked like this:
public interface IOrder
{
decimal CalculateTotal();
int NumberOfItems();
}
and we made Order Implement it. You might think to yourself, what the heck is the point of that. We already have a concrete class Order, there isn't going to be anything else except for THE Order, why would we have an interface for it. Well, let's imagine that we finished coding our all our classes, tested it and moved it to production. But then (suprise!) not all the business requirements were provided, we forgot something. We now need to add shipping calculations to the CalculateTotal order. Perhaps something like this:
public decimal CalculateTotal()
{
decimal shippingCost = 1.3m * _orderItems.Count;
decimal total = _orderItems.Sum((item) =>
{
return item.Cost * item.Quantity;
});
total = shippingCost + total + _taxFactory.GetTaxObject().CalculateTax(total);
return total;
}
Great, now we have to go into the order class and change it, but we aren't suppose to do that! Open/closed principle. What can be done? We can use a pattern that is designed to extent the behavior of a class without having to actually change it, the Decorator. Simply put, decorator objects take and maintain references to other objects that implement the same abstraction, in this case the IOrder interface. Then they use each other to add functionality on top of each other. Confusing? Lets see how it works. I don't want to change the order class but we should have had IOrder in the first place. So let's pretend it had always been there from the start.
public class Order:IOrder
{
}
Now we need new functionality and what do you always do when new functionality is needed? Create a class of course. So let's create a class that handles shipping like so:
public class ShippingOrder : IOrder
{
IOrder _order;
public ShippingOrder(IOrder order)
{
_order = order;
}
public decimal CalculateTotal()
{
decimal shippingCost = 1.3m * _order.NumberOfItems();
return shippingCost + _order.CalculateTotal();
}
public int NumberOfItems()
{
return _order.NumberOfItems();
}
}
So what is the point of this? Well, you have your default Order class as it currently exists but then you can pass it into a ShippingOrder object which will "wrap" it. It handles calculating the shipping charges itself and then just adds that to the value from CalculateTotal of the normal Order object. Then that factory can return a ShippingOrder instead of a plain old Order and the consumer will never know the difference. So lets change it to what it should have been in the first place.
public interface IOrderFactory
{
IOrder GetOrder(int orderId);
}
public class OrderFactory : IOrderFactory
{
public IOrder GetOrder(int orderId)
{
Order order = null;
return new ShippingOrder(order);
}
}
public class Basket
{
IOrder _order;
public Basket(int sessionId,IOrderFactory orderFactory)
{
int orderId = 3333;
orderFactory.GetOrder(orderId);
}
public Basket(int sessionId):this(sessionId,new OrderFactory())
{
}
}
So what do we have here? The Basket object is using the OrderFactory to get a reference to an IOrder. It has no idea if that variable is holding a reference to Order or ShippingOrder, and it shouldn't. This is exactly what we want. The OrderFactory is the only thing aware of what is going on. It just gets a Order object from the database like normal, but then puts it into a ShippingOrder object so that it can add the shipping functionality and the Order object never had to change.
So now that everything is properly in place, let's look at it one more time. The higher up managers now want us to accept coupons and discount codes. Excellent, this will be an easy change because we've set ourselves up for success.
public class DiscountOrder : IOrder
{
IOrder _order;
public DiscountOrder(IOrder order)
{
_order = order;
}
public decimal CalculateTotal()
{
return _order.CalculateTotal() - 4;
}
public int NumberOfItems()
{
return _order.NumberOfItems();
}
}
so we just have to create a class DiscountOrder and nothing has to change except the factory class:
public class OrderFactory : IOrderFactory
{
public IOrder GetOrder(int orderId)
{
Order order = null;
return new ShippingOrder(new DiscountOrder(order));
}
}
Yes I know I left a lot of things out. The Basket object isn't even using the CalculateTotal method and if it did call it, there would be a runtime exception because the tax factory is null, but you get the idea.
Ok, thats it for now. Hope that made sense and added value for somebody.