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

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);
}
}
}
 
N

Nicholas Paldino [.NET/C# MVP]

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.
 
J

Jon Skeet [C# MVP]

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?

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

Do so, and it works fine :)
 
W

Whitney Kew

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.
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);
     }
  }
}- Hide quoted text -

- Show quoted text -

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
 
W

Whitney Kew

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?

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

Do so, and it works fine :)

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.
 
J

Jon Skeet [C# MVP]

No, I don't think so. First of all, even if I override GetHashCode(),
my Equals() still never gets called.

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."

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.

It will do if it finds two objects with the same hash code.
 
N

Nicholas Paldino [.NET/C# MVP]

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.
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);
}
}
}- Hide quoted text -

- Show quoted text -

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
 
W

Whitney Kew

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.

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
 
N

Nicholas Paldino [.NET/C# MVP]

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.

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
 
J

Jon Skeet [C# MVP]

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.

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.
 
C

Chris Nahr

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.

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.
 
W

Whitney Kew

[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.

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
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top