Validation in Business Layer: How to call service methods?

Question

I have created a struct on validating models on Business Layer which is based on Steven's answer.

It is working well but something confuses my mind. I inject UserService in CreateUserValidator to able to use GetUser method. This means I call validator in UserService and create a new UserService instance to check whether user exist.

UserService -> [ValidateUser -> new UserService().GetUser()]

It works but seems to be a very bad design. But I have to use that method.

Could you please let me know how I can solve this problem, or Shouldn't I worry about it?

public class CreateUser
{
    public string Name { get; set; }
    public string Email { get; set; }
}

public sealed class CreateUserValidator : Validator<CreateUser>
{
    private IUserService _userService;
    public CreateUserValidator(IUserService userService)
    {
        _userService = userService;
    }
    protected override IEnumerable<ValidationResult> Validate(
        CreateUser entity)
    {

        var user = _userService.GetUserByEmail(entity.Email);
        if (user != null)
        {
            yield return new ValidationResult("Email", "Email address is already exist!");
        }
    }
}

UserService.cs

public partial class UserService : IUserService
{
    IGenericUnitofWork _uow = null;
    private readonly IValidationProvider _validationProvider;
    public UserService(IGenericUnitofWork uow, IValidationProvider validationProvider)
    {
        _uow = uow;
        _validationProvider = validationProvider;
    }

    public User CreateUser(CreateUser createUser)
    {
        this._validationProvider.Validate(createUser);

        var user = new User()
        {
            Email = createUser.Email,
            Name = createUser.Name,
        };
        _uow.Repository<User>().Insert(User);
        _uow.SaveChanges();
        return user;
    }

    public User GetUser(string email)
    {
        var user = _uow.Repository<User>().Where(m => m.Email == email).FirstOrDefault();
        return user;
    }
}

I guess my initial read is it seems goofy because you are combining your business logic and your data layer logic. The GetUser method should probably be in your `_uow` or equivalent which you could also pass to your validator without having to re-inject. Also, since you are just checking if an email is in use, grabbing the entire `User` might not be the best method as far as readability. Perhaps consider using `if(db.Users.Any(x=>x.Email == email)){return new Validation("Email In Use")` — Tommy, Dec 07 '17 at 11:59
I've DAL based on IGenericRepository and that handles `GetById` etc. But I thought that I've to keep it simple, special functions like `GetUserByEmail` should be in business layer. — Okan Kocyigit, Dec 07 '17 at 12:09
Injection does not always have to be on constructor level. Perhaps `validate(CreateUser entity, IUserService userService)` would be more sensible for your case? Service would call it with `this` as a second argument. — Andrei, Dec 07 '17 at 12:15
@Andrei, good point, I couldn't find a way to inject it with parameter because of IValidationProvider implementation. I'm working on it. https://stackoverflow.com/questions/4776396/validation-how-to-inject-a-model-state-wrapper-with-ninject/40284170 — Okan Kocyigit, Dec 07 '17 at 12:36
@ocanal, I don't think there is a need to inject anything via container in that case, just pass service instance as a parameter directly: `this._validationProvider.Validate(createUser, this);` — Andrei, Dec 07 '17 at 13:48

score 2 · Accepted Answer · answered Dec 08 '17 at 12:30

You dependency graph is cyclic. As described in section 6.3 of Dependency Injection in .NET second edition, dependency cycles are often caused by Single Responsibility Principle violations, as is the case in your design.

The problem is that UserService has too many responsibilities: Creating a user is a different responsibility than getting a user. Creating a user can become a very complex use case, as the validation logic hints at, while getting a user is something typically quite simple. It would therefore be beneficial to split UserService into multiple smaller classes. This would allow the validator to depend on the service that allows retrieving the user by its mail address, while the 'create user' service can depend on the validator.

To take it even one step further, you might want to remove validation from the 'create user' service completely. Validation is a cross-cutting concern, and mixing it with the class that contains the business logic, makes such class harder to maintain.

A design that might benefit you is one where you place all state changing business actions behind a common abstraction, as described here.

l would like to take it one step further by removing validation from the service completely, but it seems to be getting more complicated (for me), I'm sure that if I do that I will understand the benefist when project grows up. But now, I'm telling my self, do I really need that? Actually does project really need that? Splitting service class is a new perspective for me, because until now, I've thought that all domain service method should be in same class. I think that's a good idea. What about injecting UOW to validator, in this way I can also get user by accessing repository by uow. Thnks. — Okan Kocyigit, Dec 08 '17 at 13:12
"I've thought that all domain service methods should be in the same class". I don't know where you learned that, but that is actually a pretty bad idea. — Steven, Dec 08 '17 at 13:17

Validation in Business Layer: How to call service methods?

1 Answers1