Validating proper input

Question

Verifying the user's input is almost always required, even in really simple apps such as console calculator. Due to the wide variety of scenarios where this is useful I decided to make a few classes that will make the process easier.

I currently have 2 classes for validation, one for input that will require parsing and one for input that is already parsed to the specified type. They both inherit a common interface:

public interface IInputValidator<T> 
{
    ValidationResult<T> Validate();
}

Where the ValidationResult class is implemented as follows:

public class ValidationResult<T>
{
    public bool Success { get; }
    public T Value { get; }

    public ValidationResult(bool success, T value)
    {
        Success = success;
        Value = value;
    }

    public ValidationResult(bool success) : this(success, default(T))
    {
    }
}

The InputValidatorUnparsed<TSource, TValue> class which deals with input that needs to be parsed before operated on:

public class InputValidatorUnparsed<TSource, TValue> : IInputValidator<TValue>
{
    public delegate bool InputTryParse(TSource input, out TValue value);

    private readonly InputTryParse inputTryParse;

    private readonly Func<TSource> _getUnparsedValue;
    private Action _onFailedAction;
    private IEnumerable<TValue> _allowedItems = Enumerable.Empty<TValue>();
    private IEqualityComparer<TValue> _comparer = EqualityComparer<TValue>.Default;

    public InputValidatorUnparsed(Func<TSource> getUnparsedValue, InputTryParse tryParse)
    {
        inputTryParse = tryParse ?? throw new ArgumentNullException(nameof(tryParse));
        _getUnparsedValue = getUnparsedValue;
    }

    public InputValidatorUnparsed<TSource, TValue> WithFailedAction(Action onFailedAction)
    {
        _onFailedAction = onFailedAction;
        return this;
    }

    public InputValidatorUnparsed<TSource, TValue> WithAllowedItems(IEnumerable<TValue> allowedItems)
    {
        return WithAllowedItems(allowedItems, _comparer);
    }

    public InputValidatorUnparsed<TSource, TValue> WithAllowedItems(IEnumerable<TValue> allowedItems,
        IEqualityComparer<TValue> comparer)
    {
        _allowedItems = allowedItems ?? throw new ArgumentNullException(nameof(allowedItems));
        _comparer = comparer ?? throw new ArgumentNullException(nameof(comparer));
        return this;
    }

    public ValidationResult<TValue> Validate()
    {
        var parsingSuccess = inputTryParse.Invoke(_getUnparsedValue.Invoke(), out TValue value);
        if (parsingSuccess && IsAllowedItem(value))
        {
            return new ValidationResult<TValue>(true, value);           
        }
        _onFailedAction?.Invoke();
        return new ValidationResult<TValue>(false, value);
    }

    private bool IsAllowedItem(TValue item)
    {
        return !_allowedItems.Any() || _allowedItems.Any(i => _comparer.Equals(i, item));
    }
}

Example usage:

var validator = new InputValidatorUnparsed<string, int>(Console.ReadLine, int.TryParse)
    .WithFailedAction(() => Console.WriteLine("Invalid input please try again."))
    .WithAllowedItems(Enumerable.Range(1, 10));
var result = validator.Validate();
while (!result.Success)
{
    result = validator.Validate();
}
Console.WriteLine($"Correct input = {result.Value}");

The InputValidatorParsed<TValue> which deals with input that wont required parsing to be operated on:

public class InputValidatorParsed<TValue> : IInputValidator<TValue>
{
    private readonly Predicate<TValue> validator;
    private readonly Func<TValue> getInputValue;
    private Action _onFailedAction;
    private IEnumerable<TValue> _allowedItems = Enumerable.Empty<TValue>();
    private IEqualityComparer<TValue> _comparer = EqualityComparer<TValue>.Default;

    public InputValidatorParsed(Func<TValue> getValue, Predicate<TValue> validator)
    {
        getInputValue = getValue ?? throw new ArgumentNullException(nameof(getValue));
        this.validator = validator;
    }

    public InputValidatorParsed(Func<TValue> getValue) : this(getValue, null)
    {
    }

    public InputValidatorParsed<TValue> WithFailedAction(Action onFailedAction)
    {
        _onFailedAction = onFailedAction;
        return this;
    }

    public InputValidatorParsed<TValue> WithAllowedItems(IEnumerable<TValue> allowedItems)
    {
        return WithAllowedItems(allowedItems, _comparer);
    }

    public InputValidatorParsed<TValue> WithAllowedItems(IEnumerable<TValue> allowedItems,
        IEqualityComparer<TValue> comparer)
    {
        _allowedItems = allowedItems ?? throw new ArgumentNullException(nameof(allowedItems));
        _comparer = comparer ?? throw new ArgumentNullException(nameof(comparer));
        return this;
    }

    public ValidationResult<TValue> Validate()
    {
        var value = getInputValue.Invoke();
        if (validator == null)
        {
            return new ValidationResult<TValue>(IsAllowedItem(value), value);
        }
        if (validator.Invoke(value) && IsAllowedItem(value))
        {
            return new ValidationResult<TValue>(true, value);
        }

        _onFailedAction?.Invoke();
        return new ValidationResult<TValue>(false, value);
    }

    private bool IsAllowedItem(TValue item)
    {
        return !_allowedItems.Any() || _allowedItems.Any(i => _comparer.Equals(i, item));
    }
}

Example usage:

var validator = new InputValidatorParsed<string>(Console.ReadLine, t => !string.IsNullOrEmpty(t))
    .WithAllowedItems(new []{"value"})
    .WithFailedAction(() => Console.WriteLine("Invalid input please try again."));
var result = validator.Validate();
while (!result.Success)
{
    result = validator.Validate();
}
Console.WriteLine($"Correct input = {result.Value}");

Feel free to comment on anything, but I have few concerns in mind:

1. I'm not happy with the naming of the classes.

2. I do like the initialization of such object but I don't like the usage of it. Those classes will mostly be used in while loops I can imagine and the syntax for that isn't really pretty if you want to obtain the value of the result.

3. There is repetition in the classes that maybe an abstract class can solve in some way but I don't think it's appropriate in this case as it will either look redundant or it will be way too restrictive for the derived classes.

Answer 1

I don't think there should be two validators as they are nearly identical. The only difference between them is the parsing part. They are validators so they should get a value ready for validation and not try to parse anything. It's the responsibility of a parser to know how to convert/parse one type into another one.

The validators also mix the builder pattern with a normal object. Methods like WithAllowedItems should be only used by a builder and not the actual object that should either have properties for that values or be immutable and require all parameters via a constructor.

I find that the semi-builder pattern makes it confusing to use because at the end I expect to call ToInputValidator or Build which are not there because I'm constructing the final object with the WithX methods. What makes it even more confusing is that some arguments are already required by the constructor which means that the WithX parameters are optional and could be simply properties which are (at least to me) more natural to initialize with an object initializer then by calling methods as if it was a builder.

What I expect is either this API

var validator = 
    InputValidator<string>
        .Builder
        .Condition(Console.ReadLine, t => !string.IsNullOrEmpty(t))
        .WithAllowedItems(new []{"value"})
        .WithFailedAction(() => Console.WriteLine("Invalid input please try again."))
        .Build(); // Throws InvalidOperationException if Condition not specified.

or that one

var validator = new InputValidator<string>(Console.ReadLine, t => !string.IsNullOrEmpty(t))
{
    AllowedItems = new []{"value"},
    FailedAction = () => Console.WriteLine("Invalid input please try again.")
};