Instance validation in C#
Equality of validation

While writing tests for the code presented in the last couple of weeks, I discovered a notable bug caused by an ommission in the code. If you’re a regular reader of this blog, you may have already spotted what was left out.

Frankly, the omission is a bit embarassing, as it concerns a topic on which I recently wrote an entire series of posts … but there’s a separate lesson here that’s worth sharing. Unit testing works. (Even when you’re testing simple things you won’t ever get wrong!)

Here are the unit tests I wrote that revealed the problem:

public class ConstructorWithParams : AggregateResultTests
{
    [Fact]
    public void GivenSingleError_HasSingleError()
    {
        var aggregate = new AggregateResult(_error);
        aggregate.Errors().Should().HaveCount(1);
        aggregate.Errors().Single().Should().Be(_error);
    }

    [Fact]
    public void GivenSameErrorMultipleTimes_HasSingleError()
    {
        var aggregate = new AggregateResult(_error, _error, _error);
        aggregate.Errors().Should().HaveCount(1);
    }

    [Fact]
    public void GivenEquivilentErrors_HasSingleError()
    {
        var aggregate = new AggregateResult(_error, _sameError, _error);
        aggregate.Errors().Should().HaveCount(1);
    }
}

Some background: I’m using the xUnit test framework along with Fluent Assertions to check the results. I group tests of the same piece of functionality together in a single class as described by Phil Haack.

The first test checks that a new AggregateResults containing a single error contains the error I expect. It passed.

The second test checks that including the same error multiple times doesn’t result in the error being duplicated (this property of uniquing is very useful). It also passed.

The third test checks that including identical errors multiple times doesn’t result in the error being duplicated. It failed.

The source of the problem, of course, is that I haven’t implemented equality on ValidationResult (and its subclasses), so the system is falling back on the default implementation inherited from object, which uses reference equality.

To address this, we modify ValidationResult by declaring that it implements the interface IEquatable<ValidationResult>, and by providing a framework for implementation of Equals() by its subclasses, as follows.

public abstract class ValidationResult
    : IEquatable<ValidationResult>
{
    protected static readonly StringComparer _comparer =
        StringComparer.OrdinalIgnoreCase;

The static member _comparer is a simple way to ensure that all of our implementations are consistent in the way they treat character comparisons, helping to avoid any issues due to inconsistency between .Equals() and .GetHashCode().

We include a static .Equals() method, allowing easy comparison when either instance might already be null:

    public static bool Equals(
        ValidationResult left, ValidationResult right)
    {
        if (ReferenceEquals(left, right))
        {
            return true;
        }

        if (left == null || right == null)
        {
            return false;
        }

        // Neither left nor right is null
        return left.Equals(right);
    }

Our subclasses are forced to implement equality by marking the non-static .Equals() method abstract - this helps to force those subclasses to consider the issues around equality.

    public abstract bool Equals(ValidationResult other);

As a convenience, we implement .Equals(object) ourselves - and we seal the implementation. This has two important effects - it reduces the amount of code required for each subclass, reducing the effort of implementation, and it reduces the opportunity for inconsistency by ensuring that the entire class heirarchy has the same approach. This is a form of defensive programming.

    public sealed override bool Equals(object obj)
        => obj is ValidationResult v
           && Equals(v);
}

As you probably expect, the implementations on the subclasses are fairly straightforward. For clarity, each also implements IEquatable<T> for the specific subclass.

public sealed class ErrorResult 
    : ValidationResult, IEquatable<ErrorResult>
{

For ErrorResult, the important check is whether the two errors have the same message:

    public bool Equals(ErrorResult other)
        => other != null
           && _comparer.Equals(Message, other.Message);

The more generic method, .Equals(ValidationResult) (below), simply delegates to the more specific method, .Equals(ErrorResult) (above). By avoiding duplication of the actual equality comparison, we avoid potential issues with inconsistency if things ever change.

    public override bool Equals(ValidationResult other)
        => other is ErrorResult e
           && Equals(e);

We also mustn’t forget to override .GetHashCode(), using _comparer to generate a value based on the message itself.

    public override int GetHashCode() => _comparer.GetHashCode(Message);
}

For AggregateResult, the code is substantially similar - with the twist that the implemenations of .Equals(AggregateResult) and .GetHashCode() need to combine results from all the contained ValidationResults.

public sealed class AggregateResult 
    : ValidationResult, IEquatable<AggregateResult>
{
    public bool Equals(AggregateResult other)
        => other?.Results.SetEquals(Results) ?? false;

We’re lucky that ImmutableHashSet<T> includes the very useful .SetEquals() method that allows us to easily compare the other set of results with our own. If other is null, we fall back to returning false courtesy of the ?? operator.

To calculate the hashcode, we start with a hard coded random prime number and XOR the hashcodes of the various results contained. We don’t do any multiplication because the iteration order for a set isn’t fixed - and we need two AggregateResults with the same content to have the same hashcode even if they happen to iterate the set in different orders. (This can happen if the items in the two sets were added in different orders.)

    public override int GetHashCode()
        => Results.Aggregate(
            217_645_177,
            (hash, result) => hash ^ result.GetHashCode());
}

There’s nothing special about the number 217 645 177, I just chose a random prime. The underscores (_) are a new feature in C#7 to improve the readability of numeric constants; it’s espececially useful for binary numbers - here’s the same prime number, to illustrate: ‭1100_1111_1001_0000_0000_0111_1001‬.

For SuccessResult, things are trivially simple - since every success is equal to every other, the code has almost nothing to do:

public sealed class SuccessResult
    : SingleValidationResult, IEquatable<SuccessResult>
{
    public bool Equals(SuccessResult other)
        => other != null;

    public override bool Equals(ValidationResult other)
        => other is SuccessResult;

    public override int GetHashCode()
        => typeof(SuccessResult).GetHashCode();
}