4

In Microsoft's MSDN Library article on Object.Equals Method (Object), (http://msdn.microsoft.com/en-us/library/bsc2ak47.aspx) an example is presented to demonstrate how to override Equals. It looks like this:

class Point
{
   ... // IEquatable<Point> is not implemented.

   public override bool Equals(Object obj) 
   {
      //Check for null and compare run-time types. 
      if ((obj == null) || ! this.GetType().Equals(obj.GetType())) {
         return false;
      }
      else { 
         Point p = (Point) obj; 
         return (x == p.x) && (y == p.y);
      }   
   }
}

sealed class Point3D: Point 
{
   int z;

   public override bool Equals(Object obj) 
   {
      Point3D pt3 = obj as Point3D;
      if (pt3 == null)
         return false;
      else 
         return base.Equals((Point)obj) && z == pt3.z; // Here!!!
   }
}

In the ensuing documentation, my attention was drawn to the following statement.

(If it is a Point3D object, it is cast to a Point object and passed to the base class implementation of Equals.)

Here, return base.Equals((Point)obj) why bother casting obj into Point?

Update:

I guess it might just be a typo, as I check the .NET 4.0 version document, it is a one-liner:

return base.Equals(obj) && z == ((Point3D)obj).z
colinfang
  • 20,909
  • 19
  • 90
  • 173
  • 1
    Doesn't look like good code to me. The cast means that they get an `InvalidCastException` if `obj` is of an exotic type. Not nice. __Edit:__ Nah, sorry, too fast. They know the type is `Point3D`, so no exception, just silly-looking code. – Jeppe Stig Nielsen Jul 09 '13 at 21:39
  • Maybe it should be passing in `pt3`, which has already been casted, not `obj`. EDIT: or at least just passing in `obj` and let the base implementation handle casting/type checking. – Chris Sinclair Jul 09 '13 at 21:40
  • 1
    I *think* they're trying to make the base class method work despite the `!this.GetType().Equals()` check. (Trying pointlessly. The right way would probably be using `IEquatable` instead of overriding `Object.Equals()` for comparisons in class hierarchies.) – millimoose Jul 09 '13 at 21:47
  • Or, obviously, avoiding crazy hierarchies of data structures. `Point3D` extending from `Point` to save two field definitions seems like bad use of inheritance. – millimoose Jul 09 '13 at 21:51
  • @millimoose However, when comparing two `Point3D` it will still work. In the base class, both `GetType()` and `obj.GetType()` will return `typeof(Point3D)`, so all is good. But it's ugly code. – Jeppe Stig Nielsen Jul 09 '13 at 21:54
  • Maybe they're just trying to make it more explicit that the `base.Equals((Point)obj)` will be check the `Point` fields as though it's a `Point`, _then_ if that passes also check the `Point3D.z` field. Maybe they ("they" as in MSDN authors) _think_ this makes the information/lesson in the MSDN article more articulate or somehow more teachable for implementing `Object.Equals`. – Chris Sinclair Jul 09 '13 at 21:55
  • @ChrisSinclair Maybe, but it's not optimal, and I suspect the "silly" cast will generate some extra IL for no reason. – Jeppe Stig Nielsen Jul 09 '13 at 22:00
  • I fully agree. This is _not_ the best way to implement `Equals`; just speculating _why_ the MSDN article was written the way it is. – Chris Sinclair Jul 09 '13 at 22:02
  • @millimoose It is not recommended to implement `IEquatable` on non-sealed type http://stackoverflow.com/questions/1868316/should-iequatablet-icomparablet-be-implemented-on-non-sealed-classes – colinfang Jul 09 '13 at 22:05
  • @colinfang Interesting thread! I don't agree with its conclusion, though. Just wrote a couple of comments in it. The way I see it, all that thread proves is that when you implement `Equals` you **must** check to see if `GetType() != other.GetType()`. If so, return `false`. – Jeppe Stig Nielsen Jul 09 '13 at 22:25
  • If the passed-in object is anything other than a `Point3d` or derivative thereof, the expression `obj as Point3d` will be null. If `Point3d` inherits from `Point`, then any cast from `Point3d` to `Point` will succeed. The cast doesn't seem dangerous, but it also doesn't really do anything. I think the purpose of the cast may be to allow for the possibility that the base class might have an overload which expects an argument of type `Point`, but such things can be dangerous with types that participate in widening conversions. – supercat Jul 09 '13 at 22:50

3 Answers3

1

When one requires equality-checking, the recommendation today is to implement the IEquatable<T> generic interface. Its Equals(T) method provides type-safety and avoids boxing/unboxing overheads for value types.

If the Point class implemented the IEquatable<Point> interface, it would contain an Equals(Point) method overload, which is more efficient to call than Equals(Object). A plausible justification could be that the type-cast to Point is done pre-emptively in case the said interface gets implemented.

class Point : IEquatable<Point>
{
   public Equals(Point other)
   {
       return other != null && this.x == other.x && this.y == other.y;
   }

   public override bool Equals(Object obj) 
   {
       return this.Equals(obj as Point);
   }
}
Douglas
  • 53,759
  • 13
  • 140
  • 188
  • You should still check if `GetType() == other.GetType()` in the type-safe overload (the interface implementation) when your `class` is not `sealed`! – Jeppe Stig Nielsen Jul 09 '13 at 21:57
  • @JeppeStigNielsen: That's debatable. If the base-class fields of a derived-class instance are equal to the current base-class instance, then the two might still be considered equal. – Douglas Jul 09 '13 at 22:00
  • 1. `Point` is not value type. 2. It is not recommended to implement `IEquatable` on non-sealed class http://stackoverflow.com/questions/1868316/should-iequatablet-icomparablet-be-implemented-on-non-sealed-classes – colinfang Jul 09 '13 at 22:02
  • 1
    Maybe debatable. But you jeopardize the symmetry of the `Equals` "relation". That is, if I derive from your type (you didn't seal it) and introduce the new `z` component, then `jeppes3DPointInstance.Equals(douglasPointInstance)` would surely return `false`. But I can't get your type to behave "correctly" with `douglasPointInstance.Equals(jeppes3DPointInstance)`. – Jeppe Stig Nielsen Jul 09 '13 at 22:04
  • 1
    Inheritable types should generally not implement `IEquatable`, since there's no good pattern for assuring that invoking `IEquatable.Equals(BaseType)` on a derived-type object will yield behavior consistent with its override of `Object.Equals(Object)`, except by having the former method chain to the latter (which would negate all of the performance benefits of implementing the interface in the first place). – supercat Jul 09 '13 at 22:52
  • @supercat: I feel that a good pattern should be possible to support hierarchical equality through polymorphism (like there exists the [Dispose Patten](http://msdn.microsoft.com/en-us/library/b1yfkh5e.aspx)), but I can't seem to come across one at the moment. I might come back to this post in the future. – Douglas Jul 10 '13 at 05:21
  • @Douglas: There is a pattern: `Object.Equals`. There are at least two kinds of equivalence relations which are well-defined for all possible pairs of class objects; since the result of comparing any *class* object to any other is well-defined (an apple shouldn't refuse to compare itself to an orange--it should simply report that it's not equivalent), it makes sense for `Equals` to take a parameter of type `Object`). If a type `T` is not inheritable, there's no need to check the type of an object passed as a `T` parameter, but if it is inheritable, type checking is necessary. – supercat Jul 10 '13 at 14:43
  • @supercat: My argument is that, for inheritable types, the equality between a base-class instance and a derived-class class is domain-dependent. If a derived class does not alter the identity of the base class – say, it simply provides a layer of memoization over the base class's computed properties – then it shouldn't be constrained to be non-equal just because it's a different type. – Douglas Jul 10 '13 at 14:51
  • @Douglas: Checking whether a class object passed as a `T` is in fact an instance of `T` (as opposed to a derivative type) is no cheaper than testing whether an object passed as `Object` is an instance of `T`, so constraining a parameter to be of type `T` doesn't really buy anything. If `IEquatable` included its own `GetHashCode` method, it might be helpful to have it implement a different sort of equivalence from `Object.Equals()` [IMHO, that would be less bad than having types like `Decimal` override `Object.Equals` to mean something other than strict equivalence] but it doesn't. – supercat Jul 10 '13 at 14:52
  • @Douglas: There is nothing wrong with having an `IEqualityComparer` which can regard things of different types as equivalent, but I think `Object.Equals` should represent a fairly strict form of equality which would generally preclude that. I know what you're getting at, and have a pattern for it; I can discuss more in chat if you like. – supercat Jul 10 '13 at 14:56
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/33207/discussion-between-douglas-and-supercat) – Douglas Jul 10 '13 at 14:59
0

There is no point (haha) to casting obj to a Point. You are right that the Point.Equals method will also cast it to a Point. It is redundant.

Jashaszun
  • 9,207
  • 3
  • 29
  • 57
0

In the Point class we are comparing the types:

this.GetType().Equals(obj.GetType())

The Type Point is not equal to Point3D.

Hemario
  • 140
  • 9
  • Seems to me that the Equals function on the base class is no well defined – Hemario Jul 09 '13 at 21:41
  • The cast `(Point)obj` doesn't change the identity of the object being referred. It's just a (useless in this case) reference conversion. The `GetType()` will always return the true runtime type, no matter what compile-time type. Checking if `GetType()` and `obj.GetType()` are the same is a very good thing to do when overriding `Equals` for a non-sealed class type. – Jeppe Stig Nielsen Jul 09 '13 at 21:48