The subtle infinite loop problem

I have to say its been quite a while since I accidently locked myself into an infinite loop.  So long in fact that I didn’t realise straight away that that was what was causing the “Stack Overflow” error that I got from my latest unit test.

The problem was that I recently found a nice little analysis program (that I will post about later) and ran it against one of my simple DLL’s. To my delight, i’m in violation of about a hundred best-practice rules 🙂 So I set to work breaking my system to conform to the analysis.

One of the rules I broke was interesting. I had made the object implement the IComparable interface and overloaded the .CompareTo method.  Best practise states, quite reasonably, that if you’re going to overload CompareTo and mess with sorting for an object, you should also override .Equals, and the operators > < and ==.  So I did that.

After doing that analysis tells me that for the operators (< > == ) i’m making a bad mistake by not checking the arguments for a null value.  So I did that and hence my infinite loop error.

The error occurs, of course, in the equality operator (==).  The definition of the overload is as follows –
public static bool operator == (MyCoolObject mco1, MyCoolObject mc02)

My original code for this was a simple return mco1.equals(mco2) which works fine when we’re testing two valid objects but when mco1 is a null value, we’ll get a null reference exception.

So my next solution was thus: if (mco1 == null) { return mco2 == null } else return mco1.Equals(mco2);

The cleverest ones amongst you, cleverer than me, have already seen where my infinite loop came from.  We get so used to using == to check for null that we can do it without thinking.  As we’re currently overriding == for MyCoolObject’s, we’re calling ourselves. Over and over again.

The answer to this might be obvious to you, but it wasn’t to me straight away so there may be others searching, as I was, for a different way to test for null.  In the end we don’t need to, here is the final, working, solution.

if ((object)mco1 == null) { return (object)mco2 == null } else return mco1.Equals(mco2)

or, further simplified,

return (Object)mco1 == null ? (Object)mco2 == null : mco1.Equals(mco2);

Yeah, casting to Object allows us to use the base class equality operator.  It shouldn’t have taken me so long to figure that out, but there it is.  Hopefully this will save someone else from the same problem.



Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s