0

I am trying to get a list of distinct items from a custom collection, however the comparison seems to be getting ignored as I keep getting duplicates appearing in my list. I have debugged the code and I can clearly see that the values in the list that I am comparing are equal...

NOTE: The Id and Id2 values are strings

Custom Comparer:

public class UpsellSimpleComparer : IEqualityComparer<UpsellProduct>
{
    public bool Equals(UpsellProduct x, UpsellProduct y)
    {
        return x.Id == y.Id && x.Id2 == y.Id2;
    }

    public int GetHashCode(UpsellProduct obj)
    {
        return obj.GetHashCode();
    }
}

Calling code:

var upsellProducts = (Settings.SelectedSeatingPageGuids.Contains(CurrentItem.ID.ToString())) ?
                              GetAOSUpsellProducts(selectedProductIds) : GetGeneralUpsellProducts(selectedProductIds);

// we use a special comparer here so that same items are not included
var comparer = new UpsellSimpleComparer();
return upsellProducts.Distinct(comparer);
Matthew Pigram
  • 1,400
  • 3
  • 25
  • 65

2 Answers2

4

Most likely UpsellProduct has default implementation of GetHashCode that returns unique value for each instance of reference type.

To fix - either implement one correctly in UpsellProduct or in comparer.

public class UpsellSimpleComparer : IEqualityComparer<UpsellProduct>
{
   public bool Equals(UpsellProduct x, UpsellProduct y)
   {
       return x.Id == y.Id && x.Id2 == y.Id2;
   }

   // sample, correct GetHashCode is a bit more complex
   public int GetHashCode(UpsellProduct obj)
   {
      return obj.Id.GetHashCode() ^ obj.Id2.GetHashCode();
   }

}

Note for better code to compute combined GetHashCode check Concise way to combine field hashcodes? and Is it possible to combine hash codes for private members to generate a new hash code?

Community
  • 1
  • 1
Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
  • this was the problem, why is the default implementation of `GetHashCode()` unique for each instance of the same type? – Matthew Pigram Jan 07 '15 at 22:28
  • 1
    @MatthewPigram because by default 2 unique instances of the same type are not "Equal" - so having `GetHashCode` to return different value matches behavior of "Equal" and let Dictionary quickly check hash codes to rule out non-equal objects (note that this is the only reason `GetHashCode` exists - so it better be useful for it). – Alexei Levenkov Jan 08 '15 at 01:06
  • Does this mean then that if I had 2 instances with exactly the same data, checking for equality would result in a false result? Or does the default hashcode get generated using class members like your solution above? – Matthew Pigram Jan 08 '15 at 02:41
  • @MatthewPigram yes, check out http://stackoverflow.com/questions/17573720/should-an-override-of-equals-on-a-reference-type-always-mean-value-equality, http://stackoverflow.com/questions/1009394/c-sharp-value-type-equals-method-why-does-the-compiler-use-reflection, http://stackoverflow.com/questions/12123512/why-anonymous-types-equals-implementation-compares-fields (and links there) - value/anonymous types have Equals/GetHashCode pair generated so it take into account all field, reference types don't. – Alexei Levenkov Jan 08 '15 at 02:51
1

Your GetHashCode() doesn't return the same values even two UpsellProduct instances are consider equals by your Equals() method.

Use something like this to reflect the same logic instead.

public int GetHashCode(UpsellProduct obj)
{
    return obj.Id.GetHashCode() ^ obj.Id2.GetHashCode();
}
Fung
  • 3,508
  • 2
  • 26
  • 33