Many times I have seen validation code implemented in a similar manner as the code below.
public class SaleProcess
{
public bool IsPersistValid(Sale sale)
{
List<string> validationErrors = new List<string>();
if (sale.Date > DateTime.Now)
{
validationErrors.Add("Sale date is in the future");
}
if (sale.Customer.Id == 0)
{
validationErrors.Add("Customer has not been saved");
}
if (sale.Products.Count == 0)
{
validationErrors.Add("Sale contains no products");
}
//If there are validation errors throw an exception
if (validationErrors.Count > 0)
{
ValidationException validationException = new ValidationException
{
ValidationErrors = validationErrors
};
throw validationException;
}
return true;
}
}
Obviously this example is extremely simple but I still have number of issues with it
- It is difficult to unit test as you have to be very careful when setting up the test data to ensure all the various if statements are hit
- If you want to add new validation you have to change the SaleProcess class, which does not obey the open/closed principle.
- The IsPersistValid method is not just performing one action (single responsibility principle) it is in fact responsibility for performing many bits of validation.
- None of the validation logic in the IsPersistValid method is reusable, unless you think cut and pasting makes the code reusable.
All of the above problems will be worse for more complex validation scenarios that are implemented in the same way.
To avoid the above problems, I implement the individual pieces of validation as a series of validation commands as shown below
Interface which defines the validation command interface
public interface IValidationCommand
{
List<string> Validate();
}
Sale validation command base class, contains any common functionality shared between all the sale validation commands
public abstract class SaleValidationCommandBase : IValidationCommand
{
protected Sale Sale { get; private set; }
protected SaleValidationCommandBase(Sale sale)
{
Sale = sale;
}
public abstract List<string> Validate();
}
Listed below are two example validation commands which are responsible for validating the sale and the sale customer’s data
public class SaleValidationCommand : SaleValidationCommandBase
{
public SaleValidationCommand(Sale sale) : base(sale) { }
public override List<string> Validate()
{
List<string> validationErrors = new List<string>();
if (Sale.Date > DateTime.Now)
{
validationErrors.Add("Sale date is in the future");
}
return validationErrors;
}
}
public class SaleCustomerValidationCommand : SaleValidationCommandBase
{
public SaleCustomerValidationCommand(Sale sale) : base(sale) { }
public override List<string> Validate()
{
List<string> validationErrors = new List<string>();
if (Sale.Customer.Id == 0)
{
validationErrors.Add("Customer has not been saved");
}
return validationErrors;
}
}
As you can see the individual validation commands are only responsible for validating one piece of data and are therefore simple to test, they can also be tested in isolation from all the other validation logic. As the pieces of validation logic are encapsulated in a number of validation command classes they can be simply reused elsewhere in the code where the same validation is required to be run.
So far we have encapsulated the separate bits of validation in a number of classes but we have not actually wired the validation up. We could simply create instances of the validation commands in the IsPersistValid method and call them there. I tend to avoid this in all but the simplest cases the reasons for this are to increase the testability of the code and to ensure that the IsPersistValid method is only responsible for doing one thing - executing the validation it does not need to know what validation needs to be run, that can be the responsibility of another class as shown below
public class PersistSaleValidator : IPersistSaleValidator
{
public List<string> Validate(Sale sale)
{
List<string> validationErrors = new List<string>();
CreateValidationCommands(sale).ForEach(validator => validationErrors.AddRange(validator.Validate()));
return validationErrors;
}
private List<IValidationCommand> CreateValidationCommands(Sale sale)
{
return new List<IValidationCommand>
{
new SaleValidationCommand(sale),
new SaleCustomerValidationCommand(sale),
new SaleProductsValidationCommand(sale)
};
}
}
Below is the final version of the SaleProcess calling the PersistSaleValidator to execute the validation.
public class SaleProcess
{
private readonly IPersistSaleValidator validator;
public SaleProcess(IPersistSaleValidator validator)
{
this.validator = validator;
}
public bool IsPersistValid(Sale sale)
{
List<string> validationErrors = validator.Validate(sale);
//If there are validation errors throw an exception
if (validationErrors.Count > 0)
{
ValidationException validationException = new ValidationException
{
ValidationErrors = validationErrors
};
throw validationException;
}
return true;
}
}
Notice that the IsPersistValid method is now much simplier and it responsibilities are clear.
As an aside you might have noticed that I have created an interface for the PersistSaleValidator and that the SaleProcess constructor takes an instance of that interface. The reasons for doing this include
- It is a good idea to program against an abstraction because if the implementation of the validator changes the SaleProcess requires no change (assuming the method signature does not change).
- As the dependencies of the SaleProcess are all passed in via the constructor (Dependency Injection) we can do iteraction based testing using a framework such as Rhino Mocks.