When overloading the equality operator, what's the best way to handle null values?

Question

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

Say I have a type like this:

public class Effect
{
    public static bool operator == ( Effect a, Effect b )
    {
        return a.Equals ( b );
    }

    public static bool operator != ( Effect a, Effect b )
    {
        return !a.Equals ( b );
    }

    public bool Equals ( Effect effect )
    {
        return this.TypeID.Equals ( effect.TypeID );
    }

    public override bool Equals ( object obj )
    {
        return this.TypeID.Equals ( ( ( Effect ) obj ).TypeID );
    }
}

What's the most robust and cleanest way to handle null values?

I am not sure if I have to check for null for both the current instance (this) and the passed instance (effect/obj)? If I have null for the current instance (this), would the compiler still call effect.Equals or Object.Equals?

Also either way where should the null checks be done? I am assuming only inside the Equals methods, and not the equality operators (==, !=).

is it really a good idea to substitute equality of two objects with equality of of their properties? I can see that working with structs, but overriding == for classes seems like a breeder for obscure bugs. — Ilia G, Feb 01 '11 at 21:11
I tend to agree with liho1eye. Generally overloading the == operator is bad news. I personally believe it's better practice to get used to calling .Equals() directly in your code. — Doug, Feb 01 '11 at 21:22
@liho1eye, @Joan: There is a guideline from MS that overloading `==` and `Equals` for _mutable_ types is not a good idea. — H H, Feb 01 '11 at 22:27
@Henk: Thanks, do you know where it is? Btw this Effect type isn't mutable. — Joan Venge, Feb 01 '11 at 23:12
@Henk yes I know that, but my point is that it is a bad idea for reference types regardless of wherever they are mutable (value types should not be mutable always, otherwise you have other issues). I don't have a good example of top of my head, but it is very likely that various collection/groupping/hashing types/algorithms might be using `IEquatable.Equals()` or simply `==` and `!=`, which is likely to produce weird and hard to trace bugs with this class. — Ilia G, Feb 02 '11 at 02:24
@liho1eye : string is a reference type (and a collection). The value-based Equality works OK for it. — H H, Feb 02 '11 at 08:13
@JoanVenge just corrected title from "overriding" to "overloading" — nawfal, Dec 17 '12 at 14:07

score 2 · Answer 1 · answered Feb 01 '11 at 21:07

2

Firstly, this can never be null, at least not in code produced by the C# compiler.

Second, use the ReferenceEquals method to check for a null reference without possibly calling an overloaded version of == (or do ((object) sometypeinstance) == null).

answered Feb 01 '11 at 21:07

leppie

115,091
17
196
297

Thanks but if I have Effect e = null, wouldn't its "this" value be null in this case? – Joan Venge Feb 01 '11 at 21:17
1

+1, the only correct answer here, seriously! – nawfal Dec 16 '12 at 09:10

score 2 · Answer 2 · answered Feb 01 '11 at 21:38

2

Visual Studio has a snippet that provides a basic Equals() implementation for you. I'd follow that, unless you have a strong reason not to.

// override object.Equals
public override bool Equals(object obj)
{
    //       
    // See the full list of guidelines at
    //   http://go.microsoft.com/fwlink/?LinkID=85237  
    // and also the guidance for operator== at
    //   http://go.microsoft.com/fwlink/?LinkId=85238
    //

    if (obj == null || GetType() != obj.GetType())
    {
        return false;
    }

    // TODO: write your implementation of Equals() here
    throw new NotImplementedException();
    return base.Equals(obj);
}

// override object.GetHashCode
public override int GetHashCode()
{
    // TODO: write your implementation of GetHashCode() here
    throw new NotImplementedException();
    return base.GetHashCode();
}

answered Feb 01 '11 at 21:38

David Yaw

27,383
4
60
93

this doesn't answer the question. Please correct it or delete it.. – nawfal Dec 17 '12 at 14:10
I believe it does answer the question. In answer to the question "how do you handle null values", the snippet answers "return false". – David Yaw Dec 17 '12 at 15:02
Sorry for being harsh, my mistake, but seriously, the catch here is when handling null values inside operator overloading and that is the question too. Even if OP does what you do, it can still give null reference exception inside overloaded `==` or `!=`. – nawfal Dec 17 '12 at 15:10

score 1 · Answer 3 · answered Feb 01 '11 at 21:10

1

How about

public static bool operator == ( Effect a, Effect b )     
{  
    return object.Equals(a, b);
}

The default implementation of Object.Equals() does the null checking for you.

In case you're curious, here's how Object.Equals() does it (courtesy of .NET Reflector):

public static bool Equals(object objA, object objB)
{
    return ((objA == objB) || (((objA != null) && (objB != null)) && objA.Equals(objB)));
}

answered Feb 01 '11 at 21:10

Doug

5,208
3
29
33

Thanks, when I used a null value my type threw a null ref exception. – Joan Venge Feb 01 '11 at 21:16
See the comments to your question - overriding == and != is not something to be done lightly :) – Doug Feb 01 '11 at 21:24
Thanks, I didn't pay attention to the first part of your message. – Joan Venge Feb 01 '11 at 21:32

Spencer Booth · Answer 4 · 2011-02-01T21:25:42.990

0

It is necessary to check for nulls with every method that takes parameters making your class.

public class Effect
{
    public static bool operator == ( Effect a, Effect b )
    {
        if (a == null) && (b == null) return true;
            if (a == null) return false;
            return a.Equals ( b );
    }

    public static bool operator != ( Effect a, Effect b )
    {
        return !(a == b);
    }

    public bool Equals ( Effect effect )
    {
            if (b == null) return false;
        return this.TypeID.Equals ( effect.TypeID );
    }

    public override bool Equals ( object obj )
    {
            if (obj == null) return false;
        return this.TypeID.Equals ( ( ( Effect ) obj ).TypeID );
    }
}

Other things to look out for are GetHashCode which should be implemented if equality is being implemented and the fact that GetHashCode should only be implemented on immutable properties, if this object is going to be used in a dictionary or similar object that compares items using the hash code.

edited Feb 01 '11 at 21:25

answered Feb 01 '11 at 21:20

Spencer Booth

1,532
1
11
15

Thanks, also this wouldn't compile, right? (return !a == b;)? – Joan Venge Feb 01 '11 at 21:21
My mistake I missed a pair of brackets, I must be too used to the crutch of an IDE. I've changed it to !(a == b); – Spencer Booth Feb 01 '11 at 21:27
Np man, I am used to the IDE too much too :O – Joan Venge Feb 01 '11 at 21:31
-1, this creates infinite recursion and never really works at all. See http://stackoverflow.com/questions/73713/how-do-i-check-for-nulls-in-an-operator-overload-without-infinite-recursion – nawfal Dec 16 '12 at 09:07

score -1 · Answer 5 · edited Dec 16 '12 at 09:18

-1

Add this:

 public static bool operator == ( Effect a, Effect b )     
 {  
     return a is Effect && b is Effect && a.TypeID.Equals (b.TypeID);    
 }  
 public static bool operator != ( Effect a, Effect b )
 {         
     return !(a == b );     
 }      
 public bool Equals ( Effect effect )     
 {         
    return this == effect );     
 }      
 public override bool Equals ( object obj )     
 {   
    return obj is Effect && this == obj); 
 }

or instead of last put:

 public override bool Equals ( object obj )     
 {   
    if (obj == null) throw new ArgumentNullException(
         "obj", "obj is null");
    if (!(obj is effect)) throw new ArgumentException(
         "obj", "obj is not an effect");
    return obj is Effect && this == obj); 
 }

edited Dec 16 '12 at 09:18

nawfal

70,104
56
326
368

answered Feb 01 '11 at 21:09

Charles Bretana

143,358
22
150
216

1

Better be careful, though. `==` requires you to also define `!=`. This could potentially result in mutual recursion. – Dan Tao Feb 01 '11 at 21:12
1

If you do use that code it will be necessary to add the equivalent to the != method. – Spencer Booth Feb 01 '11 at 21:13
2

This will introduce a stack overflow, no? == calls !=, != calls ==, etc... – Chris Shain Feb 01 '11 at 21:14
1

@Charles: Chris is right; this is going to overflow. – Dan Tao Feb 01 '11 at 21:15
Yes fixing it now... perils of trying to be too quick... – Charles Bretana Feb 01 '11 at 21:18

score -1 · Answer 6 · answered Feb 01 '11 at 21:13

-1

Yes, you should chech against null. Why are you afraid of that?

Remember that when you play around with equals, you probably also want to take a look at the hashcode method! Those two methods are intertwined.

answered Feb 01 '11 at 21:13

Carlo V. Dango

13,322
16
71
114

When overloading the equality operator, what's the best way to handle null values?

6 Answers6