2

I got an API with a controller and a service, when invoking one of the action in the controller I must apply a validation, this validation need to check the data in DB to validate that is correct.

As far as I can see there are two ways of handling this

1- Validate before calling the "update" to prevent the error

public IActionResult UpdateCustomer(CustomerDto customer)
{
   if (!customerService.Validate(customer))
   {
       return Send400BadRequest();
   }
   customerService.Update(customer);

   return Send200Ok();
}

2- Call the update validate inside and throw an Exception.

 public IActionResult UpdateCustomer(CustomerDto customer)
 {
    customerService.Update(customer);    
    return Send200Ok();
 }

In customer service

  public void Update(CustomerDto customer)
  {
     if (!Validate(customer)
        throws new CustomValidationException("Customer is not valid");
     //Apply update
  }

We already have an ActionFilter to handle the CustomValidationException so it will return a BadRequest.

1) Pros +Don't use Exception to use the running flow

Cons -Controller is more fat, has more decision on each case it will decide which is the output

2) Pros +Operation is more atomic, all logic is inside the service. +Easier to test +Every use of the method will be validated.

Cons -Use exception to manage the flow.

Which is a better solution?

I really need arguments to defend one or the other.

Manjar
  • 3,159
  • 32
  • 44
  • 1
    I wouldn't consider 2 extra lines of code bloating the controller IMO. – Ryan Searle Nov 10 '16 at 09:28
  • To me is not that is bloated or not, but more that the decision shouldn't be in the controller – Manjar Nov 10 '16 at 10:03
  • 1
    @Balder, Use another action filter to do validation before hitting action. Cross-cutting concerns and single responsibility principle. – Nkosi Nov 14 '16 at 16:51
  • If you have a Business Logic Layer and a Service Layer, I prefer to keep all Business Logic Rules including Business Logic Validations in Business Logic Layer and use Service Layer as a wrapper around Business Logic Layer. – Reza Aghaei Nov 16 '16 at 14:54

7 Answers7

3

If you have a Business Logic Layer and a Service Layer, I prefer to keep all Business Logic Rules including Business Logic Validations in Business Logic Layer and use Service Layer as a wrapper around Business Logic Layer and throw Exception in Business methods.

When deciding about whether to use Exception for Business Validation rules or not, you can consider:

1) It's better that your Business methods be a Unit of Work. They should perform a complete task. So it's better they contain also validation rules. This way you can reuse such Business Logic Layer across different Service Layers or use the same Unit of Work in different methods of the same Service Layer. If you throw a Business Validation Exception in Business Logic Layer, you will not face with risk of forgetting validation or using another validation rule by mistake and each service method / action will perform a single task for you and will be as lightweight as possible.

Think about when you may need to expose a WCF service for some clients or for example if you may use ASP.NET MVC without using WebAPI or iff you want to use the same logic in another Action method in the same WebAPI.

If you put Business Logic validations in Web API controller, when creating WCF service methods or creating MVC Actions or other service methods, you may forget to apply validations or may apply wrong validations with different rules in the new Service Layer.

2) Considering the first benefit, can you return a meaningful value from methods that shows success, failure, or contain suitable information about failure reason in output?

I believe it's not suitable to use out put of method for all these goals. The method output is method output, it should be data in such business application. It should not be sometimes a status, sometimes data or some times message. Throwing exception will solve this problem.

Reza Aghaei
  • 120,393
  • 18
  • 203
  • 398
  • You answer is very good and its complete, i gave you the bounty for that, however my question was more conceptual. Actually I do believe is better to throw exception from business logic layer, but I need arguments to debate this, that was the goal of my question, because I'm receiving thoughts on my collegues like "throwing exception cause bad performance" or "throwing exception to handle logic is a code smell" that why I really need arguments to discuss that this is the best solution – Manjar Nov 21 '16 at 15:06
  • Thanks for the feedback, I tried to describe why it's better to throw exception in Business Logic Layer. It's not controlling the flow of application. It make business logic more reusable and more reliable. It guarantee each unit of work, works the same way always. But if you put business validations in an action method, you may forget to apply the same business validation in another action method which uses the same business logic. Or obviously if you use another service layer, again you should call the same business validations in the new service layer. – Reza Aghaei Nov 21 '16 at 15:11
  • @Manjar Back to this post after more than 3 years, I can just confirm the answer is the path to go. While what I shared was based on my experience in N-layered architecture, but I can confirm it's the way to go for union architecture or domain driven design as well. – Reza Aghaei Jan 30 '20 at 15:22
1

I am going against the other opinions on here and saying that the first method both clearly illustrates what the intent of your method is and if you ever decide to not return a 400 error, its a bit easier to pull that off in scenario #1.

Additionally, some thoughts on exceptions. Exceptions should be exceptional, meaning unexpected events that occur in your code. A company not passing a validation check is not an exception-al event, it either does pass or does not pass. Passing a User object into the ValidateCompany() method should throw an exception.

Here is a good answer on the similar topic of exception throwing. It uses an easy example problem to determine when in that case an exception should be thrown or not.

In regards to "Easier to test" - I don't see how. Your controller will have two tests with any option you choose, a valid company and an invalid company. Your service will have two tests with any option you choose, a valid company and an invalid company (obviously simplifying your service layer a bit here). In any case you want to ensure both your controller action and your service layer can handle an invalid and valid company object.

Community
  • 1
  • 1
Tommy
  • 39,592
  • 10
  • 90
  • 121
0

I would prefer 2 :)

Because I think service might be called from another node not only the asp.net controller so it would be nice for me if all validation logic is handled in the single layer like Service layer.

0

i think handling exception by using httpresponse message is much better then any one else. Atleast you got the proper error response with proper http response message as output.

      public HttpResponseMessage TestException()
      {
        try
        {
            //if your code works well
            return Request.CreateResponse(HttpStatusCode.OK);
        }
        catch (Exception ex)
        {

            return  Request.CreateErrorResponse(HttpStatusCode.ExpectationFailed, ex.ToString());
        }
      }
0

I would do it this way:

public IActionResult UpdateCustomer(CustomerDto customer)
    {
    try
        {
        consumerService.Update (customer);
        }
    catch (CustomValidationException)
        {
        return Send400BadRequest ();
        }

    return Send200Ok ();
    }

And in your CustomerService:

   public void Update(CustomerDto customer)     
       {
       if (!Validate(customer))
           throw new CustomValidationException("Customer is not valid");
       }

This way your service has the validation logic. So any other callers to your service will also have their inputs validated before attempting an update and hence will get proper error messages. Your controller will not have any validation code. And moreover, having the try-catch block means you can gracefully handle any other errors the Update call might throw. Hope that helps.

robinhood9
  • 137
  • 1
  • 5
0

Receive a validatable customer model, pass this model to the data service layer, and perform exception handling in the controller if the data service layer fails.

Customer model

Implement IValidatableObject to trap business logic related errors.

public class CustomerModel : IValidatableObject
{
    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        // example validation
        if ( Balance < 100 )
            yield return new ValidationResult("Balance >= 100 req.", new [] { nameof(Balance) });
    }
    public string Name { get; set; }
    public double Balance { get; set; }
}

API Controller

Receive a CustomerModel in your public facing UpdateCustomer API method, then reference ModelState to determine if the customer is valid.

public IActionResult UpdateCustomer(CustomerModel customer)
{
    if(ModelState.IsValid) // model validation succeeded
    {
        try
        {
            customerService.Update(customer);
        }
        catch (ServiceUpdateException svcEx)
        {
            // handled failure at the service layer
            return Conflict();
        }
        catch (Exception ex)
        {
            // unhandled error occured
            return InternalServerError(ex);
        }
        // update succeeded
        return Ok();
    }
    // invalid model state
    return BadRequest();
}

Service (Data Layer)

public void Update(Customer customer)
{
     //Apply update
     try
     {
         database.Update(customer);
     }
     catch (DataStorageException dbEx)
     {
         throw new ServiceUpdateException(dbEx);
     }
     catch
     {
         throw;//unknown exception
     }
}
Aaron Hudon
  • 5,280
  • 4
  • 53
  • 60
0

I think that you should validate in both the controller and the service, but validate slightly different things. Especially if what starts off as just an api turns into an api, with a mvc site and a wpf admin interface as often happens.

Using Validation Attributes and Model Binding gives the data a quick "once over" check. Did they supply the id for the customer to update? Did they submit foreign key value for a related entity? Is the price between 0 and 1,000,000? If you have a website then you also get client validation out of the box. Crucially there no need to connect to the database at all, as the data is "obviously" wrong.

Validation in the service is also required as you may end up with 3 or 4 applications calling this service, and you want to write the business logic once. Some validation eg. referencing foreign keys can only be done in the database, but there is nothing wrong in throwing an exception. Consumers of your api should have access to the range of valid values, users of the website should be choosing from a drop down etc, so this circumstance should be exceptional

ste-fu
  • 6,879
  • 3
  • 27
  • 46