IEquatable <T> checks the link
I have a class that creates IEquatable<T> . Do I need to do a refrence check in Equals() or take care of it as part of the framework?
class Foo : IEquatable<Foo> { int value; Bar system; bool Equals(Foo other) { return this == other || ( value == other.value && system.Equals(other.system) ); } } In the above example, is this==other operator redundant or necessary?
Update 1
I understand that I need to fix the code as follows:
bool Equals(Foo other) { if( other==null ) { return false; } if( object.ReferenceEquals(this, other) ) { return true; } //avoid recursion return value == other.value && system.Equals(other.system); } Thanks for answers.
I think this is necessary. Verification is the first, quick step that you can perform when comparing objects.
In the example you specified, make sure the other is not null before accessing its values.
bool Equals(Foo other) { if(other == null) return false; if(this == other) return true; // Custom comparison logic here. } Typically, optimization would be a strange Equals implementation that would fail without it. Therefore, I would not consider it necessary, but he did not “care” within the framework. ”I am a cheap optimization to achieve, so it’s usually worth turning it on.
Note that if you also overload == , then you should use object.ReferenceEquals to perform these comparisons.
Be careful. I would really strongly oppose this, because if you ever want to overload the == operator for your type Foo in terms of Equals (as is usually done in my experience), you will end up with infinite recursion.
To illustrate what I mean, here is a general implementation of == in terms of Equals :
public static bool operator ==(Foo x, Foo y) { // Will overflow the stack if Equals uses ==: return !ReferenceEquals(x, null) && x.Equals(y); } However, I can fully agree with Jon point that it might be advisable to use ReferenceEquals instead.
This is not necessary in the sense that it may not be necessary for correctness, but the structure, of course, will not “take care of it”, therefore it may be useful to introduce, as a rule, for performance reasons.
One point: if the implementation is completed EqualityComparer<T>.Default , it does not enter the user code if one or both arguments are null , so in this case it performs some measure of link checking (if not the full ReferenceEquals(x, y) ).
public override bool Equals(T x, T y) { if (x != null) { return ((y != null) && x.Equals(y)); } if (y != null) { return false; } return true; } To disable the topic, your selection method has several problems with the ability to delete names ( other may be null , this.system may be null ).
I would write your method something like:
public bool Equals(Foo other) { if(other == null) return false; if(other == this) return true; return value == other.value && EqualityComparer<Bar>.Default.Equals(bar, other.bar) // You don't *have to* go down the EqualityComparer route // but you do need to make sure you don't dereference null. }
Also remember to override GetHashCode whenever you write your own matching comparisons.
I think you need to check this == other , because you define your own equals . If you do not want it to be checked with a pointer, you write your own IEquatable .
IEquatable<T> is the interface; implementation depends on the contractor.
Since you are implementing the interface, you are responsible for all the expected actions defined by the interface.