Unit testing and reusing extracted class

Question

Currently I have 3 service classes, they all have a similar pattern, and I need to make a 4th service class which amalgamates the functionality of the other three.

Current three classes

public class PersonService : IPersonService
{
    public PersonServiceResult CreatePerson(CreatePersonParams createPersonParams)
    {
        using (var unitOfWork = _unitOfWorkFactory.Create())
        {
            // [code]

            unitOfWork.Commit();
            return new PersonServiceResult(...);
        }
    }
}

public class OrderService : IOrderService
{
    public OrderServiceResult CreateOrder(
        int personId, 
        IEnumerable<PurchaseItem> items)
    {
        using (var unitOfWork = _unitOfWorkFactory.Create())
        {
            // [code]

            unitOfWork.Commit();
            return new OrderServiceResult(...);
        }
    }
}

public class PaymentProcessingService : IPaymentProcessingService
{
    public PaymentResult ProcessPayment(int orderId, CreditCard creditCard)
    {
        using (var unitOfWork = _unitOfWorkFactory.Create())
        {
            // [code]

            unitOfWork.Commit();
            return new PaymentResult(...);              
        }
    }
}

Now I have a need to use the behavior in all 3 of these classes into one unit-of-work, I still need the 3 other classes to exist for other dependecies in the project.

My first thought would be to extract a class for each one of the 3 services which takes a unit-of-work as an argument.

Extraction of behavior from current classes

public class PersonCreator
{
    public CreatePersonResult CreatePerson(
        IUnitOfWork unitOfWork, 
        CreatePersonParams createPerson)
    {
        // [code]
    }
}

public class PersonService : IPersonService
{
    private readonly PersonCreator Creator = new PersonCreator();

    public PersonServiceResult CreatePerson(CreatePersonParams createPersonParams)
    {
        using (var unitOfWork = _unitOfWorkFactory.Create())
        {
            var result = Creator.CreatePerson(unitOfWork, createPersonParams);

            unitOfWork.Commit();
            return new PersonServiceResult(result);
        }
    }       
}

Proposed new class using extracted behavior

public class QuickOrderService : IQuickOrderService
{
    private readonly PersonCreator PersonCreator = new PersonCreator(); 
    private readonly OrderCreator OrderCreator = new OrderCreator();    
    private readonly PaymentProcessor PaymentProcessor = new PaymentProcessor();    

    public QuickOrderResult CreateQuickOrder(
        CreatePersonParams createPersonParams,
        IEnumerable<PurchaseItem> items,
        CreditCard creditCard)
    {
        using (var unitOfWork = _unitOfWorkFactory.Create())
        {
            var person = PersonCreator.CreatePerson(unitOfWork, createPersonParams);
            var order = OrderCreator.CreateOrder(unitOfWork, person.Id, items);
            var payment = PaymentProcessor.Process(unitOfWork, order.Id, creditCard);

            unitOfWork.Commit();

            return QuickOrderResult(...);
        }
    }
}

This works fine, but I am concerned about testing this new class.

Questions about proposal

1. Duplicate the majority of the tests for the QuickOrderService class
2. Do not test this class as the majority of it is covered by other tests
3. Inject PersonCreator, OrderCreator, and PaymentProcessor into QuickOrderService instead of instanciating them and just test that QuickOrderService calls the dependencies correctly

Though when doing this whole excersise it makes me think there may be a better solution that reduces duplication while retains code coverage.

Answer 1

I would start with eliminating the duplicated code first by building a generic creation service, something along the lines of

   class GenericCreatorService<T>
   {
        UnitOfWorkFactory _unitOfWorkFactory;

        // ...

        public T Create(Func<T,UnitOfWork> func)
        {
           using (var unitOfWork = _unitOfWorkFactory.Create())
           {
                T result = func(unitOfWork);
                unitOfWork.Commit();
                return result;
           }
        }
   }

Now you just need to unit-test that once. Reuse it to implement your original service classes, for example,

public class PersonService : IPersonService
{
    private readonly PersonCreator Creator = new PersonCreator();

    public PersonServiceResult CreatePerson(CreatePersonParams createPersonParams)
    {
        var gs = new GenericCreatorService<PersonServiceResult>(...);
        return gs.Create( uow => PersonCreator.CreatePerson(uow,createPersonParams))
    }       
}

You should focus on testing the individual Creator classes, one test for each, and its debatable if you really need to unit test the individual service classes any more, since they contain only some very simple orchestration code. If you feel you need an extra test for your QuickOrderService because the real implementation is more complicated, you can still do what @JDT suggested and use DI for the other creators.

Answer 2

Inject the three Creator-classes and test the calls to them by unittesting the QuickOrderService. This keeps the tests on the relevant classes.

Also, you might not need to haul the unitOfWork around. The implementation of Create could be changed to simply create a new transaction if none exists and otherwise return the existing transaction. This could keep track of the amount of calls to Create and Commit and only commit the transaction when the 'last' transaction that was requested is committed. Right now you have three services that take a unitOfWork and one that doesn't. Strive for consistency in your project and either pass ALL the services a unitOfWork or have the service manage a unitOfWork for itself.