2
\$\begingroup\$

A couple of model classes are doing the validation using FluentValidator. These model classes are used to Add and Edit the branch data and they are working well but I have no idea how to remove the code smell (duplication).

Problem: Notice the duplicate validations being written in BranchEditModel class as well. I have marked in the code itself what I think is being duplicated I thought it would be easier for you to read it.

BranchAddModel

using FluentValidation;
using FluentValidation.Attributes;

namespace Application //sample namespace
{
    [Validator(typeof(BranchAddModelValidator))]
    public class BranchAddModel
    {
        public string BranchName { get; set; }
        public byte ServiceTypeId { get; set; }
        public short TimezoneId { get; set; }
    }

    public class BranchAddModelValidator : AbstractValidator<BranchAddModel>
    {
        public BranchAddModelValidator()
        {
            RuleFor(x => x.BranchName)
                .NotEmpty()
                .Length(0, 128);  //up to 128 length of string is allowed.

            RuleFor(x => x.ServiceTypeId)
                .NotEmpty();  //mandatory

            RuleFor(x => x.TimezoneId)
                .NotEmpty();  //mandatory
        }
    }
}

BranchEditModel

BranchEditModel is inheriting the model from BranchAddModel. And, it has 2 extra properties.

namespace Application //sample namespace
{
    [Validator(typeof(BranchEditModelValidator))]
    public sealed class BranchEditModel : BranchAddModel
    {
        public int BranchId { get; set; }
        public bool IsActive { get; set; }
    }

    public class BranchEditModelValidator: AbstractValidator<BranchEditModel>
    {
        public BranchEditModelValidator()
        {
            RuleFor(x => x.BranchName) // code repeated
                .NotEmpty()
                .Length(0, 128);

            RuleFor(x => x.ServiceTypeId) // code repeated
                .NotEmpty();  

            RuleFor(x => x.TimezoneId) // code repeated
                .NotEmpty();

            RuleFor(x => x.BranchId) 
                .NotEmpty();

            RuleFor(x => x.IsActive)
                .NotEmpty();
        }
    }
}
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

I see two possibilities here.

1. Use extension methods

You can define a static class BranchModelValidationRules with extension methods for AbstractValidator<T> with methods like this:

public static void ValidateBranchName<T>(this AbstractValidator<T> validator)
    where T : BranchAddModel
{
    validator.RuleFor(x => x.BranchName)
        .NotEmpty()
        .Length(0, 128);
}

2. Extract a common base class

Create a class BranchModelValidatorBase:

public abstract class BranchModelValidatorBase<T> : AbstractValidator<T> 
    where T : BranchAddModel
{
    public BranchModelValidatorBase()
    {
        RuleFor(x => x.BranchName)
            .NotEmpty()
            .Length(0, 128);  //up to 128 length of string is allowed.

        RuleFor(x => x.ServiceTypeId)
            .NotEmpty();  //mandatory

        RuleFor(x => x.TimezoneId)
            .NotEmpty();  //mandatory
    }
}

Create concrete validators by extending the base class:

public class BranchAddModelValidator
    : BranchModelValidatorBase<BranchAddModel> 
{
    public BranchAddModelValidator()
    {
        // Create future add-specific rules here.
    }
}

public class BranchEditModelValidator
    : BranchModelValidatorBase<BranchEditModel> 
{
    public BranchEditModelValidator()
    {
        RuleFor(x => x.BranchId) 
            .NotEmpty();

        RuleFor(x => x.IsActive)
            .NotEmpty();
    }
}

Update

Regarding your comment about a more generic Name property: define an interface, for example like this:

public interface IModelWithName
{
    string Name { get; set; }
}

A matching extension method class might then look like this:

public static class ModelValidationRules
{
    public static void ValidateModelName<T>(this AbstractValidator<T> validator)
        where T : IModelWithName
    {
        validator.RuleFor(x => x.Name)
            .NotEmpty()
            .Length(0, 128);
    }
}

You can then reuse this method in any validator class that validates models implementing the interface. Alternatively you could create a validator for IModelWithName with just a single rule. Then use a matching set of validators for actual model validation.

\$\endgroup\$
4
  • \$\begingroup\$ I was thinking to extract out the BranchName into more Generic like NameField so this could be used in other Model Classes too. Not sure whether this make sense? I don't know how to implement. \$\endgroup\$ Oct 22, 2016 at 10:58
  • \$\begingroup\$ Use an interface. See my updated answer for details. \$\endgroup\$ Oct 22, 2016 at 12:21
  • \$\begingroup\$ Using Interface may help but not sure how to link up the ModelValidationRules with Concrete Class (eg: BranchAddModel). It's not possible to Add the Validator attribute to the concrete class. \$\endgroup\$ Oct 22, 2016 at 13:15
  • \$\begingroup\$ You still have do define a validator class that extends for example AbstractValidator<BranchAddModel>. In the constructor of said class you can then add the validation rule with this.ValidateModelName();, e.g. replace your current RuleFor(...) statement with it. \$\endgroup\$ Oct 22, 2016 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.