Implementation of Equality in NUnit

[Charlie Poole]

Hi All,

This bug: https://bugs.launchpad.net/nunitv2/+bug/676560
asks that we use IEquatable<T>.Equals to test for equality
on objects that implement the interface.

My first reaction was "Yes, we should do that" and I scheduled
the bug to be fixed for both 2.6 and 3.0.

As I'm looking at it more closely, however, I wonder if we should
actually bother. I'll lay out my thinking here and ask for your
opinions on the matter.

There are four cases to be dealt with:

1. Object.Equals is not overridden and IEquatable<T> is not
implemented. We use object equality, which is the only thing
available to us.

2. Object.Equals is overridden and IEquatable<T> is not implemented.
We handle this correctly now.

3. Object.Equals is not overridden but IEquatable<T> is implemented.
We use object equality in this case the bug report would call for us to
use IEquatable<T>.Equals. Note, however, that this is a pathological
case: good practice calls for overriding Object.Equals as well in this
case, using the IEquatable<T>.Equals implementation.

4. Object.Equals is overridden and IEquatable<T> is implemented.
There are two sub-cases here:
4.1 The implementations are consistent. We handle this correctly.
4.2 The implementations are inconsistent. This is a user error, which
we handle by using the Object.Equals override.

In summary, I'm now thinking that this change is only needed to deal
with incorrect implementations of equality and I see no need to do it.

What do you all think?

Charlie
We test for object equality

[Simone Busoli]

Hi Charlie, your reasoning makes sense but after thinking about it a bit in case 3 it would probably be sensible to use IEquatable because that is what the compiler would choose if you called a.Equals(b).

--
You received this message because you are subscribed to the Google Groups "NUnit-Discuss" group.
To post to this group, send email to nunit-...@googlegroups.com.
To unsubscribe from this group, send email to nunit-discus...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/nunit-discuss?hl=en.

[Charlie Poole]

Hi Simone,

Only if a and b were both of the Type T. If either were an object then
I believe that Object.Equals would be used. It will also be used in
certain collection and dictionary operations. That's why it's bad to
fail to override Object.Equals.

That said, I guess we could give the user a pass and use the
IEquatable<T> implementation if it exists. It's a bit of an effort,
however, and it might be better to just make the user override
Object.Equals like you're supposed to.

Charlie

[Simone Busoli]

On Fri, Oct 21, 2011 at 21:00, Charlie Poole <nuni...@gmail.com> wrote:

Hi Simone,

Only if a and b were both of the Type T.

Yes, that's what we're talking about, if they weren't you cannot call the implementation of IEquatable anyway and you'd fallback on object.Equals.

 

If either were an object then
I believe that Object.Equals would be used. It will also be used in
certain collection and dictionary operations. That's why it's bad to
fail to override Object.Equals.

I'm not saying it's not bad, just that doing what the compiler would do is probably more intuitive.

 

That said, I guess we could give the user a pass and use the
IEquatable<T> implementation if it exists. It's a bit of an effort,
however, and it might be better to just make the user override
Object.Equals like you're supposed to.

My considerations didn't take the effort of implementing it into account, of course :)

[Charlie Poole]

Hi Simone,

On Fri, Oct 21, 2011 at 12:05 PM, Simone Busoli <simone...@gmail.com> wrote:
> On Fri, Oct 21, 2011 at 21:00, Charlie Poole <nuni...@gmail.com> wrote:
>>
>> Hi Simone,
>>
>> Only if a and b were both of the Type T.
>
> Yes, that's what we're talking about, if they weren't you cannot call the
> implementation of IEquatable anyway and you'd fallback on object.Equals.

True

>>
>> If either were an object then
>> I believe that Object.Equals would be used. It will also be used in
>> certain collection and dictionary operations. That's why it's bad to
>> fail to override Object.Equals.
>
> I'm not saying it's not bad, just that doing what the compiler would do is
> probably more intuitive.

I see your point.

>>
>> That said, I guess we could give the user a pass and use the
>> IEquatable<T> implementation if it exists. It's a bit of an effort,
>> however, and it might be better to just make the user override
>> Object.Equals like you're supposed to.
>
> My considerations didn't take the effort of implementing it into account, of
> course :)

Well, we have to consider that, but it's separate from the question of what's
the best approach.

Charlie

[Simone Busoli]

Let's make a deal. If you free me from the bug 697329 I'll do this instead :)

[Charlie Poole]

It's a deal! I changed the bug assignments and I'll send you a note
offline about where
to find this in the code and a few thoughts on it that I have.

Charlie

[Rafael Teixeira]

I may be dreaming awake, but I think that NUnit would be a fantastic assistant to detect a bad equality-comparison implementation, if it would give an exception if IEquatable is implemented and Equals is not overriden, and also when both exist if both diverge on conclusion. 

It may be outside the scope of a testing framework but it would be tremendously useful to make sure an all-green scenario isn't hiding some hard to diagnose bugs.

Just my dreamy 2 cents,

Rafael "Monoman" Teixeira
---------------------------------------
"The most exciting phrase to hear in science, the one that heralds new discoveries, is not 'Eureka!' (I found it!) but 'That's funny ...'"
Isaac Asimov
US science fiction novelist & scholar (1920 - 1992)

[Brad Stiles]

On Fri, Oct 21, 2011 at 2:33 PM, Charlie Poole <nuni...@gmail.com> wrote:
> Hi All,
>

> 3. Object.Equals is not overridden but IEquatable<T> is implemented.
> We use object equality in this case the bug report would call for us to
> use IEquatable<T>.Equals. Note, however, that this is a pathological
> case: good practice calls for overriding Object.Equals as well in this
> case, using the IEquatable<T>.Equals implementation.

OK first, I am *not* claiming that there is a valid use case for what
you call "pathological" here. However, I've been doing this
programming gig for almost as long as some others here, and I have yet
to find a case where "good practice" absolutely always equals "never
do anything else".

Were I doing this (and I'll freely admit the potential
short-sightedness of my view), I would probably use
IEquatable<T>.Equals if available, and Object.Equals as a fallback.
That seems to me to be a simple rule, easily documented and easily
followed.

I *might*, if I were feeling more cooperative than usual, implement a
method for each, in order to facilitate testing of the "good
practice", but it's seems easy enough to me to implement tests that
just use Assert.True and Assert.False with the appropriate conditions
that use whichever one is being tested.

> 4. Object.Equals is overridden and IEquatable<T> is implemented.
> There are two sub-cases here:
>  4.1 The implementations are consistent. We handle this correctly.
>  4.2 The implementations are inconsistent. This is a user error, which
>         we handle by using the Object.Equals override.

Showing my ignorance, how do you determine that the implementation is
inconsistent? Do you execute both and compare the results?

Brad

[Charlie Poole]

Hi Brad & Rafael,

Rather than automatically running such tests, what if there were a way for the
user to express it as a test? I don't see this coming very soon, since we have
other priorities right now, but perhaps a future version could support plugins
that test certain standard notions like "All versions of equality
between two types
must give the same result."

I imagine this implemented as a sort of higher level theory - a set of
theories in fact.
Users could create their own tests or use tests built into NUnit or
distributed by others.

Something to think about?

Charlie

[Simone Busoli]

Hi, good ideas here but honestly - although cool to be implemented as an addin - I don't see it fit very well with NUnit.

There are already static code analyzers which can do this. JetBrains R# already does something similar while editing in VS, although not in this specific case, but for instance it warns if you overload Equals but not GetHashCode or the other way around. Then I think FXCop of the code analysis feature integrated in VS can do this.

[Charlie Poole]

I'm not ready to spend time on it yet, but I think you can consider
static (or dynamic) analysis
of the code as a kind of test and I can imagine NUnit running those
tests. Of course, the
actual engine for the test might be provided by some other product but
i can imagine
the NUnit runner being used to coordinate these "tests" with other
kinds of tests.

To put it in perspective, I think of stuff like this as part of NUnit
4 or 5, for which I
may not even be around. :-)

Charlie

[David Schmitt]

On 21.10.2011 20:33, Charlie Poole wrote:
> Hi All,
>
> This bug: https://bugs.launchpad.net/nunitv2/+bug/676560
> asks that we use IEquatable<T>.Equals to test for equality
> on objects that implement the interface.
>
> My first reaction was "Yes, we should do that" and I scheduled
> the bug to be fixed for both 2.6 and 3.0.
>
> As I'm looking at it more closely, however, I wonder if we should
> actually bother. I'll lay out my thinking here and ask for your
> opinions on the matter.
>
> There are four cases to be dealt with:

[...]

> In summary, I'm now thinking that this change is only needed to deal
> with incorrect implementations of equality and I see no need to do it.
>
> What do you all think?

NUnit already distinguishes between Is.Same and Is.EqualTo . I'm
wondering whether there should be an Is.EquatableTo (and/or an
Is.ConsistentlyEqualTo with additional checks [T.Equals, Object.Equals,
T.operator==, IEquatable<T>, IComparable<T>, T.operator<, etc] ?

I'm not to judge the amount of magic necessary within NUnit, just
trowing out some ideas.

Best Regards, David

[Brad Stiles]

> Rather than automatically running such tests, what if there were a way for the
> user to express it as a test? I don't see this coming very soon, since we have
> other priorities right now, but perhaps a future version could support plugins
> that test certain standard notions like "All versions of equality
> between two types must give the same result."

Well, as the guy *not* doing the work, I'm certainly not going to stop
you from implementing such a thing. :)

However, in this particular case, couldn't it be said that such a
facility already exists, if not in the precise form you suggest? It
certainly looks to me like a single abstract class would suffice for
every IEquatable/Equals consistency test for every object where such
are implemented. It shouldn't be too onerous a task to set one of
those up, even for someone like me. Tedious maybe, but not
particularly difficult.

Of course, I've made those types of assertions before, and they tend
to illustrate how badly my quest for omniscience is actually
progressing...

Brad

[Charlie Poole]

I'd love to see somebody try to put this together because it would
point out where NUnit needs changes to facilitate use of the abstract
fixture pattern. I think there are a few, although it can pretty much
be done now as you say.

My notion is that we should make it easy to create and redistribute
such fixtures. I don't think it would take a lot of work in NUnit
itself but I suspect there are a few things to do, particularly if we
would like these fixtures to work in a way similar to how theories now
work.

Charlie

[vineet chauhan]

Please help me. I want to run nester testing tool but i am unable to open VS project in that tool .the option is not highlisted.everything is blocked from my end due to this.any body have any idea

[Simone Busoli]

Charlie, if I recall correctly Greg Young some time ago wrote something (an addin?) to do something similar, not specific to equality though.

[Torbjörn Gyllebring]

I think you're thinking about
http://codebetter.com/gregyoung/2010/10/29/hei-hei-grensesnitt/

[Simone Busoli]

Exactly, thanks Torbjorn. Not an ad-hoc solution but something which would work well in this case, if you took care of implementing the specs for IEquatable.

[Charlie Poole]

That's the sort of thing I was thinking of. NUnit could distribute
some key implementations while others could be distributed as addins
or plugins.

Charlie