Identical classes with different number of type parameters

Question

I'm writing a validation class, and I need one version which takes one type parameter, and one which takes two, and who knows one day I might want one which takes three. I have a lot of repeated code as the two versions are effectively identical except for the different number of type parameters. I'm looking for suggestions about how (if possible) to refactor this to reduce the amount of repeated code:

public class Validator<T> : Validator
{
    // Because this is a static property, the rules are only evaluated once per type
    // no matter how many instances are created.
    private static IEnumerable<IValidate<T>> rules;

    public IEnumerable<IValidate<T>> Validate(T item)
    {
        if (rules == null || rules.Any() == false)
        {
            CreateRules();
        }

        return rules.Where(rule => rule.IsValid(item) == false);
    }

    private static IValidate<T> CreateRule(Type type)
    {
        return (IValidate<T>)Activator.CreateInstance(type);
    }

    protected virtual void CreateRules()
    {
        IEnumerable<Type> ruleTypes = GetTypes<T>(t => typeof(IValidate<T>).IsAssignableFrom(t));
        rules = ruleTypes.Select(CreateRule);
    }
}

public class Validator<T, K> : Validator
{
    // Because this is a static property, the rules are only evaluated once per type
    // no matter how many instances are created.
    private static IEnumerable<IValidate<T, K>> rules;

    public IEnumerable<IValidate<T, K>> Validate(T item, K itemToCompare)
    {
        if (rules == null || rules.Any() == false)
        {
            CreateRules();
        }

        return rules.Where(rule => rule.IsValid(item, itemToCompare) == false);
    }

    private static IValidate<T, K> CreateRule(Type type)
    {
        return (IValidate<T, K>)Activator.CreateInstance(type);
    }

    protected virtual void CreateRules()
    {
        IEnumerable<Type> ruleTypes = GetTypes<T>(t => typeof(IValidate<T, K>).IsAssignableFrom(t));
        rules = ruleTypes.Select(CreateRule);
    }
}

public abstract class Validator
{
    protected static IEnumerable<Type> GetTypes<T>(Func<Type, bool> predicate)
    {
        return Assembly.GetAssembly(typeof(T)).GetTypes().Where(predicate);
    }
}

Edit:

As the validator will be used many times to validate the same type of item, it's important that it only creates the rules once using Reflection, and that this unit test passes:

public class Validator_Tests
{
    [Test]
    public void Validator_Only_Creates_Rules_Once_Per_Type()
    {
        // Running this test in isolation will pass, but running it as part of a test suite can fail.
        var validator = new TestClass1_Validator();
        validator.Validate(new TestClass1());

        Assert.AreEqual(1, validator.rulesCreated);

        validator = new TestClass1_Validator();
        validator.Validate(new TestClass1());

        Assert.AreEqual(0, validator.rulesCreated);

        var validator2 = new TestClass1_Validator();
        validator2.Validate(new TestClass1());

        Assert.AreEqual(0, validator2.rulesCreated);

        var validator3 = new TestClass2_Validator();
        validator3.Validate(new TestClass2());

        Assert.AreEqual(1, validator3.rulesCreated);

        validator3 = new TestClass2_Validator();
        validator3.Validate(new TestClass2());

        Assert.AreEqual(0, validator3.rulesCreated);
    }

    private class TestClass1 { }

    private class TestClass2 { }

    private class TestClass1_Validator : Validator<TestClass1>
    {
        public int rulesCreated;

        protected override void CreateRules()
        {
            base.CreateRules();
            rulesCreated++;
        }
    }

    private class TestClass2_Validator : Validator<TestClass2>
    {
        public int rulesCreated;

        protected override void CreateRules()
        {
            base.CreateRules();
            rulesCreated++;
        }
    }

    private class A_Rule_Must_Exist_Using_TestClass1 : IValidate<TestClass1>
    {
        public bool IsValid(TestClass1 item) { return true; }

        public string ErrorMessage { get; set; }
    }

    private class A_Rule_Must_Exist_Using_TestClass2 : IValidate<TestClass2>
    {
        public bool IsValid(TestClass2 item) { return true; }

        public string ErrorMessage { get; set; }
    }
}

Answer 1

The biggest difference between the two classes is whether they use IValidate<T> or IValidate<T, K>. So, you want to create a base class that can use either of those types in a type-safe manner. You can do that by making the base class also generic, so your derived class will be for example class Validator<T, K> : ValidatorBase<IValidate<T, K>>.

Then you can refactor the classes to move most methods to the base class and keep only the one that actually differs: Validate(). Also, you will need some way to get the “main” type in the base class, for use in CreateRules(). You could use reflection to do that (assuming all IValidate types follow the same convention), but I think a better way is to use an abstract method or property.

The rewritten code could the look something like:

public class Validator<TTarget> : ValidatorBase<IValidate<TTarget>>
{
    public IEnumerable<IValidate<TTarget>> Validate(TTarget item)
    {
        CreateRulesIfNecessary();

        return Rules.Where(rule => !rule.IsValid(item));
    }

    protected override Type TargetType
    {
        get { return typeof(TTarget); }
    }
}

public class Validator<TTarget, TCompared> : ValidatorBase<IValidate<TTarget, TCompared>>
{
    public IEnumerable<IValidate<TTarget, TCompared>> Validate(TTarget item, TCompared itemToCompare)
    {
        CreateRulesIfNecessary();

        return Rules.Where(rule => !rule.IsValid(item, itemToCompare));
    }

    protected override Type TargetType
    {
        get { return typeof(TTarget); }
    }
}

public abstract class ValidatorBase<TValidate>
{
    // Because this is a static property, the rules are only evaluated once per type
    // no matter how many instances are created.
    protected static IEnumerable<TValidate> Rules { get; set; }

    protected abstract Type TargetType { get; }

    protected virtual void CreateRulesIfNecessary()
    {
        if (Rules == null || !Rules.Any())
        {
            var ruleTypes = Assembly.GetAssembly(TargetType).GetTypes()
                .Where(t => typeof(TValidate).IsAssignableFrom(t));
            Rules = ruleTypes.Select(CreateRule).ToArray();
        }
    }

    private static TValidate CreateRule(Type type)
    {
        return (TValidate)Activator.CreateInstance(type);
    }
}

Other notes:

• In your code, rules wasn't a property, so your comment was inaccurate and possibly confusing.
• Instead of == false, you can simply use !.
• Most of the time, if you have a generic method that doesn't use its type argument in its signature (like your GetTypes<T>()), you can replace it with a normal parameter of type Type, making the method usable in more cases.
• If CreateRules() will always be used the same way (enclosed with the check), you can include that in the method too, possibly renaming it reflect the change.
• The names of type parameters should follow the same rules as other names: it should be clear what they mean, so don't use single letter names unless it's clear what you mean.
• If you use the result of LINQ query directly, the whole query will be evaluated every time you use its result. You can use ToArray() (or ToList()) to evaluate it just once.