45

We're launching a system, and we sometimes get the famous exception NullReferenceException with the message Object reference not set to an instance of an object.

However, in a method where we have almost 20 objects, having a log which says an object is null, is really of no use at all. It's like telling you, when you are the security agent of a seminar, that a man among 100 attendees is a terrorist. That's really of no use to you at all. You should get more information, if you want to detect which man is the threatening man.

Likewise, if we want to remove the bug, we do need to know which object is null.

Now, something has obsessed my mind for several months, and that is:

Why doesn't .NET give us the name, or at least the type of the object reference, which is null?. Can't it understand the type from reflection or any other source?

Also, what are the best practices to understand which object is null? Should we always test nullability of objects in these contexts manually and log the result? Is there a better way?

Update: The exception The system cannot find the file specified has the same nature. You can't find which file, until you attach to the process and debug. I guess these types of exceptions can become more intelligent. Wouldn't it be better if .NET could tell us c:\temp.txt doesn't exist. instead of that general message? As a developer, I vote yes.

yoozer8
  • 693
  • 1
  • 8
  • 20
Saeed Neamati
  • 18,142
  • 23
  • 87
  • 125
  • 16
    The exception should include a stack trace with line number. I would start my investigation from there looking at every object accessed on that line. – PersonalNexus Jan 15 '12 at 11:25
  • 2
    Also, I've always wondered why the exception helper dialog in Visual Studio includes the "helpful" hint to use `new` to create instances of a class. When does such a hint every really help? – PersonalNexus Jan 15 '12 at 11:27
  • And what if for any reason, you have 10 objects on that line? Like a RESTful API URL builder, which creates the URL, based on almost 7 input parameters? Still it's not helpful that much. Also, now that I look at logs, I don't see line numbers, I don't know why. – Saeed Neamati Jan 15 '12 at 11:27
  • That is a problem, but, alas, I don't have a better answer. In fact, I've had the same problem myself and that is the best I could come up with. I am curious what other responses your question generates. – PersonalNexus Jan 15 '12 at 11:30
  • 1
    If you have a chain of ten object calls then you've got a problem with coupling in your design. – Pete Kirkham Feb 11 '13 at 09:32
  • 4
    see this question http://stackoverflow.com/questions/14787580/why-net-doesnt-show-which-object-is-null –  Feb 12 '13 at 08:15
  • 13
    I love how every single answer to this question are along the lines of "Use a debugger, log your errors, check for null, it's your fault anyway" which are not answering the question but putting the blame on you. Only on stackoverflow does someone actually give you an answer (which I think says it's too much overhead for the VM to keep track of). But really, the only people who can answer this question properly is someone from microsoft who worked on the framework. – Rocklan Feb 20 '13 at 23:19
  • "Can't it understand the type from reflection". No it can't. The object you'd reflect doesn't exist. This is like saying, "If you're absent, please raise your hand". Similarly, when variable names are not preserved in compiled code it's hard to automatically make them part of exception messages. It's not that I wouldn't love to have what you're asking for. I just know why it doesn't work that way. – candied_orange Aug 25 '15 at 05:55
  • 2
    Of course it could easily work as OP asks if we wanted it to, and in debug builds it wouldn't require any new information. One of these days Microsoft or somebody else will do what the OP is suggesting, and then the next generation of programmers will wonder how anybody lived without it, and we'll be left grumbling about "kids these days". It's just a tradition that hasn't changed yet. – Spike0xff Sep 28 '15 at 20:38
  • 1
    The wait is over dude. https://blogs.msdn.microsoft.com/dotnet/2016/08/02/announcing-net-framework-4-6-2/ – LiverpoolsNumber9 Aug 05 '16 at 06:43
  • 1
    I agree with this the title of this question. There needs to be a way for the C# to tell the developer which variable held the null which caused the exception to occur. Nothing is impossible with programming. – Fractal Oct 23 '20 at 13:07
  • @Rocklan: If you've got that many references all happening on a single line (i.e. the one from the stack trace), then the lack of clarity is indeed a problem of your own making. That's not an argument why the exception can't possibly add more information, but it is a more important area of focus in case OP were to find themselves in that situation, which would in turn preclude the issue on which the question is rooted. – Flater Oct 20 '22 at 22:45
  • @Rocklan: Maybe better expressed as _Give a developer a clearer error message, and they'll not have to deal with one uncertainty. Teach a developer to structure their code more cleanly and readably and inspect it, and they'll not have to deal with many uncertainties._ – Flater Oct 20 '22 at 22:52

8 Answers8

29

The NullReferenceException basically tells you: you are doing it wrong. Nothing more, nothing less. It's not a full-fledged debugging tool, on the contrary. In this case I'd say you're doing it wrong both because

  • there is a NullReferenceException
  • you didn't prevent it in a way you know why/where it happened
  • and also maybe: a method requiring 20 objects seems a bit off

I'm a big fan of checking everything before things start going wrong, and providing good information to the developer. In short: write checks using ArgumentNullException and the likes and write the name yourself. Here's a sample:

void Method(string a, SomeObject b)
{
    if (a == null) throw ArgumentNullException("a");
    if (b == null) throw ArgumentNullException("b");
  
    // See how nice this is, and what peace of mind this provides? As long as
    // nothing modifies a or b you can use them here and be 100% sure they're not
    // null. Should they be when entering the method, at least you know which one
    // is null.
    var c = FetchSomeObject();
    if(c == null)
    {
        throw InvalidOperationException("Fetching B failed!!");
    }
    
    // etc.
}

You could also look into Code Contracts, it has it quirks but it works pretty well and saves you some typing.

update in .Net 6 there's ThrowIfNull which removes the need for stating the parameter name. And since C# version 6, there's the nameof operator removing the rather error-prone need to repeat the argument name in a hardcoded string.

void Method(string a, SomeObject b)
{
  //.Net 6
  ArgumentNullException.ThrowIfNull(a);

  //C# 6
  if (b == null) {
    throw new ArgumentNullException(nameof(b));
  }
}

As to why this is not the default: we can only guess (or try to dig up notes somehow from people working on C# and/or .Net). My guess:

  • sometimes it's not needed or wanted
  • sometimes the message should have more than just the parameter name
  • there are alternatives (stacktrace, adding it manually)
  • it gives developers a choice
  • adding this everywhere by default isn't cheap because it means nearly all argument and variable names must be added as strings in the executable
stijn
  • 4,108
  • 1
  • 25
  • 31
  • 1
    With a project of more than 1000 methods and functions, and more than 2000 operations (imaginary, but close to real numbers), writing that checking code becomes a tedious task. Not a good solution. But thanks for your answer. :) – Saeed Neamati Jan 15 '12 at 12:29
  • 17
    @SaeedNeamati you are not implying that because you have a large codebase you should not do decent error checking, right? Imo the larger the project, the more important become the roles or error checking and reporting. – stijn Jan 15 '12 at 12:54
  • 1
    No, I'm not implying that. I only say that checking every object to get sure that it's not null, is not a good method. That's all. :) – Saeed Neamati Jan 15 '12 at 12:58
  • 6
    +1 for good advice, even if it doesn't answer the actual question (which probably only Anders Hejlsberg can answer). – Ross Patterson Jan 15 '12 at 15:09
  • 12
    +1 for excellent advice. @SaeedNeamati you should listen to this advice. Your problem is caused by carelessness and a lack of professionalism in the code. If you don't have time to write good code, you have much bigger problems... – MattDavey Oct 26 '12 at 09:49
  • 4
    If you don't have time to write *good* code, then you definitely don't have time to write *bad* code. Writing bad code takes longer than writing good code. Unless you *really* don't care about bugs. And if you *really* don't care about bugs, why write the program at all? – MarkJ Oct 26 '12 at 12:09
  • 5
    @SaeedNeamati `I only say that checking every object to get sure that it's not null, is not a good method` Seriously. It's the best method. And not just null, check every argument for reasonable values. The earlier you catch errors, the easier to find the cause. You don't want to have to backtrack several levels in the stack trace to find the causing call. – jgauffin Oct 26 '12 at 12:25
  • 2
    @SaeedNeamati The trick here is to do it from the start. When you write a method with parameters, the first thing you should do before developping the body of the method is to check the validity of these parameters. Just define a code snippet where you only need to fill out the name of the parameter to check if it's null. It is very easy to do and it's definitely the best method. Every one here agrees except you. – marco-fiset Oct 26 '12 at 13:25
  • Not opposed to the practice of checking everything beforehand, but by the logic of your first two items, `ArgumentNullException` could also go without any information on *which* argument was illegally `null` whatsoever. – O. R. Mapper Feb 09 '13 at 11:56
  • @O.R.Mapper no the parameter in the ArgumentNullException constructor explicitly tells you which parameter is null; – jk. Feb 11 '13 at 09:41
  • @jk.: Exactly - if we followed the logic of the first two items, we could skip that parameter without having any qualms. – O. R. Mapper Feb 11 '13 at 10:38
  • 1
    I like how someone asks a "why" and instead gets a "how" answer. – Mert Akcakaya Jan 19 '21 at 15:03
  • 1
    This "you're doing it wrong" answer does not answer the question in the slightest. Why can't the NullReferenceException provide this information instead of adding boilerplate to every method? Would that not greatly improve ability to diagnose errors if developers neglected to add such checks? Would it not improve readability if such checks could be omitted? Should not a progamming language reduce the effort needed to write correct code and minimize probability of errors? – Elliott Beach Oct 14 '22 at 18:26
  • @ElliottBeach you'd have to ask the .net inventors to be sure. My guess: this is not on by default because it's too expensive cause it would need to store the name of pretty much all arguments as a string. And because it might by a choice to not add the check. And because there are other ways to achieve this (adding it, stacktraces). Though, yes, it would improve readability (that's a rethorical question right?). Which is what was realized so now there's at least `ArgumentNullException.ThrowIfNull`. – stijn Oct 20 '22 at 10:29
  • lol with this argumentation you could make every exception “you have somewhere an error in one of your classes.” Unhandy? You should have written better code!!! – Dirk Boer Oct 20 '22 at 12:45
  • @DirkBoer that's exactly what an exceptions is, no? Quite useless without the stacktrace. – stijn Oct 21 '22 at 07:46
  • @stijn I prefer to have my exceptions as detailed as possible so I can instantly see what is going on when something crashes outside a debugger. I think that should be the default. KeyNotFound? Tell me what key so I instantly have a gutfeeling what it could be. Callstack is part of it, but if you just have a few similar looking parts in one method there is still a lot of ambiguity. Coding practices is a complete unrelated topic imo. – Dirk Boer Oct 21 '22 at 10:33
  • @DirkBoer I basically agree, except: it's not the default, so I'd say result is coding practices are needed to solve it, how unfortunate that may be. – stijn Oct 22 '22 at 11:49
22

It really should show exactly what you are trying to call. It's like saying "There's a problem. You need to fix it. I know what it is. I'm not going to tell you. You go figure it out" Bit like half the answers on this Stack Overflow, ironically.

So how useful would it be, for example, if you got this...

Object reference (HttpContext.Current) not set to instance of an object

...? To have to go into the code, step through it, and work out that the thing you are trying to call is null is fine, but why not just give us a little helping hand?

I agree that it is usually useful to step through the code to get to the answer (because you will probably find out more), but often a lot of time and frustration would be saved if the NullReferenceException text was more like the example above.

Just sayin.

LiverpoolsNumber9
  • 788
  • 1
  • 6
  • 13
  • How is the runtime supposed to know what is null though? – Will Feb 01 '13 at 16:27
  • 3
    The runtime has more information than it provides. Whatever useful info it has, it should provide. That's all I'm saying. In response to your question, why doesn't it know? If you know the answer to that you might be able to provide a better comment than you have. – LiverpoolsNumber9 Feb 11 '13 at 16:55
  • Agreed, and I'd say the same for [`KeyNotFoundException`](http://msdn.microsoft.com/en-us/library/system.collections.generic.keynotfoundexception.aspx) and many other annoyances... – sinelaw Mar 19 '13 at 20:33
  • Actually, in the case of null dereferencing this [looks like a limitation in the way the CLR dereferences](http://stackoverflow.com/questions/14787580/why-doesnt-net-show-which-object-is-null) (thanks to Shahrooz Jefri's comment on the OP's question) – sinelaw Mar 19 '13 at 20:37
  • 1
    https://blogs.msdn.microsoft.com/dotnet/2016/08/02/announcing-net-framework-4-6-2/ FINALLY!!! – LiverpoolsNumber9 Aug 05 '16 at 06:42
4

Your log should include a stack trace - that usually gives you a hint as to which line in the method has the problem. You may need to make your release build include PDB symbols so you have an idea which line the error is on.

Granted, it won't help you in this case:

Foo.Bar.Baz.DoSomething()

The tell don't ask principle can help to avoid such code.

As to why the information isn't included, I'm not sure - I suspect that at least in a debug build, if they really wanted to, they could figure it out. Taking a crash dump and opening in WinDBG may help.

Paul Stovell
  • 1,689
  • 1
  • 9
  • 14
2

Exceptions have been created as a tool to signal exceptional non-fatal conditions up the call chain. That is, they are not designed as a debugging tool.

If a null-pointer exception were a debugging tool, it would abort program execution right on the spot, allowing a debugger to connect, pointing it right at the incriminating line. This would give the programmer all the context information that is available. (Which is pretty much what a Segfault due to a null pointer access does in C, albeit a bit crudely.)

However, the null-pointer exception is designed as a valid runtime condition that can be thrown and caught in normal program flow. Consequently, performance considerations need to be taken into account. And any customization of the exception message requires string objects being created, concatenated, and destroyed at runtime. As such, a static message is undisputably faster.

I'm not saying that the runtime could not be programmed in a way that would yield the name of the incriminating reference, though. That could be done. It would just make exceptions even slower than they are. If anyone cared enough, such a feature could even be made switchable, so that it would not slow down production code, but allow for easier debugging; but for some reason nobody seems to have cared enough.

cmaster - reinstate monica
  • 7,941
  • 1
  • 22
  • 25
1

Cromulent hit the nail on the head I think, however there is the obvious point too that if you're getting a NullReferenceException you have uninitialised variable(s). The argument that you have circa 20 objects being passed into a method cannot be said to be a mitigation: as the creator of a chunk of code you have to be responsible for its doings, that includes its compliance to the rest of a codebase, as well as proper and correct utilisation of variables etc.

it is onerous, tedious and sometimes dull, but the rewards at the end are worth it: many are the times when I have had to trawl through log files weighing in at several gigabytes, and they are almost always helpful. However before you get to that stage, the debugger can help you, and before that stage good planning will save a lot of pain(and I do not mean a fully engineered approach to you code solution either: simple sketches and some notes can and will be better than nothing).

In regards to the Object reference not set to an instance of an object the code cannot guess at values we may like: that's our job as programmers, and it simply means you have passed an uninitialised variable in.

GMasucci
  • 246
  • 2
  • 6
  • You realize people used to offer justifications like this for writing in assembly language? And not using automatic garbage collection? And being able to do arithmetic - in hex? "onerous, tedious and sometimes dull" is how we describe jobs that should be automated. – Spike0xff Sep 28 '15 at 20:34
1

The reason that most languages don't provide this is that it's more work.

The compiler breaks up an expression like A.b.c into two operations:

  1. Get static field A.b
  2. Get field c

The runtime virtual machine executes each one separately. First it gets A.b and sees the result is null. Then it gets null.c. Oops! It throws a NullReferenceException saying you can't get null.c.

Although it's immediately obvious, when executing operation 2, that the thing on the RHS was .c it's not obvious the thing on the left was A.b - it could have been a very complicated expression involving, for example, function calls or ternary operators. Working backwards from the individual operations is not easy, and the only benefit is a slightly more precise error message.

Some language implementations put in the effort to work backwards some of the time (like if the immediately preceding operation is another "get field"); most don't bother. Lua and Java got this feature in the last several years.

user253751
  • 4,864
  • 3
  • 20
  • 27
0

Learn to use the debugger. This is exactly the kind of thing it is designed for. Set a break point on the method in question and away you go.

Simply step through your code and see exactly what the values of all your variables are at certain points.

Edit: Frankly I'm shocked that no one else has mentioned using the debugger yet.

Cromulent
  • 957
  • 1
  • 6
  • 13
  • 2
    So you are saying that all exceptions can easily be reproduced when using a debugger? – jgauffin Feb 20 '13 at 11:54
  • @jgauffin I'm saying that you can use a debugger to see why an exception is being thrown in real world code rather than synthetic unit tests that might not actually fully test the code in question or the unit tests themselves may have bugs in which causes them to miss bugs in the real code. A debugger trumps just about any other tool I can think of (except perhaps things like Valgrind or DTrace). – Cromulent Feb 20 '13 at 12:11
  • 1
    So you are saying that we always have access to the failing computer and that debugging is better than unit tests? – jgauffin Feb 20 '13 at 12:17
  • @jgauffin I'm saying if you have a choice then go with the debugger in preference to other tools. Of course if you don't have that choice then it is a bit of a non-starter. Clearly this is not the case with this question which is why I gave the answer I did. If the question had been how do you fix this issue on a clients computer with no way to do remote debugging (or local debugging) my answer would have been different. You seem to be trying to warp my answer in ways which have no relevance to the question at hand. – Cromulent Feb 20 '13 at 17:14
0

If you wanted to go with @stijn's answer and put null checks in your code, this code snippet should help. Here's some info about code snippets. Once you have this set up, just type argnull, hit tab twice, and then fill in the blank.

<CodeSnippet Format="1.0.0">
  <Header>
    <Title>EnsureArgNotNull</Title>
    <Shortcut>argnull</Shortcut>
  </Header>
  <Snippet>
    <Declarations>
      <Literal>
        <ID>argument</ID>
        <ToolTip>The name of the argument that shouldn't be null</ToolTip>
        <Default>arg</Default>
      </Literal>
    </Declarations>
    <Code Language="CSharp">
      <![CDATA[if ($argument$ == null) throw new ArgumentNullException("$argument$");$end$]]>
    </Code>
  </Snippet>
</CodeSnippet>
user2023861
  • 770
  • 4
  • 10
  • note: c#6 now has `nameof` so your snippet could be `throw new ArgumentNullException(nameof($argument$))` which has the advantages of not including magic constants, being checked by the compiler, and working better with refactoring tools – stijn Aug 22 '15 at 07:55