CODEWITHSUNDEEP
c#.net
redcodefinal
redcodefinal

Reputation: 909

HashSet doesn't remove dupes with overridden equality

I have a class Pair, which needs to have an overridden equality operator due to the two values it stores being interchangeable, the code is this

Pair.cs

public class Pair
    {
        protected bool Equals(Pair other)
        {
            return (Equals(A, other.A) && Equals(B, other.B)) || (Equals(A, other.B) && Equals(B, other.A));
        }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            if (ReferenceEquals(this, obj)) return true;
            return obj.GetType() == this.GetType() && Equals((Pair) obj);
        }

        public override int GetHashCode()
        {
            unchecked
            {
                return ((A != null ? A.GetHashCode() : 0)*397) ^ (B != null ? B.GetHashCode() : 0);
            }
        }

        public readonly Collision A, B;

        public Pair(Collision a, Collision b)
        {
            A = a;
            B = b;
        }

        public static bool operator ==(Pair a, Pair b)
        {
            if (ReferenceEquals(a, b))
                return true;

            if ((object)a == null || (object)b == null)
                return false;

            return (a.A == b.A && a.B == b.B) || (a.A == b.B && a.B == b.A);
        }

        public static bool operator !=(Pair a, Pair b)
        {
            return !(a == b);
        }
    }

I had ReSharper add the Equals and GetHashCode methods, I know Equals is ok, but is GetHashCode outputting the correct value?

I have a HashSet<Pair> that I use to store the pairs, and I need to ensure there are no duplications in this list, however when I add the pairs to a HashSet it doesn't remove the duplicates.

Just for clarification, this would be a duplicate:

Pair a = new Pair(objecta, objectb);
Pair b = new Pair(objectb, objecta);
HashSet<Pair> pairs = new HashSet<Pair>();
pairs.Add(a);
pairs.Add(b);
return pairs.Count(); //Count() returns 2 when it should be 1!

Upvotes: 2

Views: 284

Answers (2)

dee-see
dee-see

Reputation: 24078

Your GetHashCode implementation does not return the same value for (A, B) and (B, A). The HashSet checks if it already contains a given hash at the moment of insertion. If it doesn't, then the object will be considered new.

If you correct your GetHashCode, then at the moment of inserting the second Pair the HashSet will see that the hash code already exists and verify equality with the other objects sharing the same hash code.

Upvotes: 3

lightbricko
lightbricko

Reputation: 2709

Just remove this: *397

If they are considered duplicates, they must return the same int from GetHashCode(). (However, if they are not considered duplicates they are still allowed to return the same int, that will only affect performance).

Upvotes: 2

Related Questions