6

It has always bothered me that C# doesn't have a dedicated reference equality operator, one that can't be overloaded. To test if a reference is null, I want to write code like this:

if (thing == null)

But there's always this nagging thought, "What if the class has overloaded the == operator?". I'm not interested in whether the class considers the object equivalent to null. I'm interested in whether the object reference is null. The alternatives seem to be casting to object:

if ((object)thing == null)

and Object.ReferenceEquals():

if (Object.ReferenceEquals(thing, null)) // long form
if (ReferenceEquals(thing, null)) // short form

But recently I have been writing code like this:

if (thing is object) // thing != null
if (!(thing is object)) // thing == null

I read this as "if thing is an object", which is to say that it's set to an object. I realize this isn't the purpose of the "is" operator, but it does check for null references and all reference types inherit from object, so... why not?

I found that, to me at least, code like this is more readable and much more comfortable to type, especially since the affirmative case (thing is object) is much more common in my code than the negative case (!(thing is object)).

So my question is, are there any pitfalls or edge cases that I'm not aware of? Is it considered bad practice or inefficient? Is it confusing? Why don't I ever see code like this?

Nexus
  • 69
  • 1
  • 2
  • 1
    Until now, I'd never have considered `is` to return `false` for `null` values, so I'd think it'd be very confusing. Mainly because it'd be weird for *an object not to be an object*, at least to me – Camilo Terevinto Oct 10 '17 at 11:52
  • I'm not quite getting what you exactly meant with 'overloading the ==', they are equal or they are not, what's there to overload about? – Steven Oct 10 '17 at 11:53
  • 7
    I'm not sure I understand *why* you are doing this. If an object has overridden == and how it compares to null then why would you not just use that? I can't think of any situations where you would want to do something different if the object was actually a null reference compared to the object telling you it was equivalent to null. The writer of the class should be the one to worry about whether its `== null` logic is correct or not and I don't get when you would decide you know better... – Chris Oct 10 '17 at 11:53
  • 1
    @Steven c# has both value comparison and reference comparison. These are 2 different things. Meaning that *they are equal or they are not* is not that trivial. – Glubus Oct 10 '17 at 11:55
  • 4
    What’s wrong with `Object.ReferenceEquals`? That’s idiomatic. – Ry- Oct 10 '17 at 11:56
  • @Ryan: Probably just a wee bit too verbose for their liking. – BoltClock Oct 10 '17 at 11:57
  • @Chris I'd imagine the OP is thinking of something like `public static bool operator ==(object o1, object o2) => false;` but that'd be very weird – Camilo Terevinto Oct 10 '17 at 11:58
  • 2
    This is opinion-based but I think any advantage you gain in brevity is negated by the reduced readability, even if for the sole reason that `thing == null` is so ubiquitous. – Rotem Oct 10 '17 at 11:58
  • @Chris, Object.ReferenceEquals exists for a reason. There are cases where you want to know if the object reference (the pointer) is set to an instance of an object or not. I'm simply looking for a more readable form. – Nexus Oct 10 '17 at 12:03
  • 1
    This kind of it-is-not-my-bug coding is very unhealthy. If somebody fumbled operator==() then you really want to know about it, that is going to hurt somebody else. Leaving it undiagnosed is the first step towards project failure, sooner or later it is all going to collapse like a house of cards. – Hans Passant Oct 10 '17 at 12:47
  • Personally, I use `x is object` and `x is null` all the time now. It's safe, easy to understand, short and sweet. Brevity is not evil, as long as it's used properly. I've even [blogged about it](https://zoharpeled.wordpress.com/2019/11/13/the-perfect-non-null-test/) recently. – Zohar Peled Mar 05 '20 at 08:16
  • @Rotem regarding "even if for the sole reason that `thing == null` is so ubiquitous.", this has now started to change quite drastically, and `thing is null` is now basically the new ubiquitous form for the comparison. – julealgon Jan 12 '22 at 19:21

2 Answers2

5

With 

if (thing is object)
  ...

you obfuscate what you want to do - check for null. It might be obvious and clean to you at the moment, but it might not be in a few month (given you've dropped that practice) and it's certainly not obvious for anyone else. If I encountered this, it'd leave me puzzled about the intention of the author. And if you need a comment to explain what you do ... don't do it. (Of course there are situations where you can and should, but definitely not for something as simple as a null-check.)

Eventually you will render your code less maintainable, since understanding your code will always need a double take. Do yourself a favour and keep your code as clean as possible and this means being honest about your intentions.

Paul Kertscher
  • 9,416
  • 5
  • 32
  • 57
  • The is operator returns true if the reference is not null and is of the specified type (or one of its sub types), by definition. This is actually what I want. So to me, it's clear. I want to know if the reference is an object or kind of object. – Nexus Oct 10 '17 at 12:22
  • 1
    I did not state, that your solution is not *technically correct*, but that it somehow obfuscates your intention. Maybe *you* will still know later what you intended, but in the real world you won't be the only one maintaining your projects - most of the time. – Paul Kertscher Oct 10 '17 at 12:25
  • 2
    `So to me, it's clear.` Ask the next 20 C# developers you meet what that code does. And then ask them what `if (thing == null)` (or `ReferenceEquals`) does. _I would suggest that they will answer the latter question much faster than the former._ Part of programming is sticking to commonly accepted practices to reduce cognitive load. If you do 'odd things' then when a bug occurs people will spend time focusing on the wrong thing. 'That thing is weird, I'd better check it out in case it is related.' – mjwills Oct 10 '17 at 12:34
  • 4
    @Nexus: I had to double check, with the language spec in hand, whether this code was really the same thing as `!Object.ReferenceEquals(object, null)`, including for value and nullable types. As far as I can tell, it is indeed, but the fact that I had to check (as someone who has experience with C# from the day .NET 1.0 came out) suggests this may not be the idiom you want to go for. `is` is strongly associated with *checking type*. That it also happens to check for `null` is a secondary effect, and using that as the primary effect is not obvious, at least not to me. – Jeroen Mostert Oct 10 '17 at 12:42
  • @mjwills, I would suggest that most C# developers would say that (thing == null) tests whether the reference is null. And they would be wrong. It calls the overloaded operator if there is one, or otherwise, it tests whether the reference is null. That's what goes through my mind when I read (thing == null) and it's irritating. I'm just looking for a simple readable idiom for testing null and nothing else. – Nexus Oct 10 '17 at 12:57
  • @Jeroen Mostert, that it checks for null is actually the primary effect because null has no type. Only instances have types. – Nexus Oct 10 '17 at 12:59
  • 2
    `And they would be wrong.` They _could_ be wrong, but they **aren't** wrong. You are solving a non-existent (or at best _hypothetical_) problem by reducing overall maintainability and increasing cognitive load. If the == operator was implemented incorrectly, the answer is not to use `is` like you suggest. It is to **fix the bug**. Or, just use `ReferenceEquals` and stop stressing about it. _Or keep doing it the way you are doing, knowing that you understand it and it may be harder for future developers to understand._ – mjwills Oct 10 '17 at 13:06
  • 2
    @Nexus: You're going the wrong way around -- I meant the primary function fulfilled by the `is` operator, which is assuredly not checking for `null`. `x is Foo` is intended to check if `x` is of runtime type `Foo` (or a descendant). That `null` never happens to be considered a `Foo` is true, but *not* what is foremost in most people's minds when they read the statement. That you've found a form of `is` that (effectively) only checks for `null` is cute, but it is not intuitive to people who are not you, if the comments are anything to go by. – Jeroen Mostert Oct 10 '17 at 13:06
  • 2
    Even if this question is on hold right now it is very interesting to read. I was unsure if my comment would have any benefit to this conversation, but as one of the next 20 developers mjwills mentioned, I would definitely almost immediately refactor any ``if (thing is object)`` code to ``if (thing == null)`` because the intent of ``thing is object`` is not immediately clear to the reader (and code is by far more often being read than written). Most of the junior developers I have met would stumble upon what this does and thus I consider this to be a code smell. – Peit Oct 10 '17 at 13:41
  • 2
    @Peit: and if you refactored it like that, you'd introduce a bug, because it should be `if (thing != null)`. I take that as an argument in favor of avoiding the construct, though. :-) – Jeroen Mostert Oct 10 '17 at 13:51
  • Haha, screw it. That's true of course. :) – Peit Oct 10 '17 at 13:52
  • See the new question where now people saying `is object` is the right thing to do... https://stackoverflow.com/questions/58505313/proper-null-check-on-service-result-in-c-sharp-net – Ants Nov 21 '19 at 02:39
  • @JeroenMostert have you changed your mind regarding this statement now that C# has constant pattern matching? "is is strongly associated with checking type.". `x is not null` is now fairly standard, and I've also seen `x is {}` in a few places. `x is object` doesn't feel so out of place considering pattern matching as well now, at least to me it doesn't. – julealgon Jan 12 '22 at 19:19
  • @julealgon: I have not changed my mind about `x is object` not being a good pattern to use. Both `x is not null` and `x is {}` are at least unambiguous in that they are new syntax and can't be confused with plain type checks. Incidentally, I'm absolutely no fan of the new patterns either, because I'm pretty old school in the belief that a language ought not to have several different ways of doing a basic thing, but *one*. C# has now accumulated so many ways to check for `null` that it verges on unhealthy. "Syntactic sugar causes cancer of the semicolon", as Alan Perlis once quipped. – Jeroen Mostert Jan 12 '22 at 19:45
  • @JeroenMostert I don't love having many ways of doing the same thing as well, however, I understand they can't just remove the old ways of doing something for backwards compatibility, otherwise they probably would. I wasn't specifically asking you about `x is object` though, but about your more general statement regarding `is`. I see you are not a fan, but this really is here to stay... even code fixes in VS and file templates have now standardized on constant pattern matching vs `==` checks. Pattern matching objectively leads to less bugs, too, which is the reason I like it the most. – julealgon Jan 12 '22 at 21:58
  • @julealgon: Do note that `x is object` is *not* pattern matching but a classical type check from the existing `is` operator! `x is {}` and `x is [not] null` *are* pattern matches. AFAIK VS and templates are not going to offer any refactoring to `x is object` as a not-null check (as well they should not). – Jeroen Mostert Jan 13 '22 at 08:41
  • I'm aware of that @JeroenMostert, I was just pointing out how the advent of pattern matching made the use of `is` a lot more "standard" in the language and how that is going to become the new default. And yes, `x is not null` is probably going to win overall. – julealgon Jan 13 '22 at 15:18
1

I'm simply looking for a more readable form.

If you are aiming for brevity, then an extension method is probably your best bet.

public static class NullExtensions
{
    public static bool ExactlyNull(this object toCheck)
    {
        return ReferenceEquals(toCheck, null);
    }
}

In all honesty I'd just use ReferenceEquals as is though. Using extension methods like this tends to reduce maintainability and often results in some tooling (e.g. Resharper) giving less useful 'advice'.

mjwills
  • 23,389
  • 6
  • 40
  • 63