IEnumerable<T>.Intersect() not calling my object'sIEquatable<T>.Equals()?

  • Thread starter Thread starter Whitney Kew
  • Start date Start date
W

Whitney Kew

I have a class, MyObject, that implements IEquatable<MyObject>. I'm
trying to get the intersection of two MyObject[] arrays. According to
the documentation for IEnumerable<T>.Intersect(), it's supposed to use
the default equality comparer, EqualityComparer<T>.Default, to compare
values. That, in turn, according to the docs, checks whether type T
implements the System.IEquatable<T> generic interface, and if so
returns an EqualityComparer<T> that uses that implementation.

However, the Intersect() method doesn't seem to be calling
MyObject.Equals(). Here is my source code; the output, I thought,
should return the object represented by MyObject(2). The result I'm
getting is an empty collection. Am I missing something?

using System;
using System.Linq;

namespace Test
{
class MyObject : IEquatable<MyObject>
{
public int Number { get; private set; }

public MyObject(int number)
{
Number = number;
}

public bool Equals(MyObject other)
{
return Number.Equals(other.Number);
}

// ...ToString() for Console.WriteLine()...
}

class Program
{
static void Main(string[] args)
{
MyObject[] objects1 = { new MyObject(1), new MyObject(2) };
MyObject[] objects2 = { new MyObject(2), new MyObject(3) };
foreach (MyObject obj in objects1.Intersect(objects2))
Console.WriteLine(obj);
}
}
}
 
Whitney,

I've run this locally, and I definitely think that this is a bug. The
interesting thing is, if I make the call to the static Default property on
EqualityComparer<MyObject> and pass that to the Intersect method, it ^still^
doesn't call the equality comparer.

I would suggest filing a bug at the Microsoft Connect site. I actually
tried to debug through the framework source, but unfortunately,
System.Core.dll is not able to be stepped through, so I couldn't get far
with the Intersect method.
 
Whitney Kew said:
I have a class, MyObject, that implements IEquatable<MyObject>. I'm
trying to get the intersection of two MyObject[] arrays. According to
the documentation for IEnumerable<T>.Intersect(), it's supposed to use
the default equality comparer, EqualityComparer<T>.Default, to compare
values. That, in turn, according to the docs, checks whether type T
implements the System.IEquatable<T> generic interface, and if so
returns an EqualityComparer<T> that uses that implementation.

However, the Intersect() method doesn't seem to be calling
MyObject.Equals(). Here is my source code; the output, I thought,
should return the object represented by MyObject(2). The result I'm
getting is an empty collection. Am I missing something?
Click to expand...

Yup - you haven't overridden GetHashCode, which is used by equality
comparers.

Do so, and it works fine :)
 
Whitney,

    I've run this locally, and I definitely think that this is a bug.  The
interesting thing is, if I make the call to the static Default property on
EqualityComparer<MyObject> and pass that to the Intersect method, it ^still^
doesn't call the equality comparer.

    I would suggest filing a bug at the Microsoft Connect site.  I actually
tried to debug through the framework source, but unfortunately,
System.Core.dll is not able to be stepped through, so I couldn't get far
with the Intersect method.

--
          - Nicholas Paldino [.NET/C# MVP]
          - (e-mail address removed)




I have a class, MyObject, that implements IEquatable<MyObject>.  I'm
trying to get the intersection of two MyObject[] arrays.  According to
the documentation for IEnumerable<T>.Intersect(), it's supposed to use
the default equality comparer, EqualityComparer<T>.Default, to compare
values.  That, in turn, according to the docs, checks whether type T
implements the System.IEquatable<T> generic interface, and if so
returns an EqualityComparer<T> that uses that implementation.
Click to expand...
However, the Intersect() method doesn't seem to be calling
MyObject.Equals().  Here is my source code; the output, I thought,
should return the object represented by MyObject(2).  The result I'm
getting is an empty collection.  Am I missing something?
Click to expand...
using System;
using System.Linq;
Click to expand...
namespace Test
{
  class MyObject : IEquatable<MyObject>
  {
     public int Number { get; private set; }
Click to expand...
     public MyObject(int number)
     {
        Number = number;
     }
Click to expand...
     public bool Equals(MyObject other)
     {
        return Number.Equals(other.Number);
     }
Click to expand...
     // ...ToString() for Console.WriteLine()...
  }
Click to expand...
  class Program
  {
     static void Main(string[] args)
     {
        MyObject[] objects1 = { new MyObject(1), new MyObject(2) };
        MyObject[] objects2 = { new MyObject(2), new MyObject(3) };
        foreach (MyObject obj in objects1.Intersect(objects2))
           Console.WriteLine(obj);
     }
  }
}- Hide quoted text -
Click to expand...

- Show quoted text -
Click to expand...

Hi Nicholas, thank you for trying it out and for your input - I'll
file a bug report.

Incidentally, besides Intersect, this also occurs with Except(),
Distinct(), and Union().

Whitney
 
Whitney Kew said:
I have a class, MyObject, that implements IEquatable<MyObject>.  I'm
trying to get the intersection of two MyObject[] arrays.  According to
the documentation for IEnumerable<T>.Intersect(), it's supposed to use
the default equality comparer, EqualityComparer<T>.Default, to compare
values.  That, in turn, according to the docs, checks whether type T
implements the System.IEquatable<T> generic interface, and if so
returns an EqualityComparer<T> that uses that implementation.
Click to expand...
However, the Intersect() method doesn't seem to be calling
MyObject.Equals().  Here is my source code; the output, I thought,
should return the object represented by MyObject(2).  The result I'm
getting is an empty collection.  Am I missing something?
Click to expand...

Yup - you haven't overridden GetHashCode, which is used by equality
comparers.

Do so, and it works fine :)
Click to expand...

No, I don't think so. First of all, even if I override GetHashCode(),
my Equals() still never gets called. Secondly, that's not what the
docs say. The docs for Intersect() say that "the default equality
comparer, EqualityComparer<T>.Default, is used to compare values," and
the docs for EqualityComparer<T>.Default say that "the Default
property checks whether type T implements the System.IEquatable<T>
generic interface and if so returns an EqualityComparer<T> that uses
that implementation."

Therefore, since my class satisfies the requirement of implementing
System.IEquatable<T>, the framework should be calling my Equals()
method.
 
No, I don't think so. First of all, even if I override GetHashCode(),
my Equals() still never gets called.
Click to expand...

How did you override GetHashCode()? When I override it to return
Number, Equals was definitely being called.
Secondly, that's not what the
docs say. The docs for Intersect() say that "the default equality
comparer, EqualityComparer<T>.Default, is used to compare values," and
the docs for EqualityComparer<T>.Default say that "the Default
property checks whether type T implements the System.IEquatable<T>
generic interface and if so returns an EqualityComparer<T> that uses
that implementation."
Click to expand...

Yes, and how would you expect that EqualityComparer to implement
IEqualityComparer.GetHashCode(T t)? It will call GetHashCode on the
value passed to it, if it's not null.

Basically, the problem is that IEquatable<T> doesn't explicitly list
GetHashCode as a member to be overridden - but you should always
or the default said:
Therefore, since my class satisfies the requirement of implementing
System.IEquatable<T>, the framework should be calling my Equals()
method.
Click to expand...

It will do if it finds two objects with the same hash code.
 
Bleh, I can't believe I didn't see the lack of override for GetHashCode.
Jon is right, you have to override GetHashCode if you are going to override
Equals.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

Whitney,

I've run this locally, and I definitely think that this is a bug. The
interesting thing is, if I make the call to the static Default property on
EqualityComparer<MyObject> and pass that to the Intersect method, it
^still^
doesn't call the equality comparer.

I would suggest filing a bug at the Microsoft Connect site. I actually
tried to debug through the framework source, but unfortunately,
System.Core.dll is not able to be stepped through, so I couldn't get far
with the Intersect method.

--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)




I have a class, MyObject, that implements IEquatable<MyObject>. I'm
trying to get the intersection of two MyObject[] arrays. According to
the documentation for IEnumerable<T>.Intersect(), it's supposed to use
the default equality comparer, EqualityComparer<T>.Default, to compare
values. That, in turn, according to the docs, checks whether type T
implements the System.IEquatable<T> generic interface, and if so
returns an EqualityComparer<T> that uses that implementation.
Click to expand...
However, the Intersect() method doesn't seem to be calling
MyObject.Equals(). Here is my source code; the output, I thought,
should return the object represented by MyObject(2). The result I'm
getting is an empty collection. Am I missing something?
Click to expand...
using System;
using System.Linq;
Click to expand...
namespace Test
{
class MyObject : IEquatable<MyObject>
{
public int Number { get; private set; }
Click to expand...
public MyObject(int number)
{
Number = number;
}
Click to expand...
public bool Equals(MyObject other)
{
return Number.Equals(other.Number);
}
Click to expand...
// ...ToString() for Console.WriteLine()...
}
Click to expand...
class Program
{
static void Main(string[] args)
{
MyObject[] objects1 = { new MyObject(1), new MyObject(2) };
MyObject[] objects2 = { new MyObject(2), new MyObject(3) };
foreach (MyObject obj in objects1.Intersect(objects2))
Console.WriteLine(obj);
}
}
}- Hide quoted text -
Click to expand...

- Show quoted text -
Click to expand...

Hi Nicholas, thank you for trying it out and for your input - I'll
file a bug report.

Incidentally, besides Intersect, this also occurs with Except(),
Distinct(), and Union().

Whitney
 
How did you override GetHashCode()? When I override it to return
Number, Equals was definitely being called.


Yes, and how would you expect that EqualityComparer to implement
IEqualityComparer.GetHashCode(T t)? It will call GetHashCode on the
value passed to it, if it's not null.

Basically, the problem is that IEquatable<T> doesn't explicitly list
GetHashCode as a member to be overridden - but you should always


It will do if it finds two objects with the same hash code.
Click to expand...

Bleh! Pardon me while I finish off the last bite of my humble pie
over here. Jon and Nicholas, thank you for setting it straight. Jon,
I did what you said, and sure enough, things work fine. (I initially
had my GetHashCode() call base.GetHashCode(), and not surprisingly,
things didn't work.) It's certainly a documentation bug, because I
had no idea that the GetHashCode() implementation was necessary - I
didn't even know that I had to be aware of that salient point.
Subsequently, I've now read item 10 in Bill Wagner's "Effective C#",
"Understand The Pitfalls Of GetHashCode()", in an effort to learn how
to implement GetHashCode() correctly.

So, again, thanks for your input; I imagine this point has caused
confusion before.

Whitney
 
Whitney,

I don't know if it is a documentation bug, but it probably is an
assumption on their part. A core tenant of overriding Equals is overriding
GetHashCode to produce the same hash code for when Equals returns true.


How did you override GetHashCode()? When I override it to return
Number, Equals was definitely being called.


Yes, and how would you expect that EqualityComparer to implement
IEqualityComparer.GetHashCode(T t)? It will call GetHashCode on the
value passed to it, if it's not null.

Basically, the problem is that IEquatable<T> doesn't explicitly list
GetHashCode as a member to be overridden - but you should always


It will do if it finds two objects with the same hash code.
Click to expand...

Bleh! Pardon me while I finish off the last bite of my humble pie
over here. Jon and Nicholas, thank you for setting it straight. Jon,
I did what you said, and sure enough, things work fine. (I initially
had my GetHashCode() call base.GetHashCode(), and not surprisingly,
things didn't work.) It's certainly a documentation bug, because I
had no idea that the GetHashCode() implementation was necessary - I
didn't even know that I had to be aware of that salient point.
Subsequently, I've now read item 10 in Bill Wagner's "Effective C#",
"Understand The Pitfalls Of GetHashCode()", in an effort to learn how
to implement GetHashCode() correctly.

So, again, thanks for your input; I imagine this point has caused
confusion before.

Whitney
 
Nicholas Paldino said:
I don't know if it is a documentation bug, but it probably is an
assumption on their part. A core tenant of overriding Equals is overriding
GetHashCode to produce the same hash code for when Equals returns true.
Click to expand...

While that's true, you don't need to override object.Equals in order to
implement IEquatable<T> - you only need to provide the strongly typed
method. Hmm.
 
While that's true, you don't need to override object.Equals in order to
implement IEquatable<T> - you only need to provide the strongly typed
method. Hmm.
Click to expand...

Interesting discussion. Calling GetHashCode before Equals is an
optimization that makes sense when you think about it, but I would not
have expected that in a method that doesn't actually do any hashing.

As you note GetHashCode is not a member of the IEquatable<T>
interface, and I would guess that many classes out there in the wild
override Object.Equals but not GetHashCode since the authors don't
expect the objects to be used in hashtables. Prior to .NET 3.5 you
simply didn't need GetHashCode outside of hashtables, and the
incorrect GetHashCode in the original .NET release didn't help.

I know FxCop warns about a missing GetHashCode override but that is
really not enough if a non-hashtable Framework method simply breaks
when GetHashCode has not been overridden. Sigh, I wish Microsoft
would have provided a *useful* default implementation. It's really a
BCL defect that the behavior Whitney encountered is even possible. The
default hash function can be suboptimal but the one provided by MS
does not even obey the "must-have" properties listed in the method's
own documentation! GetHashCode should simply return zero by default,
not some stupid magic value related to the object address.

I think Whitney should definitely file a bug report regarding the MSDN
documentation, there needs to be a note about this behavior. And
while they're at it they should also fix the GetHashCode documentation
which still only talks about using objects in hashtables.
 
[snip]
I know FxCop warns about a missing GetHashCode override but that is
really not enough if a non-hashtable Framework method simply breaks
when GetHashCode has not been overridden.  Sigh, I wish Microsoft
would have provided a *useful* default implementation.  It's really a
BCL defect that the behavior Whitney encountered is even possible. The
default hash function can be suboptimal but the one provided by MS
does not even obey the "must-have" properties listed in the method's
own documentation!  GetHashCode should simply return zero by default,
not some stupid magic value related to the object address.

I think Whitney should definitely file a bug report regarding the MSDN
documentation, there needs to be a note about this behavior.  And
while they're at it they should also fix the GetHashCode documentation
which still only talks about using objects in hashtables.
Click to expand...

Already done: http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=324047

I initially wrote it up before I realized that I was incorrectly
implementing GetHashCode(), and I note this in a subsequent comment in
the bug writeup. :)

Whitney
 
Back
Top