Union-ing two custom classes returns duplicates

Question

I have two custom classes, ChangeRequest and ChangeRequests, where a ChangeRequests can contain many ChangeRequest instances.

public class ChangeRequests : IXmlSerializable, ICloneable, IEnumerable<ChangeRequest>,
    IEquatable<ChangeRequests> { ... }

public class ChangeRequest : ICloneable, IXmlSerializable, IEquatable<ChangeRequest>
    { ... }

I am trying to do a union of two ChangeRequests instances. However, duplicates do not seem to be removed. My MSTest unit test is as follows:

var cr1 = new ChangeRequest { CRID = "12" };
var crs1 = new ChangeRequests { cr1 };
var crs2 = new ChangeRequests
               {
                   cr1.Clone(),
                   new ChangeRequest { CRID = "34" }
               };
Assert.AreEqual(crs1[0], crs2[0], "First CR in both ChangeRequests should be equal");
var unionedCRs = new ChangeRequests(crs1.Union<ChangeRequest>(crs2));
ChangeRequests expected = crs2.Clone();
Assert.AreEqual(expected, unionedCRs, "Duplicates should be removed from a Union");

The test fails in the last line, and unionedCRs contains two copies of cr1. When I tried to debug and step through each line, I had a breakpoint in ChangeRequest.Equals(object) on the first line, as well as in the first line of ChangeRequest.Equals(ChangeRequest), but neither were hit. Why does the union contain duplicate ChangeRequest instances?

Edit: as requested, here is ChangeRequests.Equals(ChangeRequests):

public bool Equals(ChangeRequests other)
{
    if (ReferenceEquals(this, other))
    {
        return true;
    }

    return null != other && this.SequenceEqual<ChangeRequest>(other);
}

And here's ChangeRequests.Equals(object):

public override bool Equals(object obj)
{
    return Equals(obj as ChangeRequests);
}

Edit: I overrode GetHashCode on both ChangeRequest and ChangeRequests but still in my test, if I do IEnumerable<ChangeRequest> unionedCRsIEnum = crs1.Union<ChangeRequest>(crs2);, unionedCRsIEnum ends up with two copies of the ChangeRequest with CRID 12.

Edit: something has to be up with my Equals or GetHashCode implementations somewhere, since Assert.AreEqual(expected, unionedCRs.Distinct(), "Distinct should remove duplicates"); fails, and the string representations of expected and unionedCRs.Distinct() show that unionedCRs.Distinct() definitely has two copies of CR 12.

Answer 1

Make sure your GetHashCode implementation is consistent with your Equals - the Enumerable.Union method does appear to use both.

You should get a warning from the compiler if you've implemented one but not the other; it's still up to you to make sure that both methods agree with each other. Here's a convenient summary of the rules: Why is it important to override GetHashCode when Equals method is overridden?

Answer 2

I don't believe that Assert.AreEqual() examines the contents of the sequence - it compares the sequence objects themselves, which are clearly not equal.

What you want is a SequenceEqual() method, that will actually examine the contents of two sequences. This answer may help you. It's a response to a similar question, that describes how to compare to IEnumerable<> sequences.

You could easily take the responder's answer, and create an extension method to make the calls look more like assertions:

public static class AssertionExt
{
  public static bool AreSequencesEqual<T>( IEnumerable<T> expected, 
                                           IEnumerable<T> sequence )
  {
    Assert.AreEqual(expected.Count(), sequence .Count()); 

    IEnumerator<Token> e1 = expected.GetEnumerator(); 
    IEnumerator<Token> e2 = sequence .GetEnumerator(); 

    while (e1.MoveNext() && e2.MoveNext()) 
    { 
        Assert.AreEqual(e1.Current, e2.Current); 
    }
  }
}

Alternatively you could use SequenceEqual(), to compare the sequences, realizing that it won't provide any information about which elements are not equal.

Answer 3

As LBushkin says, Assert.AreEqual will just call Equals on the sequences.

You can use the SequenceEqual extension method though:

Assert.IsTrue(expected.SequenceEqual(unionedCRs));

That won't give much information if it fails, however.

You may want to use the test code we wrote for MoreLINQ which was sequence-focused - if the sequences aren't equal, it will specify in what way they differ. (I'm trying to get a link to the source file in question, but my network connection is rubbish.)