When following SRP, how should I deal with validating and saving entities?

Question

I've been reading Clean Code and various online articles about SOLID lately, and the more I read about it, the more I feel like I don't know anything.

Let's say I'm building a web application using ASP.NET MVC 3. Let's say I have a UsersController with a Create action like this:

public class UsersController : Controller
{
    public ActionResult Create(CreateUserViewModel viewModel)
    {

    }
}

In that action method I want to save a user to the database if the data that was entered is valid.

Now, according to the Single Responsibility Principle an object should have a single responsibility, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility. Since validation and saving to the database are two separate responsibilities, I guess I should create to separate class to handle them like this:

public class UsersController : Controller
{
    private ICreateUserValidator validator;
    private IUserService service;

    public UsersController(ICreateUserValidator validator, IUserService service)
    {
        this.validator = validator;
        this.service= service;
    }

    public ActionResult Create(CreateUserViewModel viewModel)
    {
        ValidationResult result = validator.IsValid(viewModel);

        if (result.IsValid)
        {
            service.CreateUser(viewModel);
            return RedirectToAction("Index");
        }
        else
        {
            foreach (var errorMessage in result.ErrorMessages)
            {
                ModelState.AddModelError(String.Empty, errorMessage);
            }
            return View(viewModel);
        }
    }
}

That makes some sense to me, but I'm not at all sure that this is the right way to handle things like this. It is for example entirely possible to pass an invalid instance of CreateUserViewModel to the IUserService class. I know I could use the built in DataAnnotations, but what when they aren't enough? Image that my ICreateUserValidator checks the database to see if there already is another user with the same name...

Another option is to let the IUserService take care of the validation like this:

public class UserService : IUserService
{
    private ICreateUserValidator validator;

    public UserService(ICreateUserValidator validator)
    {
        this.validator = validator;
    }

    public ValidationResult CreateUser(CreateUserViewModel viewModel)
    {
        var result = validator.IsValid(viewModel);

        if (result.IsValid)
        {
            // Save the user
        }

        return result;
    }
}

But I feel I'm violating the Single Responsibility Principle here.

How should I deal with something like this?

Answer 1

I really don't think you are violating Single-Responsibility-Principle in your second example.

• The UserService class has only one reason to change: if there is a need to change the way you save a user.
  

• The ICreateUserValidator class has only one reason to change: if there is a need to change the way you validate a user.
  

I must admit that your first implementation is more intuitive. However, the validation should be done by the entity that creates the user. The creator itself shouldn't be responsible for validation; it should rather delegate the responsibility to a validator class (as in your second implementation). So, I don't think the second design lacks SRP.

Answer 2

It seems to me that the first example is "closer" to true SRP; it's the responsibility of the Controller in your case to know how to wire things up and how to pass along the ViewModel.

Wouldn't it make more sense to have the whole IsValid/ValidationMessages be part of the ViewModel itself? I haven't dabbled with MVVM, but it seems like the old Model-View-Presenter pattern, where it was okay for the Presenter to handle things like that because it was directly related to presentation. If your ViewModel can check itself for validity, there's no chance to pass an invalid one to the UserService's Create method.