Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ShouldNotEqual: not working when implementing IComparable #372

Open
rbarillec opened this issue Jan 21, 2019 · 0 comments
Open

ShouldNotEqual: not working when implementing IComparable #372

rbarillec opened this issue Jan 21, 2019 · 0 comments

Comments

@rbarillec
Copy link

rbarillec commented Jan 21, 2019

Hi,

First of all, thanks for a great testing framework. We use it extensively at work and are generally very pleased with it.

However, we've run recently into a weird situation where equality tests seem to be failing when they shouldn't. It's easier to explain with an example.

Consider a Person class, which implements both IEquatable<T> and IComparable<T>:

  • 2 persons are equal if they have the same ID
  • the order between 2 persons is defined by their name (alphabetical). So a sorted list of 4 persons might look like [ Amy, Ben, Ben, Beth] (with no preference for which Ben comes first).
    class Person : IEquatable<Person>, IComparable<Person>
    {
        private readonly int _id;
        private readonly string _name;

        public Person(int id, string name)
        {
            _id = id;
            _name = name;
        }

        public bool Equals(Person other)
        {
            return (other != null) && other._id == this._id;
        }

        public override int GetHashCode()
        {
            return _id;
        }

        public int CompareTo(Person other)
        {
            return this._name.CompareTo(other._name);
        }
    }

And the following test:

    class Test
    {
        static Person person1, person2;

        Establish context = () =>
        {
            person1 = new Person(1, "John Doe");
            person2 = new Person(2, "John Doe");  // Same name, different person
        };

        It should_have_the_same_rank = () => person1.CompareTo(person2).ShouldEqual(0);         // assertion passes
        It should_not_be_the_same_person = () => person1.ShouldEqual(person2);                  // assertion passes (but should fail?)
        It should_be_2_different_persons_v1 = () => person1.Equals(person2).ShouldBeFalse();    // assertion passes 
        It should_be_2_different_persons_v2 = () => person1.ShouldNotEqual(person2);            // assertion fails (but should pass?)
    }

For some reason, 2 of these assertions fail. We had a quick look at why that might be and it seems that MSpec uses a number of Comparer strategies, as defined here.

I could be wrong, but do these mean that MSpec gives IComparable<T> preference over IEquatable<T>? If so, that seems to me like a misunderstanding. I always thought IComparable was about ordering things and IEquatable was about defining what makes 2 things equal. So I think I would expect:

  • the EquatableComparer to come first in the list - and to be the only comparer used for cases which do implement IEquatable<T>. If I have explicitly defined the meaning of equality for a type, I'd expect this to be the only definition of equality.
  • IComparable<T> to not feature in the list, since it is about comparison, not about equality. To me that's a different use case (at least that 's how I read the docs: IComparable is about ordering and sorting, IEquatable is about equality). For instance, in the example above, 2 persons can have the same name, and therefore the same order, but that doesn't mean they are the same person.

Am I missing something? Is there an underlying reason for using IComparable<T> when determining equality? What is the recommended way to test equality for objects which implement IComparable<T>?

We also found this old issue, which is somewhat related.

Thanks for your help! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants