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).

Well written code is easy to test as well. Split your functionality into granular components, test at that level. Also do the test of the whole thing in concert. Testing code need not look 100% elegant. Testers work around implementations, not the other way around. Sometimes there is a compromise, but trust me - in a real world $-making software market (most of the market), testing is given second priority (NASA does things differently). Keep it simple until there is a need for complexity. — Hamish Grubijan, Dec 18 '09 at 20:00
This kind of question can only really be answered by a multi-day whiteboard discussion. — Jeff Sternal, Dec 18 '09 at 20:09
@Jeff Sternal - Agreed - giving some ideas that would get throw on the whiteboard :) — CSharpAtl, Dec 18 '09 at 20:12

score 1 · Answer 1 · answered Dec 18 '09 at 19:52

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.

I don't have a full grasp on Ioc, prob. why I am not using it to be honest! — mrblah, Dec 18 '09 at 20:30
In a nutshell, what IoC would do for you here is to provide you with a container or context class of some kind, that can then provide you with an instance of your order manager class that already has hese dependencies set up (injected) for you. You only have to worry about the fact that you want an OrderManager, the IoC container does the rest. Then in the test scenario, you can just create a new OrderManager (rather than using the container to create it), and then inject mock objects instead in order to be able to test the OrderManager in isolation (i.e. unit test it). — David M, Dec 19 '09 at 08:59

CSharpAtl · Answer 2 · 2009-12-18T20:07:38.607

1

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.

edited Dec 18 '09 at 20:07

answered Dec 18 '09 at 19:56

CSharpAtl

7,374
8
39
53

I guess I don't fully understand IOC, hence shying away from it. – mrblah Dec 18 '09 at 20:30
Do you understand Inversion Of Control? Classes that depend on other objects, they get passed in instead of creating them inside the class. This makes it very easy to change the dependencies, like the data layer dependency, for testing. – CSharpAtl Dec 18 '09 at 20:42
@csharpatl yes I understand that, that is what I did with my dbfactory, using constructor injection. – mrblah Dec 18 '09 at 20:45
IOC takes care of the injecting for you, in most cases, you at least do not have to deal with the concrete classes directly in the application code. The IOC does the injecting with configuration or possibly Domain Specific Language. – CSharpAtl Dec 18 '09 at 21:24

Chris McCall · Answer 3 · 2009-12-18T21:02:16.983

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.

score 0 · Answer 4 · answered Dec 18 '09 at 20:11

0

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.

answered Dec 18 '09 at 20:11

Burt

7,680
18
71
127

Help designing a order manager class

4 Answers4

Linked