Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Suggestions on validating a class and its collection properties

Tags:

c#

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; }
}
like image 233
Ɖiamond ǤeezeƦ Avatar asked Dec 18 '25 23:12

Ɖiamond ǤeezeƦ


2 Answers

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.

like image 189
Matti Virkkunen Avatar answered Dec 21 '25 13:12

Matti Virkkunen


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.

like image 31
Matthew Avatar answered Dec 21 '25 11:12

Matthew



Donate For Us

If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!