Question

In Microsoft's MSDN Library article on IEquatable<T>.Equals Method, (http://msdn.microsoft.com/en-us/library/ms131190.aspx) an example is presented to demonstrate how to override Equals and Equality operator. It looks like this:

public class Person : IEquatable<Person>
{
   private string uniqueSsn;
   private string lName;

   public bool Equals(Person other) 
   {
      if (other == null) 
         return false;

      if (this.uniqueSsn == other.SSN)
         return true;
      else 
         return false;
   }

   public override bool Equals(Object obj)
   {
      if (obj == null) 
         return false;

      Person personObj = obj as Person;
      if (personObj == null)
         return false;
      else    
         return Equals(personObj);   
   }   

   public static bool operator == (Person person1, Person person2)
   {
      if ((object)person1 == null || ((object)person2) == null) // Here !!!
         return Object.Equals(person1, person2);

      return person1.Equals(person2);
   }
   ...
}

My attention was drawn to this line: if ((object)person1 == null || ((object)person2) == null) return Object.Equals(person1, person2);

To my understanding, static Object.Equals would auto check null for its parameters. Why does it bother checking null again before calling it? Is there a guideline for doing it?

I would implement it simply like this:

   public static bool operator == (Person person1, Person person2)
   {
         return Object.Equals(person1, person2);
   }

Or maybe this:

   public static bool operator == (Person person1, Person person2)
   {
      if ((object)person1 == null)
         return ((object)person2 == null)

      return person1.Equals(person2);
   }

Based on this quote from the same document: "If you do override Object.Equals(Object), your overridden implementation is also called in calls to the static Equals(System.Object, System.Object) method on your class"

Update:

I found a possible error in public bool Equals(Person other), it reads: other == null where == is overloaded! Although the result is correct, there is an extra level of indirection. I believe it should be (object)other == null.

Was it helpful?

Solution

Your approach would end up invoking the virtual Equals(Object) method, because that's what Object.Equals does. The way that's given here only uses Object.Equals when at least one of the values is null, in order to return true if they're both null and false otherwise - that path will never call back into your Equals implementations.

If both values are non-null, the MSDN approach will make a non-virtual call straight to bool Equals(Person) - which avoids not only a virtual method indirection, but also another type check which is redundant when we already know that if the references are non-null, they're both Person references.

I think the example is unfortunate in its mixture of using fields and properties, mind you - and I'd definitely write this:

if (this.uniqueSsn == other.SSN)
   return true;
else 
   return false;

as just:

return this.uniqueSsn == other.SSN;

or more obviously:

return this.uniqueSsn == other.uniqueSsn;
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top