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?
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)
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 as this and continuing as though nothing was wrong. Of course it is somewhat silly to check whether this 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 be null 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 that this is not null and never check for it.
Now that we've gotten that out of the way, let's criticize that code some more.
public override bool Equals(object o) {
if (o == null)
return (this == null);
else {
return ((o is Person) && (this.dni == (o as Person).dni));
}
}
First off there is an obvious bug. We presume that this could be null, ok, let's run with that. What stops this.dni from throwing null reference exception??? If you're going to assume that this 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 where x.Equals(y) can be true but x==y can be false! This is horrid. Please don't go there. If you're going to override Equals then overload == at the same time, and implement IEquatable<T> while you're at it.
(Now, there is a reasonable argument to be made that madness lies in either direction; if == is consistent with Equals with value semantics then personx == 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? Now Equals 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:
private static bool Equals(Person x, Person y)
{
if (ReferenceEquals(x, y))
return true;
else if (ReferenceEquals(x, null))
return false;
else if (ReferenceEquals(y, null))
return false;
else
return x.dni == y.dni;
}
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:
public static bool operator ==(Person x, Person y)
{
return Equals(x, y);
}
public static bool operator !=(Person x, Person y)
{
return !Equals(x, y);
}
public override bool Equals(object y)
{
return Equals(this, y as Person);
}
public bool Equals(Person y)
{
return Equals(this, y);
}
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 comparing this 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) and IEquatable<T>.Equals(T)) ways to implement equality, is very complicated and confusing even without assuming that this can or cannot be null.
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.
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":
public class Test
{
public void Method()
{
Console.WriteLine(this == null);
}
}
public class Program
{
public static void Main(string[] args)
{
object t = new Test();
var methodInfo = t.GetType().GetMethod("Method");
var a = (Action)Delegate.CreateDelegate(typeof(Action), null, methodInfo);
a();
}
}
Honestly, though I've never seen someone actually check thisfor null in production code.
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-string dni is equivalent to a null person:
public static bool operator ==(Person a, Person b)
{
if(String.IsNullOrEmpty(a.dni) && (object)b == null)
return true;
if(String.IsNullOrEmpty(b.dni) && (object)a == null)
return true;
return Object.ReferenceEquals(a, b);
}
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:
using System;
namespace ClassLibrary1 {
public class Class1 {
public void NullTest() {
if (this == null) Console.WriteLine("It's null");
}
}
}
C++/CLI code:
#include "stdafx.h"
using namespace System;
int main(array<System::String ^> ^args) {
ClassLibrary1::Class1^ obj = nullptr;
obj->NullTest();
Console::ReadLine();
return 0;
}
Output:
It's null
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.
