Overriding == operator. How to compare to null? [duplicate]
Asked Answered
L

9

158

Possible Duplicate:
How do I check for nulls in an ‘==’ operator overload without infinite recursion?

There is probably an easy answer to this...but it seems to be eluding me. Here is a simplified example:

public class Person
{
   public string SocialSecurityNumber;
   public string FirstName;
   public string LastName;
}

Let's say that for this particular application, it is valid to say that if the social security numbers match, and both names match, then we are referring to the same "person".

public override bool Equals(object Obj)
{
    Person other = (Person)Obj;
    return (this.SocialSecurityNumber == other.SocialSecurityNumber &&
        this.FirstName == other.FirstName &&
        this.LastName == other.LastName);
}

To keep things consistent, we override the == and != operators, too, for the developers on the team who don't use the .Equals method.

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

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

Fine and dandy, right?

However, what happens when a Person object is null?

You can't write:

if (person == null)
{
    //fail!
}

Since this will cause the == operator override to run, and the code will fail on the:

person.Equals()

method call, since you can't call a method on a null instance.

On the other hand, you can't explicitly check for this condition inside the == override, since it would cause an infinite recursion (and a Stack Overflow [dot com])

public static bool operator ==(Person person1, Person person2)
{
    if (person1 == null)
    {
         //any code here never gets executed!  We first die a slow painful death.
    }
    return person1.Equals(person2);
}

So, how do you override the == and != operators for value equality and still account for null objects?

I hope that the answer is not painfully simple. :-)

Lemmon answered 18/11, 2010 at 20:29 Comment(3)
SO is preventing further answers to this question, but as of C# 7, the best solution is to use the new is null construct i.e. if (person1 is null) ...Uracil
@Ross, no answers can be added because this is a duplicate question. Additionally, if you'd like to point out the is null construct to the original question, you'll find out that others already answered that, 2 years before your comment.Unmanly
@Unmanly That may be, but this question is #1 in Google search. It seems.... incredibly misguided... to leave this page full of (now) bad answers uncorrected, hoping that people will click the duplicate link instead of using one of the highly upvoted and workable, yet outdated, answers on this page. If the goal of SO is to help, every effort should be made to either get this page removed entirely, or make it as great as it can be.Hogshead
D
278

Use object.ReferenceEquals(person1, null) or the new is operator instead of the == operator:

public static bool operator ==(Person person1, Person person2)
{
    if (person1 is null)
    {
         return person2 is null;
    }

    return person1.Equals(person2);
}
Digitize answered 18/11, 2010 at 20:31 Comment(7)
ReferenceEquals emits a method call though, while casting to object will cause the compiler to just emit instructions to compare the references for equality. ... But that probably counts as evil micro-optimization.Wagtail
@dtb: The JIT inlines this call anyway; runtime performance will be identical. See #736054Digitize
Maybe this is obvious, but you probably also want to do a null check on the Obj parameter inside your Person.Equals override. It looks to me like doing person == null would still result in calling person.Equals(null)Regality
Yes, I was relying the Equals method to check for null arguments. I always code my own Equals methods to consider the null case, and I expect others to do so to. ;)Digitize
Sounds perfectly reasonable :) My comment was intended to make sure that the OP handled that secondary case in the Equals method rather than also in the == overload, because it sounds like some people use == and some people use .Equals. Just want to make sure that this code doesn't blow up: Person person1 = new Person(), person2 = null; if (person1.Equals(person2)) // ...Regality
VS 2017 wants me to use the <i>is</i> operator now: left is null && right is null.Grimes
Good solution. Could simplify a bit (depends on taste): return object.ReferenceEquals(person1, null) ? object.ReferenceEquals(person2, null) : person1.Equals(person2);Outtalk
H
22

I've always done it this way (for the == and != operators) and I reuse this code for every object I create:

public static bool operator ==(Person lhs, Person rhs)
{
    // If left hand side is null...
    if (System.Object.ReferenceEquals(lhs, null))
    {
        // ...and right hand side is null...
        if (System.Object.ReferenceEquals(rhs, null))
        {
            //...both are null and are Equal.
            return true;
        }

        // ...right hand side is not null, therefore not Equal.
        return false;
    }

    // Return true if the fields match:
    return lhs.Equals(rhs);
}

"!=" then goes like this:

public static bool operator !=(Person lhs, Person rhs)
{
    return !(lhs == rhs);
}

Edit
I modified the == operator function to match Microsoft's suggested implementation here.

Hynes answered 18/11, 2010 at 20:38 Comment(4)
Ugh. Sorry to say this but this is a terrible example of code bloat. You need 14 lines to express something that can be expressed, just as readably, in a single return statement with a conditional expression (distributed over two to three lines for readability).Rovit
@Konrad Rudolph - So, how do I improve this? What are the 2 or three lines that you are thinking of?Hynes
well, for one thing the null check of rhs is redundant because Equals must also perform it. The remainder can be expressed as return object.ReferenceEquals(lhs, rhs) || !object.ReferenceEquals(lhs, null) && lhs.Equals(rhs); – with appropriate formatting (linebreaks, indentation) to make it readable, obviously. If that is too obscure, put in a comment with a link to the appropriate MSDN article that describes the semantic requirements of the equality operator.Rovit
I'm up-voting this because it follows Microsoft's suggestion to the letter, but also agree that it can be simplified without loosing readability. Example: from if (System.Object.ReferenceEquals(rhs, null)) to return false; can be replaced by return System.Object.ReferenceEquals(rhs, null);Mandate
I
12

you could alway override and put

(Object)(person1)==null

I'd imagine this would work, not sure though.

Inquiring answered 18/11, 2010 at 20:32 Comment(4)
It works … but it’s a bit obscure (compared to using object.ReferenceEquals).Rovit
This is actually quite efficient since it amounts to a single CEQ instruction being emitted. The ReferenceEquals approach calls a function which is far less efficient.Amontillado
@W.KevinHazzard - That is not true; the call to ReferenceEquals is optimized out to a pointer comparison.Sjoberg
both of you have spent more time discussing this optimisation than you could ever hope to gain by using it.Shuster
P
5

Easier than any of those approaches would be to just use

public static bool operator ==(Person person1, Person person2)   
{   
    EqualityComparer<Person>.Default.Equals(person1, person2)
} 

This has the same null equality semantics as the approaches that everyone else is proposing, but it's the framework's problem to figure out the details :)

Policyholder answered 19/11, 2010 at 2:17 Comment(0)
L
3

The final (hypothetical) routine is below. It is very similar to @cdhowie's first accepted response.

public static bool operator ==(Person person1, Person person2)
{
    if (Person.ReferenceEquals(person1, person2)) return true;
    if (Person.ReferenceEquals(person1, null)) return false; //*
    return person1.Equals(person2);
}

Thanks for the great responses!

//* - .Equals() performs the null check on person2

Lemmon answered 18/11, 2010 at 21:26 Comment(0)
W
2

Cast the Person instance to object:

public static bool operator ==(Person person1, Person person2)
{
    if ((object)person1 == (object)person2) return true;
    if ((object)person1 == null) return false;
    if ((object)person2 == null) return false;
    return person1.Equals(person2);
}
Wagtail answered 18/11, 2010 at 20:33 Comment(0)
Q
2

Cast the Person to an Object and then perform the comparison:

object o1 = (object)person1;
object o2 = (object)person2;
if(o1==o2) //compare instances.
   return true;
if (o1 == null || o2 == null)  //compare to null.
   return false;
//continue with Person logic.
Quagga answered 18/11, 2010 at 20:36 Comment(0)
I
2

cdhowie is on the money with the use of ReferenceEquals, but it's worth noting that you can still get an exception if someone passes null directly to Equals. Also, if you are going to override Equals it's almost always worth implementing IEquatable<T> so I would instead have.

public class Person : IEquatable<Person>
{
  /* more stuff elided */

  public bool Equals(Person other)
  {
    return !ReferenceEquals(other, null) &&
      SocialSecurityNumber == other.SocialSecurityNumber &&
      FirstName == other.FirstName &&
      LastName == other.LastName;
  }
  public override bool Equals(object obj)
  {
    return Equals(obj as Person);
  }
  public static bool operator !=(Person person1, Person person2)
  {
    return !(person1 == person2);
  }
  public static bool operator ==(Person person1, Person person2)
  {
    return ReferenceEquals(person1, person2)
      || (!ReferenceEquals(person1, null) && person1.Equals(person2));
  }
}

And of course, you should never override Equals and not override GetHashCode()

public override int GetHashCode()
{
   //I'm going to assume that different
   //people with the same SocialSecurityNumber are extremely rare,
   //as optimise by hashing on that alone. If this isn't the case, change this
   return SocialSecurityNumber.GetHashCode();
}

It's also worth noting that identity entails equality (that is, for any valid concept of "equality" something is always equal to itself). Since equality tests can be expensive and occur in loops, and since comparing something with itself tends to be quite common in real code (esp. if objects are passed around in several places), it can be worth adding as a shortcut:

  public bool Equals(Person other)
  {
    return !ReferenceEquals(other, null) &&
      ReferenceEquals(this, other) ||
      (
        SocialSecurityNumber == other.SocialSecurityNumber &&
        FirstName == other.FirstName &&
        LastName == other.LastName
      );
  }

Just how much of a benefit short-cutting on ReferenceEquals(this, other) is can vary considerably depending on the nature of the class, but whether it is worth while doing or not is something one should always consider, so I include the technique here.

Infective answered 19/11, 2010 at 1:25 Comment(0)
R
1

Overloading these operators consistently is pretty hard. My answer to a related question may serve as a template.

Basically, you first need to do a reference (object.ReferenceEquals) test to see if the object is null. Then you call Equals.

Rovit answered 18/11, 2010 at 20:33 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.