I’ve come to the conclusion that mixing object creation logic (e.g. new statements) with business logic is a bad idea; this is because
- A method should only be responsible for one thing (Single Responsibility Principle), either it creates things or performs logic on things not both.
- Code littered with both creation logic and business logic tends to be long and hard to read.
- It is good practice to pass dependencies into a class (dependency injection) in order to promote looser coupling.
- If a method creates its dependencies it is very hard to unit test using a framework such as Rhino Mocks.
The OrderProcess class shown below is a good example of why mixing object creation logic with business logic is a bad idea.
/// <summary>
/// Order process
/// </summary>
public class OrderProcess
{
/// <summary>
/// Processes the payment of the order
/// </summary>
/// <param name="order">The order</param>
/// <param name="paymentMethod">The method of payment</param>
public void ProcessPayment(Order order, PaymentMethod paymentMethod)
{
// Calculate the order's total price
order.TotalPrice = order.Products.Sum(product => product.Price);
// If the customer is a premium customer apply a discount
// to the price
if (CustomerType.Premium.Equals(order.Customer))
{
order.TotalPrice -= 50;
}
// Get the payment processor based on
// the method of payment
IPaymentProcessor paymentProcessor = null;
switch (paymentMethod)
{
case PaymentMethod.Cash:
paymentProcessor = new CashPaymentProcessor();
break;
case PaymentMethod.Cheque:
paymentProcessor = new ChequePaymentProcessor();
break;
case PaymentMethod.CreditCard:
paymentProcessor = new CreditCardPaymentProcessor();
break;
}
// Process the order payment
if (paymentProcessor != null)
{
paymentProcessor.Process(order);
}
}
}
As you can see the ProcessPayment method is responsible for both business logic (working out the order price and applying a discount) and creating a payment processor based on the method of payment. The code would be much cleaner if the decision on which payment processor to use was another class’s responsibility. To accomplish this I’ve extracted the logic that is responsible for creating the payment processor into a factory and passed an instance of this factory into the OrderProcess class as shown below.
/// <summary>
/// Payment processor factory
/// </summary>
public class PaymentProcessorFactory : IPaymentProcessorFactory
{
/// <summary>
/// Creates a payment processor based on the
/// method of payment
/// </summary>
/// <param name="paymentMethod">Method of payment</param>
/// <returns>Payment processor instance</returns>
public IPaymentProcessor CreatePaymentProcessor(PaymentMethod paymentMethod)
{
// Get the payment processor based on
// the method of payment
IPaymentProcessor paymentProcessor = new NullPaymentProcessor();
switch (paymentMethod)
{
case PaymentMethod.Cash:
paymentProcessor = new CashPaymentProcessor();
break;
case PaymentMethod.Cheque:
paymentProcessor = new ChequePaymentProcessor();
break;
case PaymentMethod.CreditCard:
paymentProcessor = new CreditCardPaymentProcessor();
break;
}
return paymentProcessor;
}
}
/// <summary>
/// Order process
/// </summary>
public class OrderProcess
{
/// <summary>
/// Payment processor factory instance
/// </summary>
private readonly IPaymentProcessorFactory paymentProcessorFactory;
/// <summary>
/// Initializes a new instance of the OrderProcess class
/// </summary>
/// <param name="paymentProcessorFactory">Payment processor factory</param>
public OrderProcess(IPaymentProcessorFactory paymentProcessorFactory)
{
this.paymentProcessorFactory = paymentProcessorFactory;
}
/// <summary>
/// Processes the payment of the order
/// </summary>
/// <param name="order">The order</param>
/// <param name="paymentMethod">The method of payment</param>
public void ProcessPayment(Order order, PaymentMethod paymentMethod)
{
// Calculate the order's total price
order.TotalPrice = order.Products.Sum(product => product.Price);
// If the customer is a premium customer apply a discount
// to the price
if (CustomerType.Premium.Equals(order.Customer))
{
order.TotalPrice -= 50;
}
// Process the order payment
IPaymentProcessor paymentProcessor = this.paymentProcessorFactory.CreatePaymentProcessor(paymentMethod);
paymentProcessor.Process(order);
}
}
As you can see it is now much clearer what the ProcessPayment method is doing now that the object creation logic has been removed. In addition to this the OrderProcess class is now much easier to unit test as the IPaymentProcessorFactory instance passed to the constructor can be mocked or stubbed very easily. There other benefits including if we added a new method of payment (and therefore a new payment processor) no change would need to be made to the OrderProcess class as the change would be isolated to the PaymentProcessorFactory.