Help designing a order manager class

Question

So I have an order manager class that looks like:

public class OrderManager
{
     private IDBFactory _dbFactory;
     private Order _order;

     public OrderManager(IDBFactory dbFactory)
     {
         _dbFactory = dbFactory;
     }


     public void Calculate()
     {

          _order.SubTotal
          _order.ShippingTotal
          _order.TaxTotal

          _order.GrandTotal


     }
}

Now, the point here is to have a flexible/testible design. I am very concerned about being able to write solid unit tests around this Calculate method.

Considerations:

     1.  Shipping has to be abstracted out, be loose coupled since the implementation of shipping could vary depending on USPS, UPS, fedex etc.  (they have their own API's).
     2. same goes with calculating tax

Should I just create a Tax and Shipping Manager class, and have a tax/shipping factory in the constructor? (exactly how I have designed my OrderManager) class?

(the only thing that I can think of, in terms of what I am "missing", is IoC, but I don't mind that and don't need that extra level of abstraction in my view).

Answer 1

I would take the Calculate method out into a class. Depending on your circumstances OrderCalculator might need to be aware of VAT, Currency, Discounts, ...

Just a thought.

Answer 2

Well, you are already moving towards dependency injection in your approach, so why not go the whole hog and use some sort of IoC container to handle this for you?

Yes, if you want it abstrated out, then create a separate class for it. If you want to truly unit test what is left, abstract out an interface and use mock testing. The problem is, the more you abstract out like this, the more plumbing together there is to do and the more you will find yourself wishing you were using an IoC framework of some kind.

You are suggesting constructor injection, which is a common approach. You also come across property injection (parameterless constructor, set properties instead). And there are also frameworks that ask you to implement an initialization interface of some kind that allows the IoC framework to do the initialization for you in a method call. Use whatever you feel most comfortable with.

Answer 3

Just a thought:

Your Calculate() method taking no parameters, returning nothing and acting on private fields is not how I would do it. I would write it as a static method that takes in some numbers, an IShippingProvider and an ITaxJurisdiction and returns a dollar total. That way you have an opportunity to cache the expensive calls to UPS and your tax tables using memoization.

Could be that I'm prejudiced against public methods that work like that. They have burned me in the past trying to bind to controls, use code generators, etc.

EDIT: as for dependency injection/IOC, I don't see the need. This is what interfaces were made for. You're not going to be loading up a whole array of wacky classes, just some implementations of the same weight/zipcode combo.

That's what I would say if I were your boss.

Answer 4

I do think an IOC would help with the plumbing of instantiating the correct concrete classes but you still need to get your design the way you want it. I do think you need to abstract away the shipping with an interface that you can implement with a class for each of your shippers (USPS, UPS, FEDEx, etc) and could use a Factory class (ShippingManager) to pass the correct one out or depend on the IOC to do that for you.

public interface IShipper
{
//whatever goes into calculating shipping.....
 decimal CalculateShippingCost(GeoData geo, decimal packageWeight);
}

You could also just inject an IShipper and ITaxer concrete classes into your OrderManager and you calculate method just calls into those classes....and can use an IOC nicely to handle that.