I have a Query class that has a number of properties that are collections of other objects. As part of validating a Query object I want to validate all objects in each of the Query's collections. So far I have come up with three approaches to do this. I would like feedback on which approach people prefer and any other possible approaches.
Approach 1
public static ValidationResult ValidateQuery(Query query)
{
ValidationResult result;
result = ValidateColumns(query);
if (result.Passed)
{
result = ValidateFilters(query);
if (result.Passed)
{
result = ValidateSortOrders(query);
}
}
return result;
}
While the number of collection properties is small, the code is readable. But if I had significantly more, I would end up with lot of nested if statements, which in my opinion reduces readability. I try to resolve this in my next two approaches:
Approach 2
public static ValidationResult ValidateQuery(Query query)
{
ValidationResult[] results = new ValidationResult[4];
results[0] = ValidateColumns(query);
results[1] = ValidateFilters(query);
results[2] = ValidateGroups(query);
results[3] = ValidateSortOrders(query);
return results.FirstOrDefault(item => !item.Passed) ?? results[0];
}
Approach 3
public static ValidationResult ValidateQuery(Query query)
{
ValidationResult result = null;
int i = 0;
bool done = false;
do
{
switch (i)
{
case 0: result = ValidateColumns(query); break;
case 1: result = ValidateGroups(query); break;
case 2: result = ValidateSortOrders(query); break;
default: done = true; break;
}
++i;
} while (result.Passed && !done);
return result ?? new ValidationResult(true, string.Empty);
}
In case you need it, the definition for the ValidationResult class:
public class ValidationResult
{
public ValidationResult(bool passed, string message)
{
this.Passed = passed;
this.ErrorMessage = message ?? string.Empty;
}
public string ErrorMessage {get; private set; }
public bool Passed { get; private set; }
}
What's wrong with
result = ValidateSomething();
if (!result.Passed)
return result;
result = ValidateSomethingElse();
if (!result.Passed)
return result;
That is, if you really want to only return one error. If this is for user input, it results in a rather annoying interface. When the user has made multiple errors, this thing can only report one of them at a time, and the user will have to keep making corrections one by one until they're gone.
Have you considered returning a collection of validation results? You could pass the collection in to each validation method and have them add their possible errors to the collection.
And avoid the while-switch.
What about yield return?
IEnumerable<string> ValidateAll(Query query)
{
if( !ValidateSomething() ) {
yield return "Validate Something Failed...";
}
if( !ValidateSomethingelse() ) {
yield return "Something else failed...";
}
}
Of course, the enumerable type does not have to be a string.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With