0

After reading this article about validating with service layer I have some doubts.

First: Is it all right to pass the whole model view to the service as shown in article ? I have seen some example where rather then passing whole model that is used by controller (like bool success = _productService.CreateProduct(productModel)) they called services like this:

 bool success = _productService.CreateProduct(productModel.Name, productModel.Category, productModel.Cost)

What are pros/cons of both approaches ?

Second: I can see the logic in using same service for validating model and doing the actual work. On the other hand it means that the service will have to deal with 2 concerns: data validation and data processing. That means more code in the same service and worse testability, right ?

So instead of example code above would it be better to have:

bool valid = _productValidationService(productModel);
if(valid){
    _productService.CreateProduct(productModel);
    //or maybe _productService.CreateProduct(productModel.Name, productModel.Category, productModel.Cost);
}

What are the pros/cons ? Is there something that I don't see ? What do you use ? What is standard approach ?

Rasto
  • 17,204
  • 47
  • 154
  • 245

1 Answers1

1

I prefer to pass the whole model to the service instead of individual properties to avoid cluttering the methods. As far as the validation logic is concerned you are saying that it has to deal with two concerns: business logic validation and actual saving. I think that those two concerns belong to the service. A consumer of this service shouldn't be able to call only the save without validating first and perform those two things in the service ensures that this wouldn't happen. Also the actual saving could be delegated by the service method to one or multiple CRUD repository calls.

So in a controller:

string error;
if (!_productService.TryCreateProduct(productModel, out error))
{
    ModelState.AddModelError("key", error);
    return View(viewModel);
}

In the article you have linked the following approach is used to initialize the service and pass it the model state so that it writes the errors directly:

_productService = new ProductService(
    _productRepository, 
    new ModelStateValidationWrapper(this.ModelState)
);

so that later you could directly:

_productService.CreateProduct(productModel);

This is also a valid and good approach. The only problem is the following line _productService = new ProductService(... inside a controller which is very bad as it is tightly coupling the controller with a particular implementation of a service. The service should be injected by a DI framework but the problem is passing the ModelState as it would create circular dependency. There are some threads here addressing this issue.

Community
  • 1
  • 1
Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • In the article I reference they use different validation approach there is not need to any `out` parameters... Constructor of `ProductService` would be like `public ProductService(IProductRepository prodRepo, IModelValidation modelValidat)` then in controller they initialize `_productService` like `_productService = new ProductService(_productRepository, new ModelStateValidationWrapper(this.ModelState))`. And then in action method they just call `_productService.CreateProduct(productModel)` and model state gets updated automatically by the service. – Rasto Mar 26 '11 at 10:58
  • 1
    @drasto, yes, this is also a valid and good approach. The only problem is the following line `_productService = new ProductService(...` inside a controller which is very bad as they are tightly coupling the controller with a particular implementation of a service. The service should be injected by a DI framework but the problem is passing the `ModelState` as it would create circular dependency. There are some threads here on StackOverflow about solving this issue. – Darin Dimitrov Mar 26 '11 at 11:04
  • can you please edit your answer so that it is consistent with that ? Also your opinion on their approach - passing wrapped reference to ModelState to service constructor - would be interesting to me. It seems that it will make injecting that wrapper to service little compicated... – Rasto Mar 26 '11 at 11:06
  • I'll try to search that threads. If you have them starred or bookmarked or know how to find them quickly it would be very helpful. Thank you. – Rasto Mar 26 '11 at 11:09
  • @drasto, I've updated my answer and included a link to one such thread. – Darin Dimitrov Mar 26 '11 at 11:17