I have a question about the best way to design classes in order to be test-friendly. Suppose I have an OrderService class, which is used to place new orders, check the status of orders, and so on. The class will need to access customer information, inventory information, shipping information, etc. So the OrderService class will need to use CustomerService, InventoryService, and ShippingService. Each service also has its own backing repository.
What is the best way to design the OrderService class to be easily testable? The two commonly used patterns that I've seen are dependency injection and service locator. For dependency injection, I'd do something like this:
class OrderService
{
private ICustomerService CustomerService { get; set; }
private IInventoryService InventoryService { get; set; }
private IShippingService ShippingService { get; set; }
private IOrderRepository Repository { get; set; }
// Normal constructor
public OrderService()
{
this.CustomerService = new CustomerService();
this.InventoryService = new InventoryService();
this.ShippingService = new ShippingService();
this.Repository = new OrderRepository();
}
// Constructor used for testing
public OrderService(
ICustomerService customerService,
IInventoryService inventoryService,
IShippingService shippingService,
IOrderRepository repository)
{
this.CustomerService = customerService;
this.InventoryService = inventoryService;
this.ShippingService = shippingService;
this.Repository = repository;
}
}
// Within my unit test
[TestMethod]
public void TestSomething()
{
OrderService orderService = new OrderService(
new FakeCustomerService(),
new FakeInventoryService(),
new FakeShippingService(),
new FakeOrderRepository());
}
The disadvantage to this is that every time I create an OrderService object that I'm using in a test, it takes a lot of code to call the constructor within my tests. My Service classes also end up with a bunch of properties for each Service and Repository class that they use. And as I expand my program and add more dependencies between various Service and Repository classes, I have to go back and add more and more parameters to constructors of classes that I've already made.
For a service locator pattern, I could do something like this:
class OrderService
{
private CustomerService CustomerService { get; set; }
private InventoryService InventoryService { get; set; }
private ShippingService ShippingService { get; set; }
private OrderRepository Repository { get; set; }
// Normal constructor
public OrderService()
{
ServiceLocator serviceLocator = new ServiceLocator();
this.CustomerService = serviceLocator.CreateCustomerService()
this.InventoryService = serviceLocator.CreateInventoryService();
this.ShippingService = serviceLocator.CreateShippingService();
this.Repository = serviceLocator.CreateOrderRepository();
}
// Constructor used for testing
public OrderService(IServiceLocator serviceLocator)
{
this.CustomerService = serviceLocator.CreateCustomerService()
this.InventoryService = serviceLocator.CreateInventoryService();
this.ShippingService = serviceLocator.CreateShippingService();
this.Repository = serviceLocator.CreateOrderRepository();
}
}
// Within a unit test
[TestMethod]
public void TestSomething()
{
OrderService orderService = new OrderService(new TestServiceLocator());
}
I like how the service locator pattern results in less code when calling the constructors, but it also gives less flexibility.
What's the recommended way to set up my Service classes that have dependencies on several other Services and Repositories so that they can be easily tested? Are either or both of the ways that I showed above good, or is there a better way?
There is a difference between code that is "testable" and code that is loosely coupled.
The primary purpose of using DI is loose coupling. Testability is a side-benefit that is gained from loosely coupled code. But code that is testable isn't necessarily loosely coupled.
While injecting a service locator is obviously more loosely coupled than having a static reference to one, it is still not a best practice. The biggest drawback is lack of transparency of dependencies. You might save a few lines of code now by implementing a service locator and then think you are winning, but whatever is gained by doing so is lost when you actually have to compose your application. There is a distinct advantage to looking at the constructor in intellisense to determine what dependencies a class has then to locating the source code for that class to try to work out what dependencies it has.
So, as you might have guessed, I am recommending you use constructor injection. However, you also have an anti-pattern known as bastard injection in your example. The primary drawback of bastard injection is that you are tightly coupling your classes on each other by newing them up internally. This may seem innocent, but what would happen if you needed to move your services into separate libraries? There is a good chance that would cause circular dependencies in your application.
The best way to deal with this (especially when you are dealing with services and not configuration settings) is to either use pure DI or a DI container and just have a single constructor. You should also use a guard clause to ensure there is no way to create your order service without any dependencies.
class OrderService
{
private readonly ICustomerService customerService;
private readonly IInventoryService inventoryService;
private readonly IShippingService shippingService;
private readonly IOrderRepository repository;
// Constructor used for injection (the one and only)
public OrderService(
ICustomerService customerService,
IInventoryService inventoryService,
IShippingService shippingService,
IOrderRepository repository)
{
if (customerService == null)
throw new ArgumentNullException("customerService");
if (inventoryService == null)
throw new ArgumentNullException("inventoryService");
if (shippingService == null)
throw new ArgumentNullException("shippingService");
if (repository == null)
throw new ArgumentNullException("repository");
this.customerService = customerService;
this.inventoryService = inventoryService;
this.shippingService = shippingService;
this.repository = repository;
}
}
// Within your unit test
[TestMethod]
public void TestSomething()
{
OrderService orderService = new OrderService(
new FakeCustomerService(),
new FakeInventoryService(),
new FakeShippingService(),
new FakeOrderRepository());
}
// Within your application (pure DI)
public class OrderServiceContainer
{
public OrderServiceContainer()
{
// NOTE: These classes may have dependencies which you need to set here.
this.customerService = new CustomerService();
this.inventoryService = new InventoryService();
this.shippingService = new ShippingService();
this.orderRepository = new OrderRepository();
}
private readonly IOrderService orderService;
private readonly ICustomerService customerService;
private readonly IInventoryServcie inventoryService;
private readonly IShippingService shippingService;
private readonly IOrderRepository orderRepository;
public ResolveOrderService()
{
return new OrderService(
this.customerService,
this.inventoryService,
this.shippingService,
this.orderRepository);
}
}
// In your application's composition root, resolve the object graph
var orderService = new OrderServiceContainer().ResolveOrderService();
I also agree with Gordon's answer. If you have 4 service dependencies it is a code smell that your class is taking on too many responsibilities. You should consider refactoring to aggregate services to make your classes singular in responsibility. Of course, 4 dependencies is sometimes necessary, but it is always worth taking a step back to see if there is a domain concept that should be another explicit service.
NOTE: I am not necessarily saying that Pure DI is the best approach, but it can work for some small applications. When an application becomes complex, using a DI container can pay dividends by using convention-based configuration.