How to return validation errors from service class methods?

Question

Can you give me an example of how to return validation errors from a service class used in a web application. What do you think about this approach below?

using System;
using System.Linq;
using System.Web.Mvc;

using App.Data;
using App.Security;

public interface IMembershipService
{
    bool ValidateUser(string userName, string password, ModelStateDictionary model = null);
}

public class MembershipService : IMembershipService
{
    private DatabaseContext db;

    public MembershipService(DatabaseContext db)
    {
        this.db = db;
    }

    public bool ValidateUser(string userName, string password, ModelStateDictionary model)
    {
        if (string.IsNullOrWhiteSpace(userName) || userName.Length > 128 ||
            string.IsNullOrWhiteSpace(password) || password.Length > 256)
        {
            TryAddModelError(model, "Username or password provided is incorrect.");
            return false;
        }

        var user = this.db.Users.SingleOrDefault(u => u.UserName == userName);

        if (user == null || !PasswordHash.Validate(password, user.PasswordHash, user.PasswordSalt))
        {
            TryAddModelError(model, "Username or password provided is incorrect.");
            return false;
        }

        if (!user.IsApproved)
        {
            TryAddModelError(model, "Your account is suspended.");
            return false;
        }

        user.LastLoginDate = DateTime.UtcNow;
        this.db.SaveChanges();

        return true;
    }

    private static void TryAddModelError(ModelStateDictionary model, string errorMessage)
    {
        if (model != null)
        {
            model.AddModelError(string.Empty, errorMessage);
        }
    }
}

Usage sample:

[Authorize]
public class AccountController : Controller
{
    private readonly IMembershipService membershipService;

    public AccountController(IMembershipService membershipService)
    {
        this.membershipService = membershipService;
    }

    [HttpPost, AllowAnonymous, ValidateAntiForgeryToken]
    public Login(LoginModel model, string returnUrl)
    {
        if (ModelState.IsValid && this.membershipService.ValidateUser(
            model.UserName, model.Password, modelState: ModelState))
        {
            FormsAuthentication.SetAuthCookie(userName, true);
            return RedirectToLocal(returnUrl);
        }

        return View(model);
    }
}

Answer 1

Try this instead:

public bool ValidateUser(string userName, string password)
{
    if (string.IsNullOrWhiteSpace(userName) || userName.Length > 128 ||
    string.IsNullOrWhiteSpace(password) || password.Length > 256)
    {
        throw new ProviderException("Username and password are required");
    }

    var user = this.db.Users.SingleOrDefault(u => u.UserName == userName);

    if (user == null || !PasswordHash.Validate(password, user.PasswordHash, user.PasswordSalt))
    {
        throw new ProviderException("Incorrect password or username");
    }

    return true;
}

Usage of Membership service:

...
try
{
    var result = membership.ValidateUser(userName, password);
    ...
}
catch (ProviderException e)
{
    model.AddModelError(string.Empty, e.Message);
}
...

This way, your MembershipService is only responsible for validation and the ValidateUser method validates the username and password. What is done with the validation result is up to the user of the MembershipService. This is referred to as the Single Responsibility Principle

Answer 2

Short answer is to throw an exception (usually InvalidOperationException).

Long answer, you can create a custom exception to hold the informations you need. Example:

public class CustomValidationErrorException : Exception
{
    public string Id { get; set; } // Custom Id for the operation
    public IEnumerable<string> Messages { get; set; } // Multiple validation error message
}

Regarding to the requirement, you also can create a custom validation class and ref as a parameter. The parameter usage here is to avoid collision when you want to return an object.

EDIT:

There seems some discussion about throwing exception or not during validation. If it is performance matter, I cannot argue, because I'm not expert in that; and during some simple tests that I do, I don't find any performance matter. I will explain in a readability section instead why I prefer Exceptions and when it isn't.

Prefer:

In API side of view, the code will be easier to read. Consider this service example:

public interface ILoginService{
  void Login(string userName, string password);
}

Really simple and readable, and from usage point of view

public void LoginServiceConsumer(ILoginService service){
  //declare username and password
  service.Login(userName, password);
  // do what after login, if throw exception then this operation will ensure to be cancelled automatically
}

No question from my view with the implementation instead. Now, consider returning error messages:

public interface ILoginService{
  IEnumerable<string> Login(string userName, string password);
}

And from usage point of view

public void LoginServiceConsumer(ILoginService service){
  //declare username and password
  IEnumerable<string> validationResult = service.Login(userName, password);
  if(validationResult.Any())
  // do the next operation
}

Without proper documentation or good knowledge to use the object, now it popped up a question. What is the IEnumerable<string> that it returned? Is it only return the error message? Will it return "success" as a message when the validation is ok?

Another downside of this technique is, when you returning an object as execution result, but need to validate it first:

public MyObject Convert(MySource source){
   if(!source.Valid){
     //give error message here
   }
}

How can you do it? One way is to use ref to the message results and assigning it.

public MyObject Convert(MySource source, ref List<string> errorMessage){
   if(!source.Valid){
     errorMessage.Add("The source is not valid");
     return null;
   }
}

But from usage point of view:

public void ConvertConsumer(MySource source){
  List<string> errorMessage = new List<string>(); //you should create a new list
  Convert(MySource source, ref errorMessage); //then pass it
}

First, it required a new list. Second, will the user know that it can return null when the validation is bad? Last thing is the same question, What is the IEnumerable<string> that it set? Is it only set the error message? Will it set "success" as a message when the validation is ok?

Not Prefer:

One condition that I not prefer using exception as validation is when your application is tracked for its exceptions using third party, such as AVICode. It can flood your exception report.