Simple fluent library to validate text content











up vote
3
down vote

favorite
1












Before I can perform a more or less complex mappings, treatments in business objects and saving results in databases, I often need to validate contents from flat text files or excel files



All the validation libraries I know are designed to validate the content of strongly typed objects. The parsers or readers I tried are really good to load or write data but in case of an error in file, they are design for logging purpose and not for the end user. In addition, I'd like to use a library to do the validation only and be able to change the parser/reader if necessary. My goal is to validate the content of theses files and provide maximun feedbacks to the end users as if it was a form validation in web page.



Instead of wrting code that for sure no one will ever review, I decided to spend the day trying to create a library as an exercice and publish it on GitHub for differents reasons : try to write a fluent API, learn to use GitHub and git more accurately, create unit tests with xUnit and use a CI to automate test project (Travis, AppVeyor...), learn how to create a Nuget package and now try to collect some comments, ideas, remarks...and maybe use the library



For the time being, I end up with something like this :



I define a class with string properties that represent the columns of the files (I could also use a dynamic object) and validate with :



public class Row
{
public string Key { get; set; }
public string DecimalValue { get; set; }
public string DateTimeValue { get; set; }
}

// ...

ClassValidator<Row> validator = ClassValidator<Row>
.Init()
.AddProperty(PropertyValidator<Row>.For(x => x.Key).IsNotNull().HasLength(5,10))
.AddProperty(PropertyValidator<Row>.For(x => x.DateTimeValue).IsNotNull().IsDateTime("yyyyMMdd"))
.AddProperty(PropertyValidator<Row>.For(x => x.DecimalValue).IsNotNull().IsDecimal())
.Validate(new Row()
{
Key = "thiskey",
DateTimeValue = "20181201",
DecimalValue = "123,45"
});


The ClassValidator contains N PropertyValidator for the N properties to validate. Each PropertyValidator can contains N MethodValidator to do the validation (IsNull, TryDecimal, TryRegex ...). Most of the methods are defined as Func, Action delegates executed only when the Validate method is called.



The ClassValidator constructor is private and the Init method can receive some options. The Validate method loop over the List<PropertyValidator<TRow>> populated with the AddProperty method



/// <summary>
/// Validation at class level.
/// </summary>
/// <typeparam name="TRow">The type of the model.</typeparam>
public class ClassValidator<TRow>
{
private readonly IClassValidatorOptions options;

private readonly List<PropertyValidator<TRow>> listValidator = new List<PropertyValidator<TRow>>();

/// <summary>
/// Initializes a new instance of the <see cref="ClassValidator{TRow}"/> class.
/// Private constructor.
/// </summary>
private ClassValidator()
{
}

private ClassValidator(IClassValidatorOptions options)
{
this.options = options;
}

/// <summary>
/// Gets validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successful or not.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Create an instance with no option.
/// </summary>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init()
{
return new ClassValidator<TRow>();
}

/// <summary>
/// Create an instance with option.
/// </summary>
/// <param name="options">options.</param>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init(IClassValidatorOptions options)
{
return new ClassValidator<TRow>(options);
}

/// <summary>
/// Add a property validator. <see cref="PropertyValidator{TRow}"/>.
/// </summary>
/// <param name="propValidator">Targeted property validator.</param>
/// <returns>current instance.</returns>
public ClassValidator<TRow> AddProperty(PropertyValidator<TRow> propValidator)
{
this.listValidator.Add(propValidator);
return this;
}

/// <summary>
/// Validation for one row.
/// </summary>
/// <param name="row">Row to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> Validate(TRow row)
{
foreach (var validator in this.listValidator)
{
validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

return this;
}

/// <summary>
/// Validation for rows collection.
/// </summary>
/// <param name="rows">rows collection to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> ValidateList(IEnumerable<TRow> rows)
{
int index = 0;

foreach (var row in rows)
{
foreach (var validator in this.listValidator)
{
if (this.options != null && this.options.ShowRowIndex)
{
validator.SetRowIndex(index);
}

validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

index++;
}

return this;
}
}



  • If the generic type is set on the top class ClassValidator <Row>.
    Ideally, I would like to set the type once and reuse the same type
    for PropertyValidator. In the sample, I have to define the type for
    ClassValidator and for eachPropertyValidator, which could lead
    to some inconsistency.



The PropertyValidator exposes all the methods to do the validation of the property. Each validation method (eg : TryParseDecimal ) adds a MethodValidator in the Collection>.



/// <summary>
/// Define a validator for a property in <see cref="PropertyValidator{TRow}"/> type.
/// </summary>
/// <typeparam name="TRow">Type for this validator.</typeparam>
public class PropertyValidator<TRow> : IPropertyValidatorAction<TRow>
{
private readonly Collection<MethodValidator<TRow>> methods = new Collection<MethodValidator<TRow>>();
private int? rowIndex;
private string fieldName;
private Func<TRow, string> getter;
private Expression getterExpression;
private dynamic extraObject;
private bool preserveErrorHeader = true;

private PropertyValidator()
{
}

/// <summary>
/// Gets the list of validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successfull.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Creates a property validator for the given property.
/// </summary>
/// <param name="getterExpression">The targeted property.</param>
/// <param name="overrideFieldName">To override field Name. By default uses the name of property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<TRow> For(Expression<Func<TRow, string>> getterExpression, string overrideFieldName = null)
{
PropertyValidator<TRow> prop = new PropertyValidator<TRow>();
prop.getter = getterExpression.Compile();
prop.getterExpression = getterExpression;
prop.fieldName = overrideFieldName != null ? overrideFieldName : ExpressionUtiities.PropertyName(getterExpression);
return prop;
}

/// <summary>
/// Creates a property validator for a dynamic property.
/// </summary>
/// <param name="getter">The targeted dynamic property.</param>
/// <param name="fieldName">Fieldname for the dynamic property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<dynamic> ForDynamic(Func<dynamic, string> getter, string fieldName)
{
PropertyValidator<dynamic> prop = new PropertyValidator<dynamic>();
prop.getter = getter;
prop.fieldName = fieldName;
return prop;
}

/// <summary>
/// Allow to set a row index. Usefull to display a line number in error message.
/// </summary>
/// <param name="rowIndex">Row index.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetRowIndex(int rowIndex)
{
this.rowIndex = rowIndex;
return this;
}

/// <summary>
/// Allow to set an object to pass an extra object to the validator.
/// </summary>
/// <param name="extraObject">Extra dynamic object.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetExtraObject(dynamic extraObject)
{
this.extraObject = extraObject;
return this;
}

/// <summary>
/// Allow overriding default error message.
/// </summary>
/// <param name="msgErrorFunc">Custom error message as a Func.</param>
/// <param name="preserveErrorHeader">Preserve line, field name header in error message.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> OverrideErrorMessage(Func<TRow, string> msgErrorFunc, bool preserveErrorHeader = false)
{
this.preserveErrorHeader = preserveErrorHeader;
this.methods.Last().OverrideErrorMessage = (current) => msgErrorFunc(current);
return this;
}

/// <summary>
/// Check if property is not null.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNull()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNull();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is not null or empty.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNullOrEmpty()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNullOrEmpty();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to decimal.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDecimal()
{
DecimalMethods<TRow> method = new DecimalMethods<TRow>((x) => this.getter(x));
method.TryParseDecimal();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to DateTime.
/// </summary>
/// <param name="format">specified format.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDateTime(string format)
{
DateTimeMethods<TRow> method = new DateTimeMethods<TRow>((x) => this.getter(x));
method.TryParseDateTime(format);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property has required length.
/// </summary>
/// <param name="min">min length.</param>
/// <param name="max">maw length.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> HasLength(int min, int max)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.HasLength(min, max);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is in list values.
/// </summary>
/// <param name="values">List of values.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsStringValues(string values)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsStringValues(values);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property value match regex.
/// </summary>
/// <param name="pattern">Regex pattern.</param>
/// <param name="options">Regex options.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryRegex(string pattern, RegexOptions options = RegexOptions.None)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.TryRegex(pattern, options);
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a validation action of <see cref="IPropertyValidatorAction"/>.
/// </summary>
/// <param name="ifFunc">Condition as a predicate.</param>
/// <param name="action">
/// Validation action from <see cref="IPropertyValidatorAction{TRow}"/> interface.
/// Action may add 1 to N methods.
/// </param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> ifFunc, Action<IPropertyValidatorAction<TRow>> action)
{
// track method count before and after to link condition on all new methods.
var before = this.methods.Count;
action(this);
var after = this.methods.Count;
var nbAdded = after - before;

// take last N methods
var newMethods = this.methods.Skip(Math.Max(0, after - nbAdded));

// add condition on new methods
foreach (var item in newMethods)
{
item.Condition = (current) => ifFunc(current);
}

return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">tocheck if condition is ok. TRow as the current row.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">todo if condition is ok.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, dynamic, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current, this.extraObject);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Validate the property with the current row.
/// </summary>
/// <param name="row">Current row.</param>
public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName, this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}

/// <summary>
/// Create an error message.
/// </summary>
/// <param name="current">Current element.</param>
/// <param name="messageError">Error informations from method. <see cref="IMethodMessageError{TRow}"/>.</param>
/// <returns>Error message.</returns>
private string MessageErrorFactory(TRow current, IMethodMessageError<TRow> messageError)
{
StringBuilder errorMsg = new StringBuilder();

// when overriden error message suppress header by default
if (this.preserveErrorHeader)
{
if (this.rowIndex.HasValue)
{
errorMsg.Append($"{Translation.Row} {this.rowIndex} ");
}

errorMsg.Append($"{Translation.Field} {this.fieldName} : ");
}

// default or override msg
if (messageError.OverrideErrorMessage == null)
{
errorMsg.Append(messageError.ErrorMessage(current));
}
else
{
errorMsg.Append(messageError.OverrideErrorMessage(current));
}

return errorMsg.ToString();
}
}



  • The If methods should look for the last method added to the
    collection. I do not like this way of adding the condition on the
    MethodValidtor as it does not seem robust and reliable


The MethodValidator defines delegates to get the value, the method for validation and and optional condition.



The concrete implemenation is done in sub classes of the MethodValidator.



/// <summary>
/// Define a method to validate a property.
/// </summary>
/// <typeparam name="TRow">Type to validate.</typeparam>
public class MethodValidator<TRow> : IMethodMessageError<TRow>
{
/// <summary>
/// Initializes a new instance of the <see cref="MethodValidator{TRow}"/> class.
/// </summary>
/// <param name="value">delegate to get the value to validate.</param>
public MethodValidator(Func<TRow, string> value)
{
this.Value = value;
}

/// <summary>
/// Gets a method returning the value to validate.
/// </summary>
public Func<TRow, string> Value { get; private set; }

/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }

/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }

/// <summary>
/// Gets or sets a method to define a custom error message.
/// <summary>
public Func<TRow, string> OverrideErrorMessage { get; set; }

/// <summary>
/// Gets or sets a method to define the final error message.
/// <summary>
public Func<TRow, string> ErrorMessage { get; set; }
}


The whole source code is here : https://github.com/NicoDvl/StringContentValidator



What do you think of that ? How to improve the design, the source code ? Could you find the library useful ? What am I missing ?










share|improve this question









New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




















  • Welcome to Code Review! The code that you have posted in the question is just an example usage, rather than the implementation of the validator. Therefore, I am voting to close this question due to the code not being included.
    – 200_success
    2 days ago










  • @200_success I'll include more parts of the implementation. I did not want it to be too long.
    – NicoD
    2 days ago










  • @200_success I added parts of the implementation and precised some questions. Do not hesitate to tell me if it's better or not. Regards.
    – NicoD
    2 days ago










  • Thanks for adding the code. It's much better now. Close vote retracted.
    – 200_success
    yesterday















up vote
3
down vote

favorite
1












Before I can perform a more or less complex mappings, treatments in business objects and saving results in databases, I often need to validate contents from flat text files or excel files



All the validation libraries I know are designed to validate the content of strongly typed objects. The parsers or readers I tried are really good to load or write data but in case of an error in file, they are design for logging purpose and not for the end user. In addition, I'd like to use a library to do the validation only and be able to change the parser/reader if necessary. My goal is to validate the content of theses files and provide maximun feedbacks to the end users as if it was a form validation in web page.



Instead of wrting code that for sure no one will ever review, I decided to spend the day trying to create a library as an exercice and publish it on GitHub for differents reasons : try to write a fluent API, learn to use GitHub and git more accurately, create unit tests with xUnit and use a CI to automate test project (Travis, AppVeyor...), learn how to create a Nuget package and now try to collect some comments, ideas, remarks...and maybe use the library



For the time being, I end up with something like this :



I define a class with string properties that represent the columns of the files (I could also use a dynamic object) and validate with :



public class Row
{
public string Key { get; set; }
public string DecimalValue { get; set; }
public string DateTimeValue { get; set; }
}

// ...

ClassValidator<Row> validator = ClassValidator<Row>
.Init()
.AddProperty(PropertyValidator<Row>.For(x => x.Key).IsNotNull().HasLength(5,10))
.AddProperty(PropertyValidator<Row>.For(x => x.DateTimeValue).IsNotNull().IsDateTime("yyyyMMdd"))
.AddProperty(PropertyValidator<Row>.For(x => x.DecimalValue).IsNotNull().IsDecimal())
.Validate(new Row()
{
Key = "thiskey",
DateTimeValue = "20181201",
DecimalValue = "123,45"
});


The ClassValidator contains N PropertyValidator for the N properties to validate. Each PropertyValidator can contains N MethodValidator to do the validation (IsNull, TryDecimal, TryRegex ...). Most of the methods are defined as Func, Action delegates executed only when the Validate method is called.



The ClassValidator constructor is private and the Init method can receive some options. The Validate method loop over the List<PropertyValidator<TRow>> populated with the AddProperty method



/// <summary>
/// Validation at class level.
/// </summary>
/// <typeparam name="TRow">The type of the model.</typeparam>
public class ClassValidator<TRow>
{
private readonly IClassValidatorOptions options;

private readonly List<PropertyValidator<TRow>> listValidator = new List<PropertyValidator<TRow>>();

/// <summary>
/// Initializes a new instance of the <see cref="ClassValidator{TRow}"/> class.
/// Private constructor.
/// </summary>
private ClassValidator()
{
}

private ClassValidator(IClassValidatorOptions options)
{
this.options = options;
}

/// <summary>
/// Gets validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successful or not.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Create an instance with no option.
/// </summary>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init()
{
return new ClassValidator<TRow>();
}

/// <summary>
/// Create an instance with option.
/// </summary>
/// <param name="options">options.</param>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init(IClassValidatorOptions options)
{
return new ClassValidator<TRow>(options);
}

/// <summary>
/// Add a property validator. <see cref="PropertyValidator{TRow}"/>.
/// </summary>
/// <param name="propValidator">Targeted property validator.</param>
/// <returns>current instance.</returns>
public ClassValidator<TRow> AddProperty(PropertyValidator<TRow> propValidator)
{
this.listValidator.Add(propValidator);
return this;
}

/// <summary>
/// Validation for one row.
/// </summary>
/// <param name="row">Row to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> Validate(TRow row)
{
foreach (var validator in this.listValidator)
{
validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

return this;
}

/// <summary>
/// Validation for rows collection.
/// </summary>
/// <param name="rows">rows collection to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> ValidateList(IEnumerable<TRow> rows)
{
int index = 0;

foreach (var row in rows)
{
foreach (var validator in this.listValidator)
{
if (this.options != null && this.options.ShowRowIndex)
{
validator.SetRowIndex(index);
}

validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

index++;
}

return this;
}
}



  • If the generic type is set on the top class ClassValidator <Row>.
    Ideally, I would like to set the type once and reuse the same type
    for PropertyValidator. In the sample, I have to define the type for
    ClassValidator and for eachPropertyValidator, which could lead
    to some inconsistency.



The PropertyValidator exposes all the methods to do the validation of the property. Each validation method (eg : TryParseDecimal ) adds a MethodValidator in the Collection>.



/// <summary>
/// Define a validator for a property in <see cref="PropertyValidator{TRow}"/> type.
/// </summary>
/// <typeparam name="TRow">Type for this validator.</typeparam>
public class PropertyValidator<TRow> : IPropertyValidatorAction<TRow>
{
private readonly Collection<MethodValidator<TRow>> methods = new Collection<MethodValidator<TRow>>();
private int? rowIndex;
private string fieldName;
private Func<TRow, string> getter;
private Expression getterExpression;
private dynamic extraObject;
private bool preserveErrorHeader = true;

private PropertyValidator()
{
}

/// <summary>
/// Gets the list of validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successfull.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Creates a property validator for the given property.
/// </summary>
/// <param name="getterExpression">The targeted property.</param>
/// <param name="overrideFieldName">To override field Name. By default uses the name of property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<TRow> For(Expression<Func<TRow, string>> getterExpression, string overrideFieldName = null)
{
PropertyValidator<TRow> prop = new PropertyValidator<TRow>();
prop.getter = getterExpression.Compile();
prop.getterExpression = getterExpression;
prop.fieldName = overrideFieldName != null ? overrideFieldName : ExpressionUtiities.PropertyName(getterExpression);
return prop;
}

/// <summary>
/// Creates a property validator for a dynamic property.
/// </summary>
/// <param name="getter">The targeted dynamic property.</param>
/// <param name="fieldName">Fieldname for the dynamic property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<dynamic> ForDynamic(Func<dynamic, string> getter, string fieldName)
{
PropertyValidator<dynamic> prop = new PropertyValidator<dynamic>();
prop.getter = getter;
prop.fieldName = fieldName;
return prop;
}

/// <summary>
/// Allow to set a row index. Usefull to display a line number in error message.
/// </summary>
/// <param name="rowIndex">Row index.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetRowIndex(int rowIndex)
{
this.rowIndex = rowIndex;
return this;
}

/// <summary>
/// Allow to set an object to pass an extra object to the validator.
/// </summary>
/// <param name="extraObject">Extra dynamic object.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetExtraObject(dynamic extraObject)
{
this.extraObject = extraObject;
return this;
}

/// <summary>
/// Allow overriding default error message.
/// </summary>
/// <param name="msgErrorFunc">Custom error message as a Func.</param>
/// <param name="preserveErrorHeader">Preserve line, field name header in error message.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> OverrideErrorMessage(Func<TRow, string> msgErrorFunc, bool preserveErrorHeader = false)
{
this.preserveErrorHeader = preserveErrorHeader;
this.methods.Last().OverrideErrorMessage = (current) => msgErrorFunc(current);
return this;
}

/// <summary>
/// Check if property is not null.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNull()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNull();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is not null or empty.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNullOrEmpty()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNullOrEmpty();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to decimal.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDecimal()
{
DecimalMethods<TRow> method = new DecimalMethods<TRow>((x) => this.getter(x));
method.TryParseDecimal();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to DateTime.
/// </summary>
/// <param name="format">specified format.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDateTime(string format)
{
DateTimeMethods<TRow> method = new DateTimeMethods<TRow>((x) => this.getter(x));
method.TryParseDateTime(format);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property has required length.
/// </summary>
/// <param name="min">min length.</param>
/// <param name="max">maw length.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> HasLength(int min, int max)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.HasLength(min, max);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is in list values.
/// </summary>
/// <param name="values">List of values.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsStringValues(string values)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsStringValues(values);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property value match regex.
/// </summary>
/// <param name="pattern">Regex pattern.</param>
/// <param name="options">Regex options.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryRegex(string pattern, RegexOptions options = RegexOptions.None)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.TryRegex(pattern, options);
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a validation action of <see cref="IPropertyValidatorAction"/>.
/// </summary>
/// <param name="ifFunc">Condition as a predicate.</param>
/// <param name="action">
/// Validation action from <see cref="IPropertyValidatorAction{TRow}"/> interface.
/// Action may add 1 to N methods.
/// </param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> ifFunc, Action<IPropertyValidatorAction<TRow>> action)
{
// track method count before and after to link condition on all new methods.
var before = this.methods.Count;
action(this);
var after = this.methods.Count;
var nbAdded = after - before;

// take last N methods
var newMethods = this.methods.Skip(Math.Max(0, after - nbAdded));

// add condition on new methods
foreach (var item in newMethods)
{
item.Condition = (current) => ifFunc(current);
}

return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">tocheck if condition is ok. TRow as the current row.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">todo if condition is ok.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, dynamic, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current, this.extraObject);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Validate the property with the current row.
/// </summary>
/// <param name="row">Current row.</param>
public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName, this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}

/// <summary>
/// Create an error message.
/// </summary>
/// <param name="current">Current element.</param>
/// <param name="messageError">Error informations from method. <see cref="IMethodMessageError{TRow}"/>.</param>
/// <returns>Error message.</returns>
private string MessageErrorFactory(TRow current, IMethodMessageError<TRow> messageError)
{
StringBuilder errorMsg = new StringBuilder();

// when overriden error message suppress header by default
if (this.preserveErrorHeader)
{
if (this.rowIndex.HasValue)
{
errorMsg.Append($"{Translation.Row} {this.rowIndex} ");
}

errorMsg.Append($"{Translation.Field} {this.fieldName} : ");
}

// default or override msg
if (messageError.OverrideErrorMessage == null)
{
errorMsg.Append(messageError.ErrorMessage(current));
}
else
{
errorMsg.Append(messageError.OverrideErrorMessage(current));
}

return errorMsg.ToString();
}
}



  • The If methods should look for the last method added to the
    collection. I do not like this way of adding the condition on the
    MethodValidtor as it does not seem robust and reliable


The MethodValidator defines delegates to get the value, the method for validation and and optional condition.



The concrete implemenation is done in sub classes of the MethodValidator.



/// <summary>
/// Define a method to validate a property.
/// </summary>
/// <typeparam name="TRow">Type to validate.</typeparam>
public class MethodValidator<TRow> : IMethodMessageError<TRow>
{
/// <summary>
/// Initializes a new instance of the <see cref="MethodValidator{TRow}"/> class.
/// </summary>
/// <param name="value">delegate to get the value to validate.</param>
public MethodValidator(Func<TRow, string> value)
{
this.Value = value;
}

/// <summary>
/// Gets a method returning the value to validate.
/// </summary>
public Func<TRow, string> Value { get; private set; }

/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }

/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }

/// <summary>
/// Gets or sets a method to define a custom error message.
/// <summary>
public Func<TRow, string> OverrideErrorMessage { get; set; }

/// <summary>
/// Gets or sets a method to define the final error message.
/// <summary>
public Func<TRow, string> ErrorMessage { get; set; }
}


The whole source code is here : https://github.com/NicoDvl/StringContentValidator



What do you think of that ? How to improve the design, the source code ? Could you find the library useful ? What am I missing ?










share|improve this question









New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




















  • Welcome to Code Review! The code that you have posted in the question is just an example usage, rather than the implementation of the validator. Therefore, I am voting to close this question due to the code not being included.
    – 200_success
    2 days ago










  • @200_success I'll include more parts of the implementation. I did not want it to be too long.
    – NicoD
    2 days ago










  • @200_success I added parts of the implementation and precised some questions. Do not hesitate to tell me if it's better or not. Regards.
    – NicoD
    2 days ago










  • Thanks for adding the code. It's much better now. Close vote retracted.
    – 200_success
    yesterday













up vote
3
down vote

favorite
1









up vote
3
down vote

favorite
1






1





Before I can perform a more or less complex mappings, treatments in business objects and saving results in databases, I often need to validate contents from flat text files or excel files



All the validation libraries I know are designed to validate the content of strongly typed objects. The parsers or readers I tried are really good to load or write data but in case of an error in file, they are design for logging purpose and not for the end user. In addition, I'd like to use a library to do the validation only and be able to change the parser/reader if necessary. My goal is to validate the content of theses files and provide maximun feedbacks to the end users as if it was a form validation in web page.



Instead of wrting code that for sure no one will ever review, I decided to spend the day trying to create a library as an exercice and publish it on GitHub for differents reasons : try to write a fluent API, learn to use GitHub and git more accurately, create unit tests with xUnit and use a CI to automate test project (Travis, AppVeyor...), learn how to create a Nuget package and now try to collect some comments, ideas, remarks...and maybe use the library



For the time being, I end up with something like this :



I define a class with string properties that represent the columns of the files (I could also use a dynamic object) and validate with :



public class Row
{
public string Key { get; set; }
public string DecimalValue { get; set; }
public string DateTimeValue { get; set; }
}

// ...

ClassValidator<Row> validator = ClassValidator<Row>
.Init()
.AddProperty(PropertyValidator<Row>.For(x => x.Key).IsNotNull().HasLength(5,10))
.AddProperty(PropertyValidator<Row>.For(x => x.DateTimeValue).IsNotNull().IsDateTime("yyyyMMdd"))
.AddProperty(PropertyValidator<Row>.For(x => x.DecimalValue).IsNotNull().IsDecimal())
.Validate(new Row()
{
Key = "thiskey",
DateTimeValue = "20181201",
DecimalValue = "123,45"
});


The ClassValidator contains N PropertyValidator for the N properties to validate. Each PropertyValidator can contains N MethodValidator to do the validation (IsNull, TryDecimal, TryRegex ...). Most of the methods are defined as Func, Action delegates executed only when the Validate method is called.



The ClassValidator constructor is private and the Init method can receive some options. The Validate method loop over the List<PropertyValidator<TRow>> populated with the AddProperty method



/// <summary>
/// Validation at class level.
/// </summary>
/// <typeparam name="TRow">The type of the model.</typeparam>
public class ClassValidator<TRow>
{
private readonly IClassValidatorOptions options;

private readonly List<PropertyValidator<TRow>> listValidator = new List<PropertyValidator<TRow>>();

/// <summary>
/// Initializes a new instance of the <see cref="ClassValidator{TRow}"/> class.
/// Private constructor.
/// </summary>
private ClassValidator()
{
}

private ClassValidator(IClassValidatorOptions options)
{
this.options = options;
}

/// <summary>
/// Gets validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successful or not.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Create an instance with no option.
/// </summary>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init()
{
return new ClassValidator<TRow>();
}

/// <summary>
/// Create an instance with option.
/// </summary>
/// <param name="options">options.</param>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init(IClassValidatorOptions options)
{
return new ClassValidator<TRow>(options);
}

/// <summary>
/// Add a property validator. <see cref="PropertyValidator{TRow}"/>.
/// </summary>
/// <param name="propValidator">Targeted property validator.</param>
/// <returns>current instance.</returns>
public ClassValidator<TRow> AddProperty(PropertyValidator<TRow> propValidator)
{
this.listValidator.Add(propValidator);
return this;
}

/// <summary>
/// Validation for one row.
/// </summary>
/// <param name="row">Row to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> Validate(TRow row)
{
foreach (var validator in this.listValidator)
{
validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

return this;
}

/// <summary>
/// Validation for rows collection.
/// </summary>
/// <param name="rows">rows collection to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> ValidateList(IEnumerable<TRow> rows)
{
int index = 0;

foreach (var row in rows)
{
foreach (var validator in this.listValidator)
{
if (this.options != null && this.options.ShowRowIndex)
{
validator.SetRowIndex(index);
}

validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

index++;
}

return this;
}
}



  • If the generic type is set on the top class ClassValidator <Row>.
    Ideally, I would like to set the type once and reuse the same type
    for PropertyValidator. In the sample, I have to define the type for
    ClassValidator and for eachPropertyValidator, which could lead
    to some inconsistency.



The PropertyValidator exposes all the methods to do the validation of the property. Each validation method (eg : TryParseDecimal ) adds a MethodValidator in the Collection>.



/// <summary>
/// Define a validator for a property in <see cref="PropertyValidator{TRow}"/> type.
/// </summary>
/// <typeparam name="TRow">Type for this validator.</typeparam>
public class PropertyValidator<TRow> : IPropertyValidatorAction<TRow>
{
private readonly Collection<MethodValidator<TRow>> methods = new Collection<MethodValidator<TRow>>();
private int? rowIndex;
private string fieldName;
private Func<TRow, string> getter;
private Expression getterExpression;
private dynamic extraObject;
private bool preserveErrorHeader = true;

private PropertyValidator()
{
}

/// <summary>
/// Gets the list of validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successfull.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Creates a property validator for the given property.
/// </summary>
/// <param name="getterExpression">The targeted property.</param>
/// <param name="overrideFieldName">To override field Name. By default uses the name of property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<TRow> For(Expression<Func<TRow, string>> getterExpression, string overrideFieldName = null)
{
PropertyValidator<TRow> prop = new PropertyValidator<TRow>();
prop.getter = getterExpression.Compile();
prop.getterExpression = getterExpression;
prop.fieldName = overrideFieldName != null ? overrideFieldName : ExpressionUtiities.PropertyName(getterExpression);
return prop;
}

/// <summary>
/// Creates a property validator for a dynamic property.
/// </summary>
/// <param name="getter">The targeted dynamic property.</param>
/// <param name="fieldName">Fieldname for the dynamic property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<dynamic> ForDynamic(Func<dynamic, string> getter, string fieldName)
{
PropertyValidator<dynamic> prop = new PropertyValidator<dynamic>();
prop.getter = getter;
prop.fieldName = fieldName;
return prop;
}

/// <summary>
/// Allow to set a row index. Usefull to display a line number in error message.
/// </summary>
/// <param name="rowIndex">Row index.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetRowIndex(int rowIndex)
{
this.rowIndex = rowIndex;
return this;
}

/// <summary>
/// Allow to set an object to pass an extra object to the validator.
/// </summary>
/// <param name="extraObject">Extra dynamic object.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetExtraObject(dynamic extraObject)
{
this.extraObject = extraObject;
return this;
}

/// <summary>
/// Allow overriding default error message.
/// </summary>
/// <param name="msgErrorFunc">Custom error message as a Func.</param>
/// <param name="preserveErrorHeader">Preserve line, field name header in error message.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> OverrideErrorMessage(Func<TRow, string> msgErrorFunc, bool preserveErrorHeader = false)
{
this.preserveErrorHeader = preserveErrorHeader;
this.methods.Last().OverrideErrorMessage = (current) => msgErrorFunc(current);
return this;
}

/// <summary>
/// Check if property is not null.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNull()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNull();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is not null or empty.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNullOrEmpty()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNullOrEmpty();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to decimal.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDecimal()
{
DecimalMethods<TRow> method = new DecimalMethods<TRow>((x) => this.getter(x));
method.TryParseDecimal();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to DateTime.
/// </summary>
/// <param name="format">specified format.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDateTime(string format)
{
DateTimeMethods<TRow> method = new DateTimeMethods<TRow>((x) => this.getter(x));
method.TryParseDateTime(format);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property has required length.
/// </summary>
/// <param name="min">min length.</param>
/// <param name="max">maw length.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> HasLength(int min, int max)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.HasLength(min, max);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is in list values.
/// </summary>
/// <param name="values">List of values.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsStringValues(string values)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsStringValues(values);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property value match regex.
/// </summary>
/// <param name="pattern">Regex pattern.</param>
/// <param name="options">Regex options.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryRegex(string pattern, RegexOptions options = RegexOptions.None)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.TryRegex(pattern, options);
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a validation action of <see cref="IPropertyValidatorAction"/>.
/// </summary>
/// <param name="ifFunc">Condition as a predicate.</param>
/// <param name="action">
/// Validation action from <see cref="IPropertyValidatorAction{TRow}"/> interface.
/// Action may add 1 to N methods.
/// </param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> ifFunc, Action<IPropertyValidatorAction<TRow>> action)
{
// track method count before and after to link condition on all new methods.
var before = this.methods.Count;
action(this);
var after = this.methods.Count;
var nbAdded = after - before;

// take last N methods
var newMethods = this.methods.Skip(Math.Max(0, after - nbAdded));

// add condition on new methods
foreach (var item in newMethods)
{
item.Condition = (current) => ifFunc(current);
}

return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">tocheck if condition is ok. TRow as the current row.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">todo if condition is ok.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, dynamic, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current, this.extraObject);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Validate the property with the current row.
/// </summary>
/// <param name="row">Current row.</param>
public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName, this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}

/// <summary>
/// Create an error message.
/// </summary>
/// <param name="current">Current element.</param>
/// <param name="messageError">Error informations from method. <see cref="IMethodMessageError{TRow}"/>.</param>
/// <returns>Error message.</returns>
private string MessageErrorFactory(TRow current, IMethodMessageError<TRow> messageError)
{
StringBuilder errorMsg = new StringBuilder();

// when overriden error message suppress header by default
if (this.preserveErrorHeader)
{
if (this.rowIndex.HasValue)
{
errorMsg.Append($"{Translation.Row} {this.rowIndex} ");
}

errorMsg.Append($"{Translation.Field} {this.fieldName} : ");
}

// default or override msg
if (messageError.OverrideErrorMessage == null)
{
errorMsg.Append(messageError.ErrorMessage(current));
}
else
{
errorMsg.Append(messageError.OverrideErrorMessage(current));
}

return errorMsg.ToString();
}
}



  • The If methods should look for the last method added to the
    collection. I do not like this way of adding the condition on the
    MethodValidtor as it does not seem robust and reliable


The MethodValidator defines delegates to get the value, the method for validation and and optional condition.



The concrete implemenation is done in sub classes of the MethodValidator.



/// <summary>
/// Define a method to validate a property.
/// </summary>
/// <typeparam name="TRow">Type to validate.</typeparam>
public class MethodValidator<TRow> : IMethodMessageError<TRow>
{
/// <summary>
/// Initializes a new instance of the <see cref="MethodValidator{TRow}"/> class.
/// </summary>
/// <param name="value">delegate to get the value to validate.</param>
public MethodValidator(Func<TRow, string> value)
{
this.Value = value;
}

/// <summary>
/// Gets a method returning the value to validate.
/// </summary>
public Func<TRow, string> Value { get; private set; }

/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }

/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }

/// <summary>
/// Gets or sets a method to define a custom error message.
/// <summary>
public Func<TRow, string> OverrideErrorMessage { get; set; }

/// <summary>
/// Gets or sets a method to define the final error message.
/// <summary>
public Func<TRow, string> ErrorMessage { get; set; }
}


The whole source code is here : https://github.com/NicoDvl/StringContentValidator



What do you think of that ? How to improve the design, the source code ? Could you find the library useful ? What am I missing ?










share|improve this question









New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











Before I can perform a more or less complex mappings, treatments in business objects and saving results in databases, I often need to validate contents from flat text files or excel files



All the validation libraries I know are designed to validate the content of strongly typed objects. The parsers or readers I tried are really good to load or write data but in case of an error in file, they are design for logging purpose and not for the end user. In addition, I'd like to use a library to do the validation only and be able to change the parser/reader if necessary. My goal is to validate the content of theses files and provide maximun feedbacks to the end users as if it was a form validation in web page.



Instead of wrting code that for sure no one will ever review, I decided to spend the day trying to create a library as an exercice and publish it on GitHub for differents reasons : try to write a fluent API, learn to use GitHub and git more accurately, create unit tests with xUnit and use a CI to automate test project (Travis, AppVeyor...), learn how to create a Nuget package and now try to collect some comments, ideas, remarks...and maybe use the library



For the time being, I end up with something like this :



I define a class with string properties that represent the columns of the files (I could also use a dynamic object) and validate with :



public class Row
{
public string Key { get; set; }
public string DecimalValue { get; set; }
public string DateTimeValue { get; set; }
}

// ...

ClassValidator<Row> validator = ClassValidator<Row>
.Init()
.AddProperty(PropertyValidator<Row>.For(x => x.Key).IsNotNull().HasLength(5,10))
.AddProperty(PropertyValidator<Row>.For(x => x.DateTimeValue).IsNotNull().IsDateTime("yyyyMMdd"))
.AddProperty(PropertyValidator<Row>.For(x => x.DecimalValue).IsNotNull().IsDecimal())
.Validate(new Row()
{
Key = "thiskey",
DateTimeValue = "20181201",
DecimalValue = "123,45"
});


The ClassValidator contains N PropertyValidator for the N properties to validate. Each PropertyValidator can contains N MethodValidator to do the validation (IsNull, TryDecimal, TryRegex ...). Most of the methods are defined as Func, Action delegates executed only when the Validate method is called.



The ClassValidator constructor is private and the Init method can receive some options. The Validate method loop over the List<PropertyValidator<TRow>> populated with the AddProperty method



/// <summary>
/// Validation at class level.
/// </summary>
/// <typeparam name="TRow">The type of the model.</typeparam>
public class ClassValidator<TRow>
{
private readonly IClassValidatorOptions options;

private readonly List<PropertyValidator<TRow>> listValidator = new List<PropertyValidator<TRow>>();

/// <summary>
/// Initializes a new instance of the <see cref="ClassValidator{TRow}"/> class.
/// Private constructor.
/// </summary>
private ClassValidator()
{
}

private ClassValidator(IClassValidatorOptions options)
{
this.options = options;
}

/// <summary>
/// Gets validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successful or not.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Create an instance with no option.
/// </summary>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init()
{
return new ClassValidator<TRow>();
}

/// <summary>
/// Create an instance with option.
/// </summary>
/// <param name="options">options.</param>
/// <returns>The created instance.</returns>
public static ClassValidator<TRow> Init(IClassValidatorOptions options)
{
return new ClassValidator<TRow>(options);
}

/// <summary>
/// Add a property validator. <see cref="PropertyValidator{TRow}"/>.
/// </summary>
/// <param name="propValidator">Targeted property validator.</param>
/// <returns>current instance.</returns>
public ClassValidator<TRow> AddProperty(PropertyValidator<TRow> propValidator)
{
this.listValidator.Add(propValidator);
return this;
}

/// <summary>
/// Validation for one row.
/// </summary>
/// <param name="row">Row to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> Validate(TRow row)
{
foreach (var validator in this.listValidator)
{
validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

return this;
}

/// <summary>
/// Validation for rows collection.
/// </summary>
/// <param name="rows">rows collection to validate.</param>
/// <returns>Current instance.</returns>
public ClassValidator<TRow> ValidateList(IEnumerable<TRow> rows)
{
int index = 0;

foreach (var row in rows)
{
foreach (var validator in this.listValidator)
{
if (this.options != null && this.options.ShowRowIndex)
{
validator.SetRowIndex(index);
}

validator.Validate(row);

if (!validator.IsValid)
{
this.ValidationErrors.AddRange(validator.ValidationErrors);
}
}

index++;
}

return this;
}
}



  • If the generic type is set on the top class ClassValidator <Row>.
    Ideally, I would like to set the type once and reuse the same type
    for PropertyValidator. In the sample, I have to define the type for
    ClassValidator and for eachPropertyValidator, which could lead
    to some inconsistency.



The PropertyValidator exposes all the methods to do the validation of the property. Each validation method (eg : TryParseDecimal ) adds a MethodValidator in the Collection>.



/// <summary>
/// Define a validator for a property in <see cref="PropertyValidator{TRow}"/> type.
/// </summary>
/// <typeparam name="TRow">Type for this validator.</typeparam>
public class PropertyValidator<TRow> : IPropertyValidatorAction<TRow>
{
private readonly Collection<MethodValidator<TRow>> methods = new Collection<MethodValidator<TRow>>();
private int? rowIndex;
private string fieldName;
private Func<TRow, string> getter;
private Expression getterExpression;
private dynamic extraObject;
private bool preserveErrorHeader = true;

private PropertyValidator()
{
}

/// <summary>
/// Gets the list of validation errors.
/// </summary>
public List<ValidationError> ValidationErrors { get; private set; } = new List<ValidationError>();

/// <summary>
/// Gets a value indicating whether the validation is successfull.
/// </summary>
public bool IsValid
{
get { return this.ValidationErrors.Count == 0; }
}

/// <summary>
/// Creates a property validator for the given property.
/// </summary>
/// <param name="getterExpression">The targeted property.</param>
/// <param name="overrideFieldName">To override field Name. By default uses the name of property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<TRow> For(Expression<Func<TRow, string>> getterExpression, string overrideFieldName = null)
{
PropertyValidator<TRow> prop = new PropertyValidator<TRow>();
prop.getter = getterExpression.Compile();
prop.getterExpression = getterExpression;
prop.fieldName = overrideFieldName != null ? overrideFieldName : ExpressionUtiities.PropertyName(getterExpression);
return prop;
}

/// <summary>
/// Creates a property validator for a dynamic property.
/// </summary>
/// <param name="getter">The targeted dynamic property.</param>
/// <param name="fieldName">Fieldname for the dynamic property.</param>
/// <returns>A property validator.</returns>
public static PropertyValidator<dynamic> ForDynamic(Func<dynamic, string> getter, string fieldName)
{
PropertyValidator<dynamic> prop = new PropertyValidator<dynamic>();
prop.getter = getter;
prop.fieldName = fieldName;
return prop;
}

/// <summary>
/// Allow to set a row index. Usefull to display a line number in error message.
/// </summary>
/// <param name="rowIndex">Row index.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetRowIndex(int rowIndex)
{
this.rowIndex = rowIndex;
return this;
}

/// <summary>
/// Allow to set an object to pass an extra object to the validator.
/// </summary>
/// <param name="extraObject">Extra dynamic object.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> SetExtraObject(dynamic extraObject)
{
this.extraObject = extraObject;
return this;
}

/// <summary>
/// Allow overriding default error message.
/// </summary>
/// <param name="msgErrorFunc">Custom error message as a Func.</param>
/// <param name="preserveErrorHeader">Preserve line, field name header in error message.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> OverrideErrorMessage(Func<TRow, string> msgErrorFunc, bool preserveErrorHeader = false)
{
this.preserveErrorHeader = preserveErrorHeader;
this.methods.Last().OverrideErrorMessage = (current) => msgErrorFunc(current);
return this;
}

/// <summary>
/// Check if property is not null.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNull()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNull();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is not null or empty.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsNotNullOrEmpty()
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsNotNullOrEmpty();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to decimal.
/// </summary>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDecimal()
{
DecimalMethods<TRow> method = new DecimalMethods<TRow>((x) => this.getter(x));
method.TryParseDecimal();
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is convertible to DateTime.
/// </summary>
/// <param name="format">specified format.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryParseDateTime(string format)
{
DateTimeMethods<TRow> method = new DateTimeMethods<TRow>((x) => this.getter(x));
method.TryParseDateTime(format);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property has required length.
/// </summary>
/// <param name="min">min length.</param>
/// <param name="max">maw length.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> HasLength(int min, int max)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.HasLength(min, max);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property is in list values.
/// </summary>
/// <param name="values">List of values.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> IsStringValues(string values)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.IsStringValues(values);
this.methods.Add(method);
return this;
}

/// <summary>
/// Check if property value match regex.
/// </summary>
/// <param name="pattern">Regex pattern.</param>
/// <param name="options">Regex options.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> TryRegex(string pattern, RegexOptions options = RegexOptions.None)
{
StringMethods<TRow> method = new StringMethods<TRow>((x) => this.getter(x));
method.TryRegex(pattern, options);
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a validation action of <see cref="IPropertyValidatorAction"/>.
/// </summary>
/// <param name="ifFunc">Condition as a predicate.</param>
/// <param name="action">
/// Validation action from <see cref="IPropertyValidatorAction{TRow}"/> interface.
/// Action may add 1 to N methods.
/// </param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> ifFunc, Action<IPropertyValidatorAction<TRow>> action)
{
// track method count before and after to link condition on all new methods.
var before = this.methods.Count;
action(this);
var after = this.methods.Count;
var nbAdded = after - before;

// take last N methods
var newMethods = this.methods.Skip(Math.Max(0, after - nbAdded));

// add condition on new methods
foreach (var item in newMethods)
{
item.Condition = (current) => ifFunc(current);
}

return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">tocheck if condition is ok. TRow as the current row.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Add a condition on a custom validation action.
/// </summary>
/// <param name="predicate">Condition as a predicate.</param>
/// <param name="tocheck">todo if condition is ok.</param>
/// <returns>Current instance.</returns>
public PropertyValidator<TRow> If(Func<TRow, bool> predicate, Func<TRow, dynamic, bool> tocheck)
{
MethodValidator<TRow> method = new MethodValidator<TRow>((x) => this.getter(x));
method.Condition = (current) => predicate(current);
method.ToCheck = (current) => tocheck(current, this.extraObject);
method.ErrorMessage = (current) => Translation.IfError;
this.methods.Add(method);
return this;
}

/// <summary>
/// Validate the property with the current row.
/// </summary>
/// <param name="row">Current row.</param>
public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName, this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}

/// <summary>
/// Create an error message.
/// </summary>
/// <param name="current">Current element.</param>
/// <param name="messageError">Error informations from method. <see cref="IMethodMessageError{TRow}"/>.</param>
/// <returns>Error message.</returns>
private string MessageErrorFactory(TRow current, IMethodMessageError<TRow> messageError)
{
StringBuilder errorMsg = new StringBuilder();

// when overriden error message suppress header by default
if (this.preserveErrorHeader)
{
if (this.rowIndex.HasValue)
{
errorMsg.Append($"{Translation.Row} {this.rowIndex} ");
}

errorMsg.Append($"{Translation.Field} {this.fieldName} : ");
}

// default or override msg
if (messageError.OverrideErrorMessage == null)
{
errorMsg.Append(messageError.ErrorMessage(current));
}
else
{
errorMsg.Append(messageError.OverrideErrorMessage(current));
}

return errorMsg.ToString();
}
}



  • The If methods should look for the last method added to the
    collection. I do not like this way of adding the condition on the
    MethodValidtor as it does not seem robust and reliable


The MethodValidator defines delegates to get the value, the method for validation and and optional condition.



The concrete implemenation is done in sub classes of the MethodValidator.



/// <summary>
/// Define a method to validate a property.
/// </summary>
/// <typeparam name="TRow">Type to validate.</typeparam>
public class MethodValidator<TRow> : IMethodMessageError<TRow>
{
/// <summary>
/// Initializes a new instance of the <see cref="MethodValidator{TRow}"/> class.
/// </summary>
/// <param name="value">delegate to get the value to validate.</param>
public MethodValidator(Func<TRow, string> value)
{
this.Value = value;
}

/// <summary>
/// Gets a method returning the value to validate.
/// </summary>
public Func<TRow, string> Value { get; private set; }

/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }

/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }

/// <summary>
/// Gets or sets a method to define a custom error message.
/// <summary>
public Func<TRow, string> OverrideErrorMessage { get; set; }

/// <summary>
/// Gets or sets a method to define the final error message.
/// <summary>
public Func<TRow, string> ErrorMessage { get; set; }
}


The whole source code is here : https://github.com/NicoDvl/StringContentValidator



What do you think of that ? How to improve the design, the source code ? Could you find the library useful ? What am I missing ?







c# validation csv library fluent-interface






share|improve this question









New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











share|improve this question









New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









share|improve this question




share|improve this question








edited 2 days ago





















New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









asked 2 days ago









NicoD

1163




1163




New contributor




NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.





New contributor





NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






NicoD is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.












  • Welcome to Code Review! The code that you have posted in the question is just an example usage, rather than the implementation of the validator. Therefore, I am voting to close this question due to the code not being included.
    – 200_success
    2 days ago










  • @200_success I'll include more parts of the implementation. I did not want it to be too long.
    – NicoD
    2 days ago










  • @200_success I added parts of the implementation and precised some questions. Do not hesitate to tell me if it's better or not. Regards.
    – NicoD
    2 days ago










  • Thanks for adding the code. It's much better now. Close vote retracted.
    – 200_success
    yesterday


















  • Welcome to Code Review! The code that you have posted in the question is just an example usage, rather than the implementation of the validator. Therefore, I am voting to close this question due to the code not being included.
    – 200_success
    2 days ago










  • @200_success I'll include more parts of the implementation. I did not want it to be too long.
    – NicoD
    2 days ago










  • @200_success I added parts of the implementation and precised some questions. Do not hesitate to tell me if it's better or not. Regards.
    – NicoD
    2 days ago










  • Thanks for adding the code. It's much better now. Close vote retracted.
    – 200_success
    yesterday
















Welcome to Code Review! The code that you have posted in the question is just an example usage, rather than the implementation of the validator. Therefore, I am voting to close this question due to the code not being included.
– 200_success
2 days ago




Welcome to Code Review! The code that you have posted in the question is just an example usage, rather than the implementation of the validator. Therefore, I am voting to close this question due to the code not being included.
– 200_success
2 days ago












@200_success I'll include more parts of the implementation. I did not want it to be too long.
– NicoD
2 days ago




@200_success I'll include more parts of the implementation. I did not want it to be too long.
– NicoD
2 days ago












@200_success I added parts of the implementation and precised some questions. Do not hesitate to tell me if it's better or not. Regards.
– NicoD
2 days ago




@200_success I added parts of the implementation and precised some questions. Do not hesitate to tell me if it's better or not. Regards.
– NicoD
2 days ago












Thanks for adding the code. It's much better now. Close vote retracted.
– 200_success
yesterday




Thanks for adding the code. It's much better now. Close vote retracted.
– 200_success
yesterday










1 Answer
1






active

oldest

votes

















up vote
4
down vote













I'll take time to review just one of the APIs which I find is the most confusing one:




public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName,this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}



This method does some very strange things:




  • calls .Condition that returns a condition

  • if condition is true then ToCheck returns a bool and if that one is false the validation failes

  • stops evaluating other methods on first error without any particular reason


There is not a single helpful name here. Every single line just makes it worse. But at least you have commented these APIs so that one can look there for an explanation:




/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }



OK, I understand it's a pre-validation-condition but why do we need this at all? This should be part of the validation logic. A value is either valid or not. This puts it somehow inbetween. One could argue that it's inconclusive when Condition is false but I'd say if it's not invalid then it's valid. How else would I handle the inconclusive state?



I would remove this step or if you insist on keeping it than you should rename it to something like CanValidate.




/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }



This API also needs to be changed. Since this is the actual validation method then you should name it IsValid.




break; // by default breaks if error



There is still this unexplained break. I see what it's doing so this comment does not help me to understand the logic. You need to tell the reader why this is the default. To me, a casual reader, it's simply weird that it cannot continue with other checks.



When you apply these suggestions and squeeze the conditions together then the method can be shortend to this nice LINQ:



public void Validate(TRow row)
{
ValidationErrors =
(
from method in methods
where method.CanValidate(row) && !method.IsValid(row)
select ValidationError.Failure(fieldName, MessageErrorFactory(row, method))
).ToList();
}





share|improve this answer





















  • Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
    – NicoD
    yesterday










  • @NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
    – t3chb0t
    yesterday












  • I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
    – NicoD
    yesterday












  • @NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
    – t3chb0t
    yesterday






  • 1




    That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
    – NicoD
    yesterday











Your Answer





StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");

StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});






NicoD is a new contributor. Be nice, and check out our Code of Conduct.










draft saved

draft discarded


















StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208729%2fsimple-fluent-library-to-validate-text-content%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown

























1 Answer
1






active

oldest

votes








1 Answer
1






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
4
down vote













I'll take time to review just one of the APIs which I find is the most confusing one:




public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName,this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}



This method does some very strange things:




  • calls .Condition that returns a condition

  • if condition is true then ToCheck returns a bool and if that one is false the validation failes

  • stops evaluating other methods on first error without any particular reason


There is not a single helpful name here. Every single line just makes it worse. But at least you have commented these APIs so that one can look there for an explanation:




/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }



OK, I understand it's a pre-validation-condition but why do we need this at all? This should be part of the validation logic. A value is either valid or not. This puts it somehow inbetween. One could argue that it's inconclusive when Condition is false but I'd say if it's not invalid then it's valid. How else would I handle the inconclusive state?



I would remove this step or if you insist on keeping it than you should rename it to something like CanValidate.




/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }



This API also needs to be changed. Since this is the actual validation method then you should name it IsValid.




break; // by default breaks if error



There is still this unexplained break. I see what it's doing so this comment does not help me to understand the logic. You need to tell the reader why this is the default. To me, a casual reader, it's simply weird that it cannot continue with other checks.



When you apply these suggestions and squeeze the conditions together then the method can be shortend to this nice LINQ:



public void Validate(TRow row)
{
ValidationErrors =
(
from method in methods
where method.CanValidate(row) && !method.IsValid(row)
select ValidationError.Failure(fieldName, MessageErrorFactory(row, method))
).ToList();
}





share|improve this answer





















  • Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
    – NicoD
    yesterday










  • @NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
    – t3chb0t
    yesterday












  • I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
    – NicoD
    yesterday












  • @NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
    – t3chb0t
    yesterday






  • 1




    That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
    – NicoD
    yesterday















up vote
4
down vote













I'll take time to review just one of the APIs which I find is the most confusing one:




public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName,this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}



This method does some very strange things:




  • calls .Condition that returns a condition

  • if condition is true then ToCheck returns a bool and if that one is false the validation failes

  • stops evaluating other methods on first error without any particular reason


There is not a single helpful name here. Every single line just makes it worse. But at least you have commented these APIs so that one can look there for an explanation:




/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }



OK, I understand it's a pre-validation-condition but why do we need this at all? This should be part of the validation logic. A value is either valid or not. This puts it somehow inbetween. One could argue that it's inconclusive when Condition is false but I'd say if it's not invalid then it's valid. How else would I handle the inconclusive state?



I would remove this step or if you insist on keeping it than you should rename it to something like CanValidate.




/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }



This API also needs to be changed. Since this is the actual validation method then you should name it IsValid.




break; // by default breaks if error



There is still this unexplained break. I see what it's doing so this comment does not help me to understand the logic. You need to tell the reader why this is the default. To me, a casual reader, it's simply weird that it cannot continue with other checks.



When you apply these suggestions and squeeze the conditions together then the method can be shortend to this nice LINQ:



public void Validate(TRow row)
{
ValidationErrors =
(
from method in methods
where method.CanValidate(row) && !method.IsValid(row)
select ValidationError.Failure(fieldName, MessageErrorFactory(row, method))
).ToList();
}





share|improve this answer





















  • Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
    – NicoD
    yesterday










  • @NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
    – t3chb0t
    yesterday












  • I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
    – NicoD
    yesterday












  • @NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
    – t3chb0t
    yesterday






  • 1




    That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
    – NicoD
    yesterday













up vote
4
down vote










up vote
4
down vote









I'll take time to review just one of the APIs which I find is the most confusing one:




public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName,this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}



This method does some very strange things:




  • calls .Condition that returns a condition

  • if condition is true then ToCheck returns a bool and if that one is false the validation failes

  • stops evaluating other methods on first error without any particular reason


There is not a single helpful name here. Every single line just makes it worse. But at least you have commented these APIs so that one can look there for an explanation:




/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }



OK, I understand it's a pre-validation-condition but why do we need this at all? This should be part of the validation logic. A value is either valid or not. This puts it somehow inbetween. One could argue that it's inconclusive when Condition is false but I'd say if it's not invalid then it's valid. How else would I handle the inconclusive state?



I would remove this step or if you insist on keeping it than you should rename it to something like CanValidate.




/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }



This API also needs to be changed. Since this is the actual validation method then you should name it IsValid.




break; // by default breaks if error



There is still this unexplained break. I see what it's doing so this comment does not help me to understand the logic. You need to tell the reader why this is the default. To me, a casual reader, it's simply weird that it cannot continue with other checks.



When you apply these suggestions and squeeze the conditions together then the method can be shortend to this nice LINQ:



public void Validate(TRow row)
{
ValidationErrors =
(
from method in methods
where method.CanValidate(row) && !method.IsValid(row)
select ValidationError.Failure(fieldName, MessageErrorFactory(row, method))
).ToList();
}





share|improve this answer












I'll take time to review just one of the APIs which I find is the most confusing one:




public void Validate(TRow row)
{
this.ValidationErrors = new List<ValidationError>();

foreach (var method in this.methods)
{
bool condition = true;
if (method.Condition != null)
{
condition = method.Condition(row);
}

if (condition)
{
bool ok = method.ToCheck(row);
if (!ok)
{
this.ValidationErrors.Add(ValidationError.Failure(this.fieldName,this.MessageErrorFactory(row, method)));

break; // by default breaks if error
}
}
}
}



This method does some very strange things:




  • calls .Condition that returns a condition

  • if condition is true then ToCheck returns a bool and if that one is false the validation failes

  • stops evaluating other methods on first error without any particular reason


There is not a single helpful name here. Every single line just makes it worse. But at least you have commented these APIs so that one can look there for an explanation:




/// <summary>
/// Gets or sets a condition prior to check if property value is valid.
/// </summary>
public Func<TRow, bool> Condition { get; set; }



OK, I understand it's a pre-validation-condition but why do we need this at all? This should be part of the validation logic. A value is either valid or not. This puts it somehow inbetween. One could argue that it's inconclusive when Condition is false but I'd say if it's not invalid then it's valid. How else would I handle the inconclusive state?



I would remove this step or if you insist on keeping it than you should rename it to something like CanValidate.




/// <summary>
/// Gets or sets a method to check if property value is valid.
/// </summary>
public Func<TRow, bool> ToCheck { get; set; }



This API also needs to be changed. Since this is the actual validation method then you should name it IsValid.




break; // by default breaks if error



There is still this unexplained break. I see what it's doing so this comment does not help me to understand the logic. You need to tell the reader why this is the default. To me, a casual reader, it's simply weird that it cannot continue with other checks.



When you apply these suggestions and squeeze the conditions together then the method can be shortend to this nice LINQ:



public void Validate(TRow row)
{
ValidationErrors =
(
from method in methods
where method.CanValidate(row) && !method.IsValid(row)
select ValidationError.Failure(fieldName, MessageErrorFactory(row, method))
).ToList();
}






share|improve this answer












share|improve this answer



share|improve this answer










answered yesterday









t3chb0t

33.8k746108




33.8k746108












  • Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
    – NicoD
    yesterday










  • @NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
    – t3chb0t
    yesterday












  • I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
    – NicoD
    yesterday












  • @NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
    – t3chb0t
    yesterday






  • 1




    That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
    – NicoD
    yesterday


















  • Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
    – NicoD
    yesterday










  • @NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
    – t3chb0t
    yesterday












  • I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
    – NicoD
    yesterday












  • @NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
    – t3chb0t
    yesterday






  • 1




    That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
    – NicoD
    yesterday
















Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
– NicoD
yesterday




Perfectly right, I will improve the naming : .Condition that return a condition by .PreCondition that return a result. This PreCondition is used to condition a Validation eg : .If(x => x.Type == "P", p.IsNotNull().TryParseDecimal())); that's why it's not included in the validation. The unexplained break stands for : stop validation on first failure. It could be helpfull if a validator depends on the previous validator to succeed. I agree it should not be the default and need clarification.
– NicoD
yesterday












@NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
– t3chb0t
yesterday






@NicoD I agree it should not be the default - not quite what I mean ;-) it can be default but there has to be a reason for that. If you have one then it's perfectly fine, just put this reason as a comment above the break - you virtually should write // by default breaks if error because... ;-]
– t3chb0t
yesterday














I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
– NicoD
yesterday






I really think it should not be the default but seems easier for the first draft ;). I will update this part and comments. What is the best way to share corrections: I update the code in my question?
– NicoD
yesterday














@NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
– t3chb0t
yesterday




@NicoD I update the code in my question? - no, please don't - it's not allowed when there are already answers posted - the best you can do is to wait a couple of days for more reviews to come and when you think there is some helpful information you can accept one answer and post your new code either as a self-answer or a new question is you'd like to have one more review.
– t3chb0t
yesterday




1




1




That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
– NicoD
yesterday




That makes sense. I will update the Git repository and I'll keep you informed if you want to take a look. Thank you for the review :)
– NicoD
yesterday










NicoD is a new contributor. Be nice, and check out our Code of Conduct.










draft saved

draft discarded


















NicoD is a new contributor. Be nice, and check out our Code of Conduct.













NicoD is a new contributor. Be nice, and check out our Code of Conduct.












NicoD is a new contributor. Be nice, and check out our Code of Conduct.
















Thanks for contributing an answer to Code Review Stack Exchange!


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


Use MathJax to format equations. MathJax reference.


To learn more, see our tips on writing great answers.





Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


Please pay close attention to the following guidance:


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208729%2fsimple-fluent-library-to-validate-text-content%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







Popular posts from this blog

Quarter-circle Tiles

build a pushdown automaton that recognizes the reverse language of a given pushdown automaton?

Mont Emei