1
\$\begingroup\$

I have a custom system for data validation; it handles anything from method parameters, return values, objects, etc. The core of this system is IValidationRule and its generic twin. There is a base class for actual implementations of validation rules (things like string length, database entity exists,...).

Data can have multiple validation rules bound to them, and they are mostly specified via attributes. For brevity, I will omit the attributes, rules, and params definitions as I do not think they are necessary and would only make this more complicated.

public interface IValidationRule {
    bool Validate( object? value, object? @params, object? context );
}

public interface IValidationRule<in TData, in TParams, in TContext> : IValidationRule {
    bool Validate( TData value, TParams @params, TContext context );
}

public abstract class ValidationRule<TData, TParams, TContext> : IValidationRule<TData, TParams, TContext> {
    bool IValidationRule.Validate( object? value, object? @params, object? context ) {
#pragma warning disable CS8601 // Possible null reference assignment.
        return Validate((TData)value, (TParams)@params, (TContext)context);
#pragma warning restore CS8601 // Possible null reference assignment.
    }

    public abstract bool Validate( TData value, TParams @params, TContext context );
}

There is a couple of things I'd like to consult.

  1. The generic interface uses the non-generic (in a similar fashion as IEnumerable<T>). I remember reading somewhere how generics made "object as jack of all trades" obsolete and the only reason why IEnumerable<T> was implemented this way was backwards compatibility. I do not think I can achieve my goal without this approach.

  2. Since the library is currently being switched to use C# 8's ref type nullability, I had to disable some warnings. On the one hand, this feels wrong, but on the other hand, the IValidationRule must use the signature with nullable objects as some rules might use nullable types. However, the generic versions will often specify non-nullable types, such as int.

  3. It cannot be seen in this example, but let's say the IValidationRule has a property of type object. Is it frowned upon to override this in the generic interface, using a generic type for the property? I think the answer is somewhat. While it is an indicator of possible code smell, some cases warrant this (mostly aggregated evaluation).

What are your thoughts?

Some additional examples:

public interface IValidationDescriptor {
    string ErrorCode { get; }
    object Params { get; }
    Type Rule { get; set; }
    ErrorSeverity Severity { get; set; }
    ValidationTargets Targets { get; set; }
    int Order { get; set; }
    ExecutionConditions Conditions { get; set; }
}

public class Validator {
    private readonly IErrorMessageProvider _messageProvider;
    private readonly IPropertyNameProvider _propertyNameProvider;
    private readonly IValidationRuleProvider _ruleProvider;

    public Validator( IValidationRuleProvider ruleProvider, IErrorMessageProvider messageProvider,
                        IPropertyNameProvider propertyNameProvider ) {
        _ruleProvider = ruleProvider;
        _messageProvider = messageProvider;
        _propertyNameProvider = propertyNameProvider;
    }

  private IEnumerable<IDataError> ValidateObject( object targetObject, Type parentType, string? path ) {
    var errors = new List<IDataError>();
    var props = targetObject.GetType().GetProperties().Where(p => p.CanRead);

    foreach( var prop in props ) {
      var propValue = prop.GetValue(targetObject);
      var propPath = CombinePaths(path, _propertyNameProvider.GetName(prop));

errors.AddRange(Validate(prop.GetCustomAttributes<ValidationDescriptorAttribute>(), prop.GetCustomAttribute<ValidationOptionsAttribute>(),
                           propValue, prop.PropertyType,
                           targetObject, parentType,
                           propPath));
    }

    return errors;
  }
  private IEnumerable<IDataError> ValidateData( IEnumerable<IValidationDescriptor> descriptors, 
                                                object? value, Type valueType, 
                                                object? parentValue, Type parentType, string? path ) {
    var errors = new List<IDataError>();

    foreach( var descriptor in descriptors.OrderBy(d => d.Order) ) {
      if( descriptor.Conditions.HasFlag(ExecutionConditions.NoError) && errors.Any() )
        continue;

      if( descriptor.Targets.HasFlag(ValidationTargets.Self) )
        ApplyDescriptor(errors, descriptor, value, valueType, parentValue, parentType, path);

      if( !descriptor.Targets.HasFlag(ValidationTargets.Children) || !(value is IEnumerable enumerable) )
        continue;

      var childType = (value as IEnumerable).AsQueryable().ElementType;
      var i = 0;
      foreach( var child in enumerable ) {
        if( descriptor.Conditions.HasFlag(ExecutionConditions.NoError) && errors.Any() )
          break;
        ApplyDescriptor(errors, descriptor,
                                child, childType,
                                value, valueType,
                                CombinePaths(path, i.ToString()));
        i++;
      }
    }

    return errors;
  }

  private void ApplyDescriptor( ICollection<IDataError> errors, IValidationDescriptor descriptor,
                                object? value, Type valueType,
                                object? parentValue, Type parentType,
                                string path ) {
    var isValid = _ruleProvider.Get(descriptor.Rule)
                               .Validate(value, descriptor.Params, parentValue);
    if( isValid )
      return;
    var @params = TypeActivator.CreateGenericInstance(
        typeof(ErrorGenerationParams<,,>),
        new[] { valueType, parentType, descriptor.Params.GetType() },
        new[] { value, parentValue, descriptor.Params, null }
    );
    var message = _messageProvider.Get( descriptor.ErrorCode,
                                            (IErrorGenerationParams)@params );
    errors.Add(new DataError(descriptor.ErrorCode, message, descriptor.Severity, path, value));
  }
 private static string CombinePaths( string path1, string path2 ) {
    if( path1 == null )
      return path2;
    if( path2 == null )
      return path1;
    return $"{path1}.{path2}";
  }
}

More implementation details:

// an example rule
public class BoolValueRule : ValidationRule<bool, IBoolValueParams, object> {
    public override bool Validate( bool value, IBoolValueParams @params, object context ) {
        return value == @params.RequiredValue;
    }
}

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.ReturnValue, AllowMultiple = true)]
public class ValidationDescriptorAttribute : Attribute, IValidationDescriptor {
    public ValidationDescriptorAttribute( string errorCode, Type rule ) {
        ErrorCode = errorCode;
        Rule = rule;
    }
    public ValidationDescriptorAttribute( string errorCode, Type rule, int order, ExecutionConditions conditions ) {
        ErrorCode = errorCode;
        Rule = rule;
        Order = order;
        Conditions = conditions;
    }

    public string ErrorCode { get; }
    public virtual object Params => new object();
    public Type Rule { get; set; }
    public ErrorSeverity Severity { get; set; } = ErrorSeverity.Error;
    public ValidationTargets Targets { get; set; } = ValidationTargets.Self;
    public int Order { get; set; }
    public ExecutionConditions Conditions { get; set; }
}

public class BoolValueAttribute : ValidationDescriptorAttribute {
    public BoolValueAttribute( string errorCode, bool requiredValue ) : base(errorCode, typeof(BoolValueRule)) {
        RequiredValue = requiredValue;
    }
    public BoolValueAttribute( string errorCode, bool requiredValue, int order, ExecutionConditions conditions )
        : base(errorCode, typeof(BoolValueRule), order, conditions) {
        RequiredValue = requiredValue;
    }
    public bool RequiredValue { get; }
    public override object Params => new BoolValueParams(RequiredValue);
}

public class BoolValueParams : IBoolValueParams {
    public BoolValueParams( bool requiredValue ) {
        RequiredValue = requiredValue;
    }
    public bool RequiredValue { get; }
}

// an object we want to validate
public class MyValidatableObject {
    [BoolValue(true)]
    public bool AcceptedToS { get; set; }
}

public enum ErrorSeverity {
    Warning,
    Error,
}

public interface IGenericError {
    string Code { get; }
    string Message { get; }
    ErrorSeverity Severity { get; }
}

public interface IDataError : IGenericError {
    string Path { get; }
    object? Value { get; }
}

[Flags]
public enum ValidationTargets {
    None = 0,
    Self = 1,
    Children = 2,
    All = Self | Children
}

public enum TraversalMode {
    Recursive,
    RecursiveOnNoErrors,
    Stop
}

[Flags]
public enum ExecutionConditions {
    None = 0,
    NoError = 1
}

public interface IErrorMessageProvider {
    string Get( string code, IErrorGenerationParams? @params );
}

public interface IPropertyNameProvider {
    string GetName( PropertyInfo property );
}

public interface IValidationRuleProvider {
    IValidationRule Get( Type type );
}
\$\endgroup\$
7
  • 4
    \$\begingroup\$ "I do not think I can achieve my goal without this approach." I think we'd need to see some actual usage of the interface to comment on whether that usage is achievable with a purely genericised interface. \$\endgroup\$ Oct 4, 2019 at 8:38
  • \$\begingroup\$ @PeterTaylor I added a simplified implementation of the Validator class. Is this sufficient? \$\endgroup\$
    – pikausp
    Oct 4, 2019 at 9:13
  • 2
    \$\begingroup\$ Yes, I think it would be benefical to post all the code or at least enough so that the example makes sense. \$\endgroup\$
    – t3chb0t
    Oct 4, 2019 at 14:01
  • 4
    \$\begingroup\$ Keeping the code concise is not as important on Code Review as on Stack Overflow. Code review needs to see all the related code to provide a good review. Stack Overflow wants a concise block of code and/or description to provide an answer to a how to question. \$\endgroup\$
    – pacmaninbw
    Oct 5, 2019 at 14:44
  • 1
    \$\begingroup\$ I added more interface/type definitions and implementation details; I apologize for the delay. \$\endgroup\$
    – pikausp
    Oct 8, 2019 at 15:07

0

Your Answer

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

Browse other questions tagged or ask your own question.