compairing to null when operator is overridden

[Pesso]

I'm having a difficulty compairing null to a class object whose "equal"
operator is overridden.. Consider the following:

class Foo
{
// ...
public static bool operator==(Foo f1, Foo f2)
{ return (f1.n == f2.n); }

Now the following throws:
Foo foo = new Foo();
if(null == foo) // throws System.NullReferenceException
{ //

If I check for null inside my operator override, it causes stack overflow.
public static bool operator==(Foo f1, Foo f2)
{
if(null == t1) ... // stack overflow

So my question is what's the correct way to handle null comparison when your
class overrides the equal operator?

 

[Tasos Vogiatzoglou]

The correct way to override == is to first override .Equals and
..HashCode and then call .Equals method. You can check for null values
inside the .Equals method.

Your overloaded operator will look like :

public static bool operator==(Foo f1, Foo f2)
{ return (f1.Equals(f2)); }

and the .Equals method can look like :

public override bool Equals(object obj)
{
if (obj == null)
return false;
return ((Foo)obj).n.Equals(n);
}

 

[Jon Skeet [C# MVP]]

I'm having a difficulty compairing null to a class object whose "equal"
operator is overridden.. Consider the following:

class Foo
{
// ...
public static bool operator==(Foo f1, Foo f2)
{ return (f1.n == f2.n); }

Now the following throws:
Foo foo = new Foo();
if(null == foo) // throws System.NullReferenceException
{ //

If I check for null inside my operator override, it causes stack overflow.
public static bool operator==(Foo f1, Foo f2)
{
if(null == t1) ... // stack overflow

So my question is what's the correct way to handle null comparison when your
class overrides the equal operator?

Either cast t1 to object first, or use object.ReferenceEquals.

 

[Christoph Nahr]

The correct way to override == is to first override .Equals and
.HashCode and then call .Equals method. You can check for null values
inside the .Equals method.

Your overloaded operator will look like :

public static bool operator==(Foo f1, Foo f2)
{ return (f1.Equals(f2)); }

That's not quite enough, you must check f1 against null before
attempting to invoke a method on it. Add the following line before
the return:

if ((object) f1 == null) return ((object) f2 == null);

Additionally, your Object.Equals override won't get called by
operator== if the class also provides a strongly-typed overload of
Equals, and in such a strongly-typed overload you'll once again have
to use an object cast for the null check.

 

[Christoph Nahr]

Either cast t1 to object first, or use object.ReferenceEquals.

Do you know the IL generated for ReferenceEquals? If it's a method
call you should use an object cast instead because ((object) x ==
null) gets compiled into optimal IL (a single null comparison opcode).

 

[Jon Skeet [C# MVP]]

Do you know the IL generated for ReferenceEquals? If it's a method
call you should use an object cast instead because ((object) x ==
null) gets compiled into optimal IL (a single null comparison opcode).

It will certainly be a method call in IL, but of course it may well be
inlined by the JIT. However, I wouldn't use that as the main criterion
at all - for me, it depends which you find most readable. Use the most
readable form until you've got good evidence that the code in question
is actually a bottleneck in your application.

 

[Christoph Nahr]

It will certainly be a method call in IL, but of course it may well be
inlined by the JIT.

Good point, my mistake. Object.ReferenceEquals is a static method
that contains just the single line "return (x == y)" so it's certainly
going to be inlined, and the effect will be the same... at least when
optimization is turned on.

However, I wouldn't use that as the main criterion
at all - for me, it depends which you find most readable. Use the most
readable form until you've got good evidence that the code in question
is actually a bottleneck in your application.

Personally I don't see any difference in readability between the two
variants -- the operator== version would be more readable if you
didn't need the object cast, but since you do it's a wash.

In general, though, if there's one place where premature optimization
is a good idea it's in infrastructure methods such as equality tests.
These low-level methods are likely to show up at the end of a lot of
call trees, and so will have a broad influence on your application's
performance regardless of the presence of specific hotspots.

If you never pay any attention to performance before profiling you may
well end up with an application that's quite sluggish everywhere, but
does not have any bottlenecks that are detectable by profiling...

 

[Jon Skeet [C# MVP]]

Good point, my mistake. Object.ReferenceEquals is a static method
that contains just the single line "return (x == y)" so it's certainly
going to be inlined, and the effect will be the same... at least when
optimization is turned on.
Yup.


Personally I don't see any difference in readability between the two
variants -- the operator== version would be more readable if you
didn't need the object cast, but since you do it's a wash.

It's very much a matter of taste. I don't find either particularly more
readable, but I think object.ReferenceEquals is perhaps a little more
explicit.

In general, though, if there's one place where premature optimization
is a good idea it's in infrastructure methods such as equality tests.
These low-level methods are likely to show up at the end of a lot of
call trees, and so will have a broad influence on your application's
performance regardless of the presence of specific hotspots.

If you never pay any attention to performance before profiling you may
well end up with an application that's quite sluggish everywhere, but
does not have any bottlenecks that are detectable by profiling...

You *may* do - but in my experience, you don't actually do so. Firstly,
it would depend on how often Equals was actually called - I don't find
myself calling Equals on my own types on a very regular basis.

Secondly, the architecture design is still more likely to be the
overwhelming factor, IMO. *That* is very hard to change later on,
whereas improving the performance of "leaf" methods like Equals is very
easy to do *if* you need to.

(Profiling *would* show this up to be a bottleneck if it genuinely was,
IMO - if you can see a significant portion of your time spent in Equals
methods, that should ring alarm bells.)

 

[Tasos Vogiatzoglou]

You are correct... I didn't thing the other situations... My mistake
....