Simple fluent library to validate text content
up vote
3
down vote
favorite
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
forPropertyValidator
. 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
New contributor
add a comment |
up vote
3
down vote
favorite
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
forPropertyValidator
. 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
New contributor
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
add a comment |
up vote
3
down vote
favorite
up vote
3
down vote
favorite
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
forPropertyValidator
. 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
New contributor
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
forPropertyValidator
. 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
c# validation csv library fluent-interface
New contributor
New contributor
edited 2 days ago
New contributor
asked 2 days ago
NicoD
1163
1163
New contributor
New contributor
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
add a comment |
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
add a comment |
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 acondition
- if
condition
istrue
thenToCheck
returns abool
and if that one isfalse
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();
}
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 thebreak
- 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
add a comment |
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 acondition
- if
condition
istrue
thenToCheck
returns abool
and if that one isfalse
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();
}
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 thebreak
- 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
add a comment |
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 acondition
- if
condition
istrue
thenToCheck
returns abool
and if that one isfalse
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();
}
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 thebreak
- 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
add a comment |
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 acondition
- if
condition
istrue
thenToCheck
returns abool
and if that one isfalse
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();
}
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 acondition
- if
condition
istrue
thenToCheck
returns abool
and if that one isfalse
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();
}
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 thebreak
- 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
add a comment |
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 thebreak
- 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
add a comment |
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.
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.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
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