In an example, my professor has implemented Equals as follows:
public class Person {
private string dni;
// ...
public override bool Equals(object o) {
if (o == null)
return (this == null);
else {
return ((o is Person) && (this.dni == (o as Person).dni));
}
}
}
I have no experience with C#, but as far as I know this
cannot be null inside a member function (at least this is true in C++ and Java, the languages I know) so the if seems weird to me.
Am I right or is there any component in c# I dont know of which makes the test this == null
necesary?
One way
this
could==
null
is if you practiced some black magic by overloading the==
operator. For example, if someone decides that having a null or empty-stringdni
is equivalent to a null person:Note this is probably a Really Bad Idea.
Yes, it is possible and in fact not entirely unlikely. The C# language gives an excellent guarantee that no method can be called on a null reference to a class object, it generates code that makes a null check at the call site. Which certainly does avoid a lot of head-scratching, a NullReferenceException inside the method can be pretty hard to debug without that check.
That check is however specific to C#, not all .NET languages implement it. The C++/CLI language doesn't for example. You can see it for yourself by creating a solution with a C++ CLR Console mode project and a C# class library. Add a reference in the CLR project to the C# project.
C# code:
C++/CLI code:
Output:
This is not otherwise undefined behavior or illegal in any way. It is not verboten by the CLI spec. And as long as the method doesn't access any instance members then nothing goes wrong. The CLR handles this as well, a NullReferenceException is also generated for pointers that are not null. Like it will be when NullTest() accesses a field that isn't the first field in the class.
Yes, in some cases. The most common case is if you invoke an instance method using a delegate created by reflection and pass a null receiver. This will print "True":
Honestly, though I've never seen someone actually check
this
for null in production code.Let's begin by noting that your statement is false.
In C++, dispatching a method on a null receiver is undefined behavior and undefined behavior means that anything can happen. "Anything" includes the program passing
NULL
asthis
and continuing as though nothing was wrong. Of course it is somewhat silly to check whetherthis
is null in C++ because the check can only be true if you already do not know what your program is doing, because its behavior is undefined.Whether
this
can be null in Java I have no idea.Now to address your question about C#. Let's assume that
==
is not overloaded. We'll come back to this point later.Your method is written in C#. Suppose it is invoked from a C# program with a null receiver. The C# compiler evaluates whether the receiver could possibly be null; if it could possibly be null then it ensures that it generates code that does a null check before invoking the method. Therefore this check is pointless in that scenario. This is of course the 99.9999% likely scenario.
Suppose it is invoked via Reflection, as in mike z's answer. In that case it is not the C# language that is performing the invocation; rather, someone is deliberately abusing reflection.
Suppose it is invoked from another language. We have a virtual method; if it is invoked from this other language with virtual dispatch then a null check must be performed, because how else could we know what is in the virtual slot? In that scenario it cannot be null.
But suppose it is invoked from another language using non-virtual dispatch. In that case the other language need not implement the C# feature of checking for null. It could just invoke it and pass null.
So there are several ways in which
this
could benull
in C#, but they are all very much out of the mainstream. It is therefore very rare for people to write code as your professor has. C# programmers idiomatically suppose thatthis
is notnull
and never check for it.Now that we've gotten that out of the way, let's criticize that code some more.
First off there is an obvious bug. We presume that
this
could be null, ok, let's run with that. What stopsthis.dni
from throwing null reference exception??? If you're going to assume thatthis
can be null then at least do so consistently! (At Coverity we refer to this sort of situation as a "forward null defect".)Next: we are overriding
Equals
and then using==
inside, presumably to mean reference equality. This way lies madness! Now we have a situation wherex.Equals(y)
can be true butx==y
can be false! This is horrid. Please don't go there. If you're going to overrideEquals
then overload==
at the same time, and implementIEquatable<T>
while you're at it.(Now, there is a reasonable argument to be made that madness lies in either direction; if
==
is consistent withEquals
with value semantics thenpersonx == persony
can be different than(object)personx == (object)persony
, which seems strange also. The takeaway here is that equality is pretty messed up in C#.)Moreover: what if
==
is overridden later? NowEquals
is calling an overridden==
operator, when the author of the code clearly wishes to be doing a reference comparison. This is a recipe for bugs.My recommendations are (1) write one static method that does the right thing, and (2) use
ReferenceEquals
every time there could possibly be any confusion over what kind of equality is meant:That nicely covers every case. Notice that it is crystal clear to the reader when reference equality semantics are meant. Also note that this code makes it very easy to put breakpoints on each possibility, for debugging purposes. And finally, notice that we take the cheapest possible early out; if the objects are reference equal then we don't have to do the potentially expensive comparison of the fields!
Now the other methods are easy:
Notice how much more elegant and clear my way is than your professor's way. And notice that my way handles a null
this
without ever comparingthis
to null directly.Again: all of this illustrates that the compromise position arrived at, in which both value and reference equality are possible and there are four (
==
,!=
,object.Equals(object)
andIEquatable<T>.Equals(T)
) ways to implement equality, is very complicated and confusing even without assuming thatthis
can or cannot benull
.If this subject interests you, I describe a slightly harder problem on my blog this week: how to implement comparisons in general, including inequalities.
http://ericlippert.com/2013/10/07/math-from-scratch-part-six-comparisons/
The comments are particularly interesting as a critique of how C# handles equality.
Finally: don't forget to override
GetHashCode
. Make sure you do it right.