Validation rules with support for immediate termination of processing

Question

I am picking up a code smell from this code and I am not sure of the best way to rid myself of it.

I have a validation class that has a function that returns a list of IValidationRule<T> implementations (RuleOne, RuleTwo, RuleThree).

public interface IValidationRule<T>
{
    ValidationRuleResult Validate(T instance);
}

public override ValidationRules<Order> AllRules() 
=> new ValidationRules<Order>
{
    Rules =
    {
        RuleOne(),
        RuleTwo(),
        new StopProcessingIfInvalidCheckpoint<Order>(),
        RuleThree()
    }
};

As the rules are iterated through, the IValidationRule<T>.Validate method is called on them. They will return their results which are added to a collection.

I implemented a class that inherits from IValidationRule<T> called StopProcessingIfInvalidCheckpoint which will cease the processing of any rules following it if any of the rules before it fail.
I felt this was a clean and visual way to kick out of the validation routine early.

What I don't like is that the StopProcessingIfInvalidCheckpoint class has to be passed the same generic as all the other rules around it. The Validate method on this class is never even called and thus, it does not matter that it knows it's generic is an Order.

Below is the extension method I use to validate a list of rules and you can see where I kick out if I come across the StopProcessingIfInvalidCheckpoint class:

public static ValidationRuleResult Validate<T>(
    this IEnumerable<IValidationRule<T>> validationRules, 
    T instance
)
{
    var validationResults = new List<ValidationResult>();

    foreach (var vr in validationRules)
    {
        if (vr is StopProcessingIfInvalidCheckpoint<T> && validationResults.Any()) { break; }
        var result = vr.Validate(instance);
        if (result.IsValid) { continue; }
        validationResults.AddRange(result.ValidationResults);
    }

    var res = new ValidationRuleResult
    {
        ValidationResults = validationResults,
        IsValid = !validationResults.Any()
    };
    return res;
}

How can I improve this code?

Answer 1

You shouldn't bind the implementation to any particular type in this case the StopProcessingIfInvalidCheckpoint.

I think it would be better to drop this class and instead add a new property IsFinal to the interface. You could check if the validation fails and IsFinal is set to true then you stop processing other rules.

If you want to get rid of the generic argument but at same time you want to keep it for other implementations then add another abstraction layer. Make the interface non-generic and add an abstract class that is derived from the non-generic interface:

interface IValidationRule
{
    ValidationRuleResult Validate(object instance);
    bool IsFinal { get; }
}

abstract class ValidationRule<T> : IValidationRule
{
    public abstract ValidationRuleResult Validate(object instance);
    public bool IsFinal { get; set; }
}

class FooValidationRule : ValidationRule<int>
{
    public override ValidationRuleResult Validate(object instance)
    {
        ... do something with (T)instance;
    }
}

and

foreach (var vr in validationRules)
{
    var result = vr.Validate(instance);
    if (!result.IsValid && vr.IsFinal)
    {
        break;
    }
    validationResults.AddRange(result.ValidationResults);
}

so

Rules =
{
    RuleOne(),
    RuleTwo() { IsFinal = true },        
    RuleThree()
}