Skip to content

Should().Be() / NotBe() prefers IComparable<T>.Compare over object.Equals #172

@ulrichb

Description

@ulrichb

Given the following class with value semantics.

public class Ranked<T> : IComparable<Ranked<T>>
{
    public Ranked (int ranking, T name) { Ranking = ranking; Item = name; }

    public int Ranking { get; private set; }
    public T Item { get; private set; }

    public override bool Equals (object obj)
    {
        if (ReferenceEquals (null, obj))
            return false;
        if (obj.GetType() != this.GetType())
            return false;
        var other = (Ranked<T>) obj;
        return Ranking == other.Ranking && Equals (Item, other.Item);
    }

    public override int GetHashCode () { /* ... */ }

    public int CompareTo (Ranked<T> other)
    {
        if (other == null) return 1;

        return this.Ranking.CompareTo (other.Ranking);
    }
}

At the moment (v3.2.1) the assertion new Ranked<string>(1, "Some item").Should().Be (new Ranked<string>(1, "Other item")) passes whereas casting the first part to object, ((object)new Ranked<string>(1, "Some item")).Should().Be (new Ranked<string>(1, "Other item")), fails. The first assertion calls CompareTo, which just compares the Ranking value and the second one calls Equals.

The reason for the different behavior of the two assertions is the IComparable<> overload of Should(). The idea of this overload is to be able to offer fluent methods for IComparable<> values, like BeInRange(). Unfortunately also Be() and NotBe() are "overridden" to check for CompareTo() == 0 resp. CompareTo() != 0.

In my opinion this is not a good idea, because Be() / NotBe() are the standard methods in FluentAssertions to check for equality / inequality. And sort ordering is not the same as equality. The example above shows a legitimate case, where just a subset of the properties is part of the ordering.

Thinking of another example of a Person-entity where equality is defined by ID-equality (e.g. a SocialSecurityNumber property) and the ordering should be applied for LastName / GivenName shows that the set of properties used for equality / ordering don't even have to intersect.

We should think about a (breaking) change of the behavior of ComparableTypeAssertions<T>.Be(). So that Should().Be() always checks for equality and introduce a new Should().HaveSameOrderingPositionAs() (probably there's better name), which checks for CompareTo() == 0.

The reason why I would even prefer the breaking change is that a) it's unintuitive that Should.Be() delegates to CompareTo depending on the compile time type of the left operand and b) the duality of Should().Be() is unsafe for example when adding IComparable<T> to the Ranked<T> example above after there are already a lot of unit tests, which use Should.Be() and rely on its behavior.

Further notes:

  • The MSDN documentation team strengthened the definition of the zero result of IComparable.CompareTo from to "This instance is equal to obj" to "This current instance occurs in the same position in the sort order as the object specified by the CompareTo method" in .NET 3.5 (obviously to distinguish between equality and sort ordering). Interestingly they didn't strengthen the definition of the generic version, but the semantics are the same for both interfaces. (This happened in the meantime.)
  • NUnit's Assert.AreEqual and Assert.That only use IEquatable<>.Equals and object.Equals (depending on the runtime type), which both have the same semantics.
  • MSTest's Assert.AreEqual only uses object.Equals.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions