Solving an unwieldy inheritance framework function

Question

I have a simple inheritance framework:

class BaseValidator
{ 
    protected virtual void Validate()
    { 
        // Base validator code .....
    }
}
class PhoneValidator:BaseValidator
{
    public override void Validate()
    {
        base.Validate();
        // custom validation logic...
    }
}
class EmailValidator:BaseValidator
{
    public override void Validate()
    {
        base.Validate();
        // custom validation logic...
    }
}

The problem is in //custom validation logic. The more custom validation logic code that is written , the more unwieldy the Validate() function becomes. I can organize custom validation logic into separate functions, but that would be another mess.

public override void Validate()
{
    base.Validate();
    CustomValidation1();
    CustomValidation2();
    CustomValidation3();
    ...
}

What are other patterns\practices that I can use to manage the Validate() function? Sometimes I feel this is a the problem with inheritance, but suggestions are welcome in any direction.

Answer 1

Take a look at the specification pattern (pdf).

In particular, Microsoft has already implemented this kind of framework as part of one of their (old) application blocks. You can look there for many related ideas.

Answer 2

So I think what you are trying to solve is how to elegantly group separate custom validation checks (each custom validation method) for different uses (email, or phone, etc.).

The issue is you have to declare this information somewhere, and as your existing class structure for the validation uses is good - ie. grouped accordingly to your needs and inheriting from a base class so there is a common signature between all the different validator classes - I don't necessarily think the solution is to change that. So using the Chain of Responsibility pattern, although a nice pattern for validation, isn't really solving the problem as you still need to declare the collection of validators for each validation use.

So I think the answer comes down to declarative preference; would you like to declare them as you do via method calls (simple), attributes on a class using reflection, or a Dictionary of Funcs / Actions? Or as configuration somewhere? You have to declare this information somewhere. Maybe there are some other requirements which impact how you might declare these relationships?

Answer 3

I think your best chance in this is to use the Chain Of Responsiblity pattern, also know as pipeline.

Your class hierarchy is good, however i'd add an extra property to the base class

public bool IsValid {get; set;}

Then you would have your pipeline defined like this

List<BaseValidator> validationPipeline;

in this collection you have to add all the validators you require for a specific scenario, with the following rule, if an object cannot validate the input (because it's not its reponsibilty) then it treats the input as valid and passes the input to the next object in the pipeline, if at the end of the iterarion you have a valid result, then the input is ok, otherwise you can break the validation cycle at the first invalid result

the validation sequence code should be something like this

bool valid = true;
foreach (var validator in validationPipeline)
{
    validator.Validate();
    valid = validator.IsValid;
    if(!valid)
      break;
}
//If valid == true at this point, the input is valid

In this way you can split your validation logic in several classes without compromising the desing or coupling it so much, the important things to keep in mind are:

• The order of the validators is important (the most likely to fail should go first)
• If a validator doesn't have to handle the input it should treat it as valid and leave it for another validator down the chain

A quick & dirty example would be a credit card number validator, with the following validation sequence

1. Validate credit card number (with the standard algortihm)
2. If the credit card is VISA, check something with VISA's system
3. If the credit card is Mastercard, check something with Mastercard's systems

And four scenarios can occur

1- The credit card number is invalid, the first validator runs and stops the iteration because if the number isn't right you shouldn't waste time doing anything else.

2- The credit card is valid, and its a visa card, so the second validator runs against some services and it is successful control is passed to the mastercard validator that sould return that its valid because its not its resposiblity to handle visa cards.

3- The credit card is valid, and its a mastercard, so the second validator returns its valid because it only handles visa cards, and the third validator runs and determines the outcome of the validation process

4- Suppose its a valid credit card number, but it's neither visa nor mastercard, lets say its an American Express card, you could add a fourth validator that would return always false because you only accept visa or mastercard.

Hope it's clear and that it's helpful to you or anyone out there

Answer 4

Well, there are probably several good solutions, but the first one that comes to mind is to shift to an interface-based approach rather than an inheritance based one. You might define an IValidation interface that defines a Validation method, which each class implementor could then customize as necessary.

You could even construct an abstract base class to implement the interface, and provide a basic Validation mechanism that could be called from child classes.

Something like (and this is kinda rough):

interface IValidateData
{
    void Validate();
}

public abstract class BaseValidator:IValidateData
{
    public void DefaultValidation()
    { 
    }

    void Validate()
    {
       //Custom validation here
    }
 }

 public class ConcreteValidator:BaseValidator
 {
    override void Validate()
    {
        this.DefaultValidation();

        //my custom validation here
    }
 }

As I said, that's only a very rough sketch of how you might approach an interface-based structure with an abstract class to provide a base implementation from which other concrete classes could be derived. The details here might not be perfect, but I think the concept is at least there.

Hope that's helpful in some way.

Answer 5

I was inspired by @theringostars's suggestion to use to dictionary of actions to clean the validate function . Here is the completed sample. This is a community wiki, so feel free to edit.

using System;
using System.Reflection;
using System.Collections.Generic;

namespace ValidatorPattern
{
    //classfication of you custom validations.
    enum ValidatorType
    {
        CustomValidation1,
        CustomValidation2
    }

    //attribute applied to all customvalidation methods.
    class ValidatorAttribute : Attribute
    {
        public ValidatorType ValidatorType { get; set; }

        public ValidatorAttribute(ValidatorType validatorType)
        {
            ValidatorType = validatorType;
        }
    }

    abstract class BaseValidator
    {
        //key:List of custom validation Actions to be performed
    protected List<Action<string>> customValidations = new List<Action<string>>();

    public BaseValidator()
    {
        InitializeValidators();
    }

    public virtual void Validate()
    {
        Console.WriteLine("BaseValidator was called");
    }

        private void InitializeValidators()
        {
            Type emailValidator = this.GetType();
        foreach (MethodInfo emailValidatorTypeMethod in emailValidator.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance))
        {
            foreach (Attribute a in Attribute.GetCustomAttributes(emailValidatorTypeMethod))
            {
                if (a.GetType() == typeof(ValidatorAttribute))
                    customValidations.Add((Action<string>)emailValidatorTypeMethod.Invoke(this, null));
            }
        }
        }

    }

    class EmailValidator : BaseValidator
    {
        //Comment: Validate() method looks clean. No matter how many custom validation logics are added.
        public override void Validate()
        {
            Console.WriteLine("EmailValidator was called");
            base.Validate();//call base validations

            //loops thru all validators marked with 'ValidatorAttribute' in the current class.
            foreach (KeyValuePair<Tuple<ValidatorType, string>, Action<string>> validation in customValidations)
            {
                validation.Value("email string");
            }
        }

        [Validator(ValidatorType.CustomValidation1)]
        private Action<string> CustomValidation1()
        {
            Action<string> customValiation1 = (x) => { Console.WriteLine("CustomValidation1 was called"); };
            return customValiation1;
        }

        [Validator(ValidatorType.CustomValidation2)]
        private Action<string> CustomValidation2()
        {
            Action<string> customValiation2 = (x) => { Console.WriteLine("CustomValidation2 was called"); };
            return customValiation2;
        }
    }

    class Client
    {
        static void Main()
        {
            BaseValidator emailValidator = new EmailValidator();
            emailValidator.Validate();

        }
    }
}

PS: Also, explore Microsoft's entlib standard as metioned by @jordao