Short answer:
You are checking the wrong thing.
Very long answer:
You are trying to check PurchaseOrder but this is an implementation detail. Instead, you should check the operation itself, in which case the partNumber and supplierName parameters.
Checking these two parameters alone would be inconvenient, but it is caused by your design - you are missing an abstraction.
In short, the problem is in your IPurchaseOrderService interface. It should not accept two string arguments, but rather one single argument ( parameter object). Let this parameter of the CreatePurchaseOrder object be CreatePurchaseOrder :
public class CreatePurchaseOrder { public string PartNumber; public string SupplierName; }
With the changed IPurchaseOrderService interface:
interface IPurchaseOrderService { void CreatePurchaseOrder(CreatePurchaseOrder command); }
CreatePurchaseOrder parameters CreatePurchaseOrder wraps the original arguments. This object parameter is a message that describes the purpose of creating a purchase order. In other words: this is a team.
Using this command, you can create an implementation of IValidator<CreatePurchaseOrder> that can perform all the necessary checks, including checking for the availability of a suitable parts supplier and generating user-friendly error reports.
But why is IPurchaseOrderService responsible for checking? Validation is an interdisciplinary task, and you should avoid mixing it with business logic. Instead, you can define a decorator for this:
public class ValidationPurchaseOrderServiceDecorator : IPurchaseOrderService { private readonly IValidator<CreatePurchaseOrder> validator; private readonly IPurchaseOrderService decoratee; ValidationPurchaseOrderServiceDecorator( IValidator<CreatePurchaseOrder> validator, IPurchaseOrderService decoratee) { this.validator = validator; this.decoratee = decoratee; } public void CreatePurchaseOrder(CreatePurchaseOrder command) { this.validator.Validate(command); this.decoratee.CreatePurchaseOrder(command); } }
This way you can add validation by simply wrapping the actual PurchaseOrderService object:
var service = new ValidationPurchaseOrderServiceDecorator( new CreatePurchaseOrderValidator(), new PurchaseOrderService());
The problem, of course, with this approach is that it would be very inconvenient to define such a decorator class for each service in the system. This would cause serious code publishing.
But the problem is caused by a flaw. Defining an interface for a particular service (e.g. IPurchaseOrderService ) is usually problematic. You defined CreatePurchaseOrder and therefore already have that definition. Now you can define one single abstraction for all business operations in the system:
public interface ICommandHandler<TCommand> { void Handle(TCommand command); }
With this abstraction, you can now reorganize your PurchaseOrderService into the following:
public class CreatePurchaseOrderHandler : ICommandHandler<CreatePurchaseOrder> { public void Handle(CreatePurchaseOrder command) { var po = new PurchaseOrder { Part = ..., Supplier = ..., }; unitOfWork.Savechanges(); } }
With this design, you can now define one single universal decorator to handle all checks for each business operation in the system:
public class ValidationCommandHandlerDecorator<T> : ICommandHandler<T> { private readonly IValidator<T> validator; private readonly ICommandHandler<T> decoratee; ValidationCommandHandlerDecorator( IValidator<T> validator, ICommandHandler<T> decoratee) { this.validator = validator; this.decoratee = decoratee; } void Handle(T command) { var errors = this.validator.Validate(command).ToArray(); if (errors.Any()) { throw new ValidationException(errors); } this.decoratee.Handle(command); } }
Note that this decorator is almost the same as the previously defined ValidationPurchaseOrderServiceDecorator , but now as a generic class. This decorator can be wrapped around your new class of service:
var service = new ValidationCommandHandlerDecorator<PurchaseOrderCommand>( new CreatePurchaseOrderValidator(), new CreatePurchaseOrderHandler());
But since this decorator is generic, you can wrap it around every command handler on your system. Blimey! How is it for DRY?
This design also makes it easy to add cross-cutting issues later. For example, your service currently seems to be responsible for calling SaveChanges on a unit of work. It can also be seen as a cross-cutting issue and can be easily passed to the decorator. This way, your service classes will become much easier with less code left for testing.
The CreatePurchaseOrder validator might look like this:
public sealed class CreatePurchaseOrderValidator : IValidator<CreatePurchaseOrder> { private readonly IRepository<Part> partsRepository; private readonly IRepository<Supplier> supplierRepository; public CreatePurchaseOrderValidator( IRepository<Part> partsRepository, IRepository<Supplier> supplierRepository) { this.partsRepository = partsRepository; this.supplierRepository = supplierRepository; } protected override IEnumerable<ValidationResult> Validate( CreatePurchaseOrder command) { var part = this.partsRepository.GetByNumber(command.PartNumber); if (part == null) { yield return new ValidationResult("Part Number", $"Part number {command.PartNumber} does not exist."); } var supplier = this.supplierRepository.GetByName(command.SupplierName); if (supplier == null) { yield return new ValidationResult("Supplier Name", $"Supplier named {command.SupplierName} does not exist."); } } }
And your command handler like this:
public class CreatePurchaseOrderHandler : ICommandHandler<CreatePurchaseOrder> { private readonly IUnitOfWork uow; public CreatePurchaseOrderHandler(IUnitOfWork uow) { this.uow = uow; } public void Handle(CreatePurchaseOrder command) { var order = new PurchaseOrder { Part = this.uow.Parts.Get(p => p.Number == partNumber), Supplier = this.uow.Suppliers.Get(p => p.Name == supplierName),
Please note that command messages will become part of your domain. There is a one-to-one mapping between use cases and commands, and instead of checking entities, these entities will be implementation details. Teams become a contract and receive confirmation.
Please note that this will probably make your life a lot easier if your teams contain as many identifiers as possible. Thus, your system can benefit from the definition of a team as follows:
public class CreatePurchaseOrder { public int PartId; public int SupplierId; }
When you do this, you will not need to check if the part with the specified name exists. The presentation level (or external system) has given you the identifier, so you no longer need to check the existence of this part. The command handler, of course, should fail if there is no part with this identifier, but in this case either a programming error or a concurrency conflict occurs. In any case, there is no need to inform the client of obvious verification errors.
This, however, moves the problem of getting the correct identifiers at the presentation level. At the presentation level, the user will have to select a part from the list so that we can get the identifier of this part. But still, I have tested this to make the system much simpler and more scalable.
It also solves most of the problems mentioned in the comments section of the article you are linking to, for example:
- The problem with serializing entities disappears because the commands can be easily serialized and model binding.
- DataAnnotation attributes can be easily applied to commands, allowing client-side validation (Javascript).
- The decorator can be applied to all command handlers that wrap the completed operation in a database transaction.
- It removes the cyclic link between the controller and the service level (via the ModelState controller), eliminating the need for a controller for a new class of service.
If you want to know more about this type of design, you should definitely read this article .