1

This is something hat has been rolling in my mind for quite some time now, but I still haven't found a good approach to it. So here's the thing. We have a server application that has quite a few complex use cases which involve a lot of ifs and elses and handling of customer data. I can't go into the exact details here so let's make up a sample for some context. Assume you have an order processing system where you process orders for a customer. We use Java/Spring with dependency injection and all the bells and whistles so our service would look like this:

class OrderService {

    // these are all dependency-injected by Spring -leaving this out for clarity.
    private CustomerService customerService;
    private OrderDao orderDao;
    private DiscountCodeService discountCodeService;
    private CustomerLoyaltyProgramService loyaltyProgramService;
    private TaxService taxService;


    public Order createOrder(OrderForm form) {
         // form is pre-validated through JSR303 annotations but there are 
         // some additional validations
         // that need to be made
         // 
         if (formHasError) {
              throw SomeException();
         }

         // ok we can continue here.
         Customer customer = customerService.getCustomerById(form.getCustomerId())

         // build up an order from the form
         Order order = new Order();
         // copy over stuff..

         // check if customer has a discount code
         if (form.getDiscountCode() != null) {
             discountCodeInfo = discountCodeService.getDiscountCodeInfoFor(form.getDiscountCode())
             // now walk over line items to see if there is some applicable order line item.
         }

         // check if the customer is in any loyalty program that will give him 
         // additional discounts
         List<LoyaltyProgram> programs = loyaltyProgramService.findByCustomer(customer);
         // do some more ifs and elses to apply loyalty program discounts

         // check applicable sales tax for customer
         SalesTax tax = taxService.findSalesTaxFor(form.getAddress())

         // do some tax calculations, more ifs and elses

         // finally store order
         orderDao.save(order);
         return order;

    }

}

Testing such a service with a unit test is basically a nightmare. You have to mock all the dependencies where the mocking depends on the flow through the method and the different business rules that are applied on certain cirumstances. At a certain amount of mocks and code paths your head will simply explode.

Another approach we tried was to split this up into several private (well protected, as you need to access them in a test) methods, which would make it more testable. However then our business logic is spread all over the place which makes it hard to follow in code and you still have to have some "master method" which calls all the little helper methods in correct order and you need to test that as well so you're basically back to square one.

So what I'm looking for is some way to reorganise this which allows you to test the methods easier without having to think about all the dependencies while still keeping the code maintainable and not spreading it all out to a thousand different places where you cannot follow the logic anymore. I figure that this is probably going to be some trade-off, but I wonder how you may have tackled this in your projects.

  • What are you trying to test? That the createOrder method calls the services in right sequence, or that calculations like discounts, loyalty programs and taxes are calculated correctly? If the later, how come you just glossed it over in your example? You seem to be focusing on least important of your business logic. – Euphoric Aug 10 '17 at 8:38
  • Well ultimately i want to test that the order that comes out of this method is processed correctly. But in order to do that, I still need to mock all the collaborating services, so the method can execute properly. Also the return values of the collaborators affect the expected outcome (e.g. if the customer does not participate in a loyalty program then the order would look differently, even when I put the exact same order form in). – kork Aug 10 '17 at 9:08
5

I think this method is doing too much. It's not cohesive. Splitting into private methods won't solve the issue here, and it will probably make it more worse.

I think you should change the design so that several components can collaborate. Each component will be small, cohesive, and focused.

For your specific example I would recommend a filter chain. A filter chain is a linked list of processors that are executed sequentially and each link in the chain can choose to bail execution, or can adjust the message being passed through the chain.

You can redesign your method into several links like so:

enter image description here

Each component will only have a couple dependencies, and you can create the chain at your composition root.

OrderProcessor persistOrder = new PersistOrderProcessor(null);
OrderProcessor taxOrder = new TaxOrderProcessor(taxService, persistOrder);
OrderProcessor discountCodeProcessor = new DiscountCodeOrderProcessor(discountCodeService, taxOrder);
OrderProcessor loyaltyProcessor = new LoyaltyOrderProcessor(loyaltyService, discountCodeProcessor);
OrderProcessor root = new ValidateOrderProcessor(loyaltyProcessor);

Each processor's implementation looks something like this:

public Order process(order: Order) throws OrderProcessorException {
    // optionally validate order
    if (!valid) {
        throw new InvalidOrderException(...);
    }
    // modify order
    order.setTax(.3f * order.getSubtotal());
    // continue the chain (or decide not to)
    if (next != null) {
        order = next.process(order);
    }
    // optionally modify the order again
    return order;
}

Then you modify OrderService to inject an OrderProcessor as a dependency and start the chain.

class OrderService {

    // these are all dependency-injected by Spring -leaving this out for clarity.
    private OrderProcessor orderProcessor;

    public Order createOrder(OrderForm form) { 
         // build up an order from the form
         Order order = new Order();
         // copy over stuff..

         // start the chain
         return orderProcessor.process(order);
    }
} 

This design allows you to unit-test each part of your order pipeline in isolation with one or two mocks. You have a design that is scalable and flexible to your growing needs to add more logic to order placement. Each step in the processing chain is small and cohesive. The naming of the component is indicative to the responsibility and will be easy for others to navigate.

  • 2
    This is a great extraction of existing logic. If the order of the steps does not matter, they could each be a Visitor pattern; however, this is unlikely the case. Instead, you may want a Workflow because you have to account for complex exceptional scenarios such as a 3rd party Loyalty API returns an error invalidating the otherwise valid discount. And I'm sure in no time you'll have multiple incompatible discounts, taxes/exemptions, shipping, inventory, and loyalty programs which are different for each item within the order. – Graeme Wicksted Aug 10 '17 at 14:23
  • @GraemeWicksted that seems to be the way of all things in business software. – kork Aug 11 '17 at 6:10
  • @Samuel this looks like a very nice approach to me. In fact it is so obvious that I wonder why I didn't come up with this myself. Sometimes it's good to leave your hut for a few hours and get some fresh air and different opinions in. Thanks a lot :) – kork Aug 11 '17 at 6:11
0

Maybe, testing the front-end may serve better at that point than unit-testing. Think of a test server that you initialize in a defined state (known test records in database), then simulate all of the ordering processes using a framework such as Selenium, and afterwards check if all of the orders have correctly been written to the database.

  • Yes, we use Geb/Spock to do frontend tests as well and they help a great deal. The drawback of these is that they are very slow to execute, so we'd like as much tests as possible to be unit tests, so we can quickly run them and not need to wait for an hour or so to see if we introduced a bug. – kork Aug 10 '17 at 9:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.