8
\$\begingroup\$

Writing tests is sometimes a really boring task especially if you need to write the same test for the n-th time like when you are testing another custom EqualityComparer<T>.

I thought if I write another such test I'm going crazy ;-) then maybe I'll try to automate it at least a little bit so I wrote a base class for such tests. In particular it tests the GetHashCode and Equals methods with the possibility to disable the hash-code part because I sometimes just use a 0 if I am not interested in it or it's not possible to calculate it.

public abstract class EqualityComparerTest<T>
{
    private readonly IEqualityComparer<T> _comparer;

    protected EqualityComparerTest(IEqualityComparer<T> comparer)
    {
        _comparer = comparer;
    }

    protected bool IgnoreHashCode { get; set; }

    [TestMethod]
    public void GetHashCode_SameElements_SameHashCodes()
    {
        if (IgnoreHashCode)
        {
            return;
        }

        foreach (var x in GetEqualElements())
        {
            Assert.AreEqual(_comparer.GetHashCode(x.Left), _comparer.GetHashCode(x.Right), $"{x.Left} == {x.Right}");
        }
    }

    [TestMethod]
    public void GetHashCode_DifferentElements_DifferentHashCodes()
    {
        if (IgnoreHashCode)
        {
            return;
        }

        foreach (var x in GetNonEqualElements())
        {
            Assert.AreNotEqual(_comparer.GetHashCode(x.Left), _comparer.GetHashCode(x.Right), $"{x.Left} != {x.Right}");
        }
    }

    [TestMethod]
    public void Equals_SameElements_True()
    {
        foreach (var x in GetEqualElements())
        {
            Assert.IsTrue(_comparer.Equals(x.Left, x.Right), $"{x.Left} == {x.Right}");
            Assert.IsTrue(_comparer.Equals(x.Left, x.Left), $"{x.Left} == {x.Right}");
            Assert.IsTrue(_comparer.Equals(x.Right, x.Right), $"{x.Left} == {x.Right}");
        }
    }

    [TestMethod]
    public void Equals_DifferentElements_False()
    {
        foreach (var x in GetNonEqualElements())
        {
            Assert.IsFalse(_comparer.Equals(x.Left, x.Right), $"{x.Left} != {x.Right}");
        }
    }

    protected abstract IEnumerable<(T Left, T Right)> GetEqualElements();

    protected abstract IEnumerable<(T Left, T Right)> GetNonEqualElements();
}

The data for the tests comes from two abstract methods that the actual test needs to provide.

Here I'm testing an ImmutableNameSet that implements the IImmutableSet<string> interface and internaly the set uses the StringComparer.OrdinalIgnoreCase and they are considered equal if they overlap.

I added the comparer I test just for reference (the question is about the test-base class and the implementation - the comparer works as required).

private sealed class ImmutableNameSetEqualityComparer : IEqualityComparer<IImmutableSet<string>>
{
    public bool Equals(IImmutableSet<string> x, IImmutableSet<string> y)
    {
        if (ReferenceEquals(x, null)) return false;
        if (ReferenceEquals(y, null)) return false;
        return ReferenceEquals(x, y) || x.Overlaps(y) || (!x.Any() && !y.Any());
    }

        // The hash code are always different thus this comparer. We need to check if the sets overlap so we cannot relay on the hash code.
        public int GetHashCode(IImmutableSet<string> obj) => 0;
    }
}

And this is the actual test for it:

[TestClass]
public class ImmutableNameSetEqualityComparerTest : EqualityComparerTest<IImmutableSet<string>>
{
    public ImmutableNameSetEqualityComparerTest() : base(ImmutableNameSet.Comparer)
    {
        IgnoreHashCode = true;
    }

    protected override IEnumerable<(IImmutableSet<string> Left, IImmutableSet<string> Right)> GetEqualElements()
    {
        yield return (ImmutableNameSet.Create("foo"), ImmutableNameSet.Create("FOO"));
        yield return (ImmutableNameSet.Create("foo"), ImmutableNameSet.Create("FOO", "bar"));
        yield return (ImmutableNameSet.Create("foo"), ImmutableNameSet.Create("foo"));
    }

    protected override IEnumerable<(IImmutableSet<string> Left, IImmutableSet<string> Right)> GetNonEqualElements()
    {
        yield return (ImmutableNameSet.Create("foo"), ImmutableNameSet.Create("bar"));
        yield return (ImmutableNameSet.Create("baz"), ImmutableNameSet.Create("foo"));
        yield return (null, ImmutableNameSet.Create("foo"));
        yield return (null, null);
    }
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ In order for an IEqualityComparer<T> to be useful as, say, a dictionary comparer, it must be a proper equivalence relation -- i.e., reflexive, symmetric and transitive. Your ImmutableNameSetEqualityComparer does not appear to be transitive, e.g. ("a", "b") overlaps ("b", "c") which overlaps ("c", "d") - but the first does not overlap the third. You might want to think how to automatically validate such a requirement so that your existing implementation would get flagged. \$\endgroup\$
    – dbc
    Jul 4, 2017 at 20:29
  • \$\begingroup\$ @dbc this is fine as this time I consider ab=bc or bc=cd, this means if you try to add a bc item to a dictionary that already contains an ab item it would throw a duplicate-key-exception. These are command names (normal and aliases) and they must not overlap, if they do it's not possible to find one. It's ok that the first does not overlap the last, this means they are different commands in my case. But let's not get distracted by this :-) the question is about testing equality-comparers in general so that I don't have to implement the same test over and over again. \$\endgroup\$
    – t3chb0t
    Jul 5, 2017 at 4:10
  • \$\begingroup\$ @dbc where did you actually find the dictionary key requirements? I can't remember reading it in the documentation. You must be a mathematician ;-) \$\endgroup\$
    – t3chb0t
    Jul 5, 2017 at 4:21
  • 4
    \$\begingroup\$ Actually yes, my education was in math, so I was just aware of this requirement as background knowledge. But I was also able to find it in the documentation for IEqualityComparer<T>.Equals: The Equals method is reflexive, symmetric, and transitive. \$\endgroup\$
    – dbc
    Jul 5, 2017 at 5:42
  • \$\begingroup\$ @dbc thx, I'll keep this in mind for future implementations. In this case this won't be possible. That's why I use 0 for the hash code because I cannot calculate it. The equality can only be determined by an overlap. Imagine there is a command Open with an alias O, this means there cannot be another key that containes either name and my key is such a multiple names key. It should just make sure no name or alias is used twice and that I can get a command either by its full name or an alias. I had to break the rule :-] \$\endgroup\$
    – t3chb0t
    Jul 5, 2017 at 6:12

3 Answers 3

4
\$\begingroup\$
public bool Equals(IImmutableSet<string> x, IImmutableSet<string> y)
{
    if (ReferenceEquals(x, null)) return false;
    if (ReferenceEquals(y, null)) return false;
    return ReferenceEquals(x, y) || x.Overlaps(y) || (!x.Any() && !y.Any());
}

Well, with this implementation, Equals(null, null) is going to return false. I don't think that's the best option.

The intended meaning of Equals is "is identical to", and without a doubt, null is identical to null, because null is the same value as null. You can give Equals whatever behavior you want, but if you give it behavior that contradicts its intended meaning, people will be surprised and confused.

Furthermore, if you pass this instance of IEqualityComparer into some code which expects Equals(x, x) to always be true, then that code may malfunction. For example, if you use it to create a dictionary which allows null as a key, then that dictionary will allow inserting multiple key-value pairs with null keys, and will fail to ever retrieve any of them.

You write in a comment:

I consider anything that is not a valid element and not equal as un-equal thus I consider two nulls are not equals. It wouldn't make any sense to have to null names and consider their equality as true. With this assumption or rather adjustment I don't have to filter null values.

Well, there's no such thing as "two nulls". There is only one null reference, so if two variables both contain a null reference, then those two variables contain the same value. I'm not sure what you mean by "I don't have to filter null values"; when would you have to filter null values if Equals(null, null) returned true?

\$\endgroup\$
1
  • \$\begingroup\$ After giving it a second and third thought and hearing it from two reviews ;-) I've change it to behave as expected. The example with the dictionary is a good one. I actually use it with a dictionary and missed this case. I also like the explanation about not being able to ever find it again. \$\endgroup\$
    – t3chb0t
    Jul 6, 2017 at 15:41
4
\$\begingroup\$
[TestMethod]
public void Equals_SameElements_True()
{
    foreach (var x in GetEqualElements())
    {
        Assert.IsTrue(_comparer.Equals(x.Left, x.Right), $"{x.Left} == {x.Right}");
        Assert.IsTrue(_comparer.Equals(x.Left, x.Left), $"{x.Left} == {x.Right}");
        Assert.IsTrue(_comparer.Equals(x.Right, x.Right), $"{x.Left} == {x.Right}");
    }
}  

As I was reading this I thought that it looks strange. You Assert that x.Left should Equals x.Left and if this fails you output the message $"{x.Left} == {x.Right}". The same is true for comparing x.Right to x.Right.

IMO there should be a different message stating that any of these two Assert's failed.

While we are at the assertation failing messages there are some more.

In GetHashCode_SameElements_SameHashCodes() this line

Assert.AreEqual(_comparer.GetHashCode(x.Left), _comparer.GetHashCode(x.Right), $"{x.Left} == {x.Right}");  

is strange as well. If AreEqual fails the message states $"{x.Left} == {x.Right}".

In GetHashCode_DifferentElements_DifferentHashCodes() and in Equals_DifferentElements_False() you will find them yourself ;-)

\$\endgroup\$
2
  • \$\begingroup\$ Oh! I didn't see these ones. I adjusted the properties but not the messages. I rarely do copy/paste so I'm no so good at it ;-) \$\endgroup\$
    – t3chb0t
    Jul 5, 2017 at 4:40
  • 1
    \$\begingroup\$ I'd also add one Assert.IsTrue() to check if x.Right == x.Left to check if implementation is symmetric. \$\endgroup\$ Jul 5, 2017 at 11:00
4
\$\begingroup\$

I don't like GetHashCode_DifferentElements_DifferentHashCodes method, because hashcode collisions happen. And when they do your software should not break. Therefore it is unclear why the test should fail.

Also, I can't come up with a case where null == null returning false makes any sense. :) If it is intentional, then you should probably document this behavior and explain why it works that way.

\$\endgroup\$
8
  • \$\begingroup\$ I consider anything that is not a valid element and not equal as un-equal thus I consider two nulls are not equals. It wouldn't make any sense to have to null names and consider their equality as true. With this assumption or rather adjustment I don't have to filter null values. \$\endgroup\$
    – t3chb0t
    Jul 5, 2017 at 9:34
  • \$\begingroup\$ @t3chb0t, dunno. Don't wanna go all philosophical about the nature of nothingness, I'm just used to the fact that every reference is equal to itself, and null reference is no exception usually. \$\endgroup\$
    – Nikita B
    Jul 5, 2017 at 9:54
  • 2
    \$\begingroup\$ @t3chb0t how's that different? They might be equal (no collisions) or they might not (in case of collision). The only thing you should test is that equal items have equal hashcode (not the other way around!). That is the only thing every hashcode implementation must guarantee when it comes to equality. But you already have a test for that. \$\endgroup\$
    – Nikita B
    Jul 5, 2017 at 10:28
  • 1
    \$\begingroup\$ @t3chb0t: Equality of null/null should be true in the .Net world. You can check it using one of the .Net's default comparer:StringComparer.InvariantCulture.Equals(null, null); \$\endgroup\$
    – JanDotNet
    Jul 5, 2017 at 14:49
  • 1
    \$\begingroup\$ @dfhwze ofcourse, you are right, sorry! I guess I am not fully awake yet :) \$\endgroup\$
    – Nikita B
    Oct 6, 2019 at 8:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.