Note
Access to this page requires authorization. You can try signing in or changing directories.
Access to this page requires authorization. You can try changing directories.
The API Design Guidelines encourage developers to check all their arguments and thereby avoid throwing a NullReferenceException. If an argument is null and the contract of the method forbids null arguments an ArgumentNullException should be thrown.
I get pushback on this periodically from developers that feel like the extra checks they are forced to implement are redundant. Consider the following code:
public void PrintValue(object o)
{
//Insert possibly deep callstackâ€
Console.WriteLine(o.ToString());
}
If you pass an instance that happens to be null nothing will blow-up or AV the process… you will simply get a NullReferenceException. That is way better than the equivalent thing in Win32, so what is the problem? The problem is developer productivity. If I have been up all night working on a program and I hit a NullReferenceException from 10 levels deep into Microsoft code I am likely to assume the bug is in the platform rather than my code (Occam's razor not withstanding). Now a few more hours later and maybe a call to PSS and I will realize my mistake, but I will certainly not be amazingly happy with the platform.
Suppose the author of the PrintValue() method followed our design guidelines
public void PrintValue(object o)
{
if (o == null) throw new ArgumentNullException();
//Insert possibly deep callstackâ€
Console.WriteLine(o.ToString());
}
Now, in the exact same circumstance I get an ArgumentNullException that specifically tells me I passed a null value where it was not expected. I immediately know the error is in my code and I can quickly find and fix my bug. Although I may not be able to attribute it to exactly this instance, I am overall much more happy and productive on the platform.
What do you think? Is this kind of error checking helpful in the .NET Framework and WinFX?
Comments
- Anonymous
July 11, 2004
I use this all a lot, and I enforce it in code reviews. However, I never use the parameterless constructor.. I always use something like:
throw new ArgumentNullException("paramName", resourceManager.GetString(blah)); - Anonymous
July 11, 2004
The comment has been removed - Anonymous
July 11, 2004
I think it is helpful, and use within my own code alot. However I often wonder what sort of performance improvement would be gained in say a tight loop, by not checking the argument.
For example I notice that in the ControlPaint class (System.Windows.Forms), any method that takes System.Drawing.Graphics argument is never checked for null. Is this going really going to increase drawing speed? - Anonymous
July 11, 2004
Sorry, I don't see the difference that much. I can see the point you make, for sure ... but the names of the exceptions are basically the same from my perspective. There's no indication that one is basically an OS-level sigsegv, while the other is a design-by-contract argument assertion. - Anonymous
July 11, 2004
Yes, it's definately a huge advantage and should be done, except in very special performance situations. - Anonymous
July 11, 2004
Yes it's an advantage, but this is the sort of thing developers should be able to declare with design-by-contract, not implement in each case. - Anonymous
July 11, 2004
I do that also. The sooner you catch the bug, the better. Also, this is the kind of feature that you would like to see in the compiler itself, using custom attributes for instance:
public void PrintValue([NonNull] object o)
{
// the compiler will add the check here
Console.WriteLine(o.ToString());
}
The custom attribute nonnull tells the compiler to add non-null checking. - Anonymous
July 11, 2004
No, there's hardly any advantage. The ArgumentNullException can still come from deep in the framework and it can either be a problem in my code or your code. Both exceptions tell me that something was null when it wasn't expected. The only extra information that ArgumentNullException carries is that a function parameter was null as opposed to a local or member variable. This is cool if the exeption is thrown by my immediate callee otherwise it has little value. - Anonymous
July 11, 2004
I dont understand what is the problem? Call stack is always dumped. Thus it's easy to see what has happend to the program and the system.
Method should check for nulls if it's error to use null. But often it's quite natural to use null and the method should accept the null gracefully.
Moreover the programing system could provide some protection against such errors. Such facilities as preconditions should help.
The problem in .NET is existence of untyped references (System.Object) and "null" being instance of every(!) class. Why method declared to accept MyType:
void Method1 (MyType mt);
receives "null" which I did not declare as a possible instance of MyType? If "null" is yet an instance of MyType, why does not it respond to the interface I declared for MyType? If NullReferenceExpection is the response of "null" why have I no mean to declare another behaviour, for example "do nothing"? This would make code more modular and shorter and thereafter maintainable.
I used to skimp cicles and checking every reference everytime it falls into view seems to be out. I write short methods thus the call stack is deep. Some parameters are passed through unchaged and this will lead to multiple checkings of every reference.
Withal often references are surely not equal to null. Sometimes I write two methods: checked and unchecked. The latter is called when parameters are surely not equal to null. The former simply checks parameters and calls unchecked one. Usually I declare unchecked variants as internal. - Anonymous
July 11, 2004
Getting a NullReferenceException at the earliest possible point is certainly useful, but you know what would be MUCH more useful? A bit of "design by contract" so we don't have to write all those null checks manually!
Since most reference parameters can never be null, the Framework should check all such parameters for null when entering a method, and automatically throw an exception if one is null. An optional attribute could be introduced to specify which ref params may be null.
For bonus points, auto-generate XML comments for the null reference exceptions! - Anonymous
July 11, 2004
http://www.torqsoftware.com/blogs/apolon/2004/07/null-checksplumbing-code.html - Anonymous
July 11, 2004
I always do this in my source, and it irritates me to see it missing from other people’s. - Anonymous
July 11, 2004
Ashutosh Nilkanth's Blog » NullReferenceException or ArgumentNullException - Anonymous
July 12, 2004
The comment has been removed - Anonymous
July 12, 2004
Best Practice - yes. Not thoroughly enforced, though.
I say - remove the parameterless constructor for any argument related exception. - Anonymous
July 12, 2004
This approach is very hard to make consistent and therefore is not worse the effort.
Consider the casses then you pass class/struct that contains members which are nulls. Also consider func A calling func B and func B throws InvalidArgument it's a little ugly to get InvalidArgument exception with the argument you never supplied. - Anonymous
July 12, 2004
Aleksei, if you don't want your objects to allow null values then declare them as structs. There's performance issues associated with doing that so you have to decide what's more important. - Anonymous
July 12, 2004
The comment has been removed - Anonymous
July 12, 2004
I’m a big believer in enhancing and providing as much information as possible when providing error handling. We are actually implementing this rule in our source code analyzer to provide for a coding standard. What we have found is that no one (and rarely even ourselves) implements this. If you apply it blindly it seems very excessive. It seems there might be some instances where it is most appropriate. Some of the considerations we have made are:
- public and potentially protected methods - not private (although MSDN states private and protected – which seems opposite of what I would want),
- not in event handlers where inputs are many times passed via system events
- not in properties where arguments are typically just assigned to a private field
What are people’s thoughts on when to apply this standard? Is the above list too limited? Not limited enough? Or is there no general rule that can be applied? - Anonymous
July 12, 2004
Yes, checking arguments in public API's is professional and expected by users. Checking in internal API's is not as important. In VG.net we use static methods, so we can write this:
Check.ArgumentNotNull(o, "o")
Which throws the correct exception with an error message. Because C# does not support macros, the call stack is always 1 level deeper than it should be, to save a bit of typing. - Anonymous
July 12, 2004
The comment has been removed - Anonymous
July 12, 2004
In fact this turns to unit testing. The algorithms should ensure some arguments to be non-nulls. But the end user needs not to know what error happened and where. The user needs speed and usability. In fact this information is useful durng debug stage, but in release those statments may be removed. Conditional compilation should help.
And about struct... Why should I change to value type just to show it cannot be null? And what should I do if this is a nonnullable subclass of pretty nullable one? - Anonymous
July 12, 2004
this kind of check could be useful in debug mode ...
most of these kind of checks are not that useful at compile time, but for maintainance/operations in recovery situations where you don't have the luxury of hours of debugging ... - Anonymous
July 12, 2004
I agree with Stefan. ArgumentNullException is a "developer" exception, in that it shows the developer he/she coded something wrong or something was unexpectedly null. In theory, the exception should never occur once the code is complete, since the developer would have checked their own code before then and done something appropriate. Not sure how the check statement could be optimized away though. - Anonymous
July 13, 2004
Easy to optimize away check statements. To continue my previous example, you can also write:
Check.ArgumentNotNullDebug(...)
and it will be optimized away in the Check class:
[Conditional("DEBUG")]
public static void ArgumentNotNullDebug(object argument, string argumentName)
{
if (argument == null) throw new ArgumentNullException(argumentName);
}
If you have VG.net installed, look at the Check class in the documentation.
http://www.vgdotnet.com - Anonymous
July 13, 2004
Yes. "[Conditional ("DEBUG")]" is what will help to have early alerts and not waste cycles on double checking. Surely this will lower the chance of early detection of an error during normal operation, but I think if error had occurend on the user side it will not be easy to fix after a minute or two. And again the user will not be able to fix the error, thus the detailed info is almost useless. On debugging stage when You are running through the code dozens of times, the seconds lost on deciding what happened, accumulated to minutes and hours. That's the place where this info is useful. And it's more useful if available at compile time. The compiler should be able to alert if a null could treakle to a method which is not ready to accept nothing. That is why the language should provide some means to control if method can accept null, or even if a variable may reference to nothing. - Anonymous
July 14, 2004
The comment has been removed - Anonymous
July 15, 2004
We use Debug.Assert(o!= null, "o cannot be null") to check all input parameters but of course these get compiled out of release assemblies.
I have mixed feelings about the use of this since junior developers may believe they are including a runtime check, forgetting that this is removed in production code.
However it does mean that for developer productivity there is something there to aid debugging and the code is a little more compact than the null checking & exception throwing?
I wondered what anyones opinion was on this use of debug asserts? - Anonymous
July 16, 2004
The comment has been removed - Anonymous
July 16, 2004
Kannan: yes. Unfortunately C# would not privide the facility.
Now they discuss how to catch those quirky nulls. But where do we get them? I often get nulls while dealing with collections. Hashtable returns null if it maps given key to null or if the key is not in the map's domain. But I think in the latter case the hashtable should throw an exception.
...Ah, they say "exception" means something exceptional, extraordinary happened, but absence of a value in a hashtable is a usual practice. "Exception" is not a felicitous name. This could be named "Signal". Hashtable signals if the key is not in the domain...
The item property shoud be declared as
template <T,K>
T Item[K key];
While the key is not in the domain, the hashtable cannot return any object of type T. But it may not return anything except an object of type T according to the declaration. Thus it should not return anything. The only way to leave the method without returning any object is raising a signal or throwing an exception, name it as You wish.
They say throwing an exception affects performance. But double, triple, endless checking for null does affect that performance too. While exception handling made slower starting and leaving of only the outermost method using the dangerous reference, cheking for null runs in every method in the call chain and finally it throws an exception if the reference equals to null.
Another source of nulls I hate is System.Xml. It returns an empty string when the given element does not have the requested attribute at all. Those empty strings actually are reincarnations of nulls to text format.
The code will get a good structure if it had separated blocks for every situation. One block for the case when the attribute present, other blocks receive control on signals. Actualy we do write those blocks with ifs, nested ifs, elseifs.
It would be much better to close null sources and return null if there is actually null. - Anonymous
July 17, 2004
The comment has been removed - Anonymous
July 18, 2004
If performance is really a problem, wrap what would throw NullReferenceException with a try { } catch and throw an ArgumentNullException when the NullReferenceException throws. Frankly, I expect the JIT realizes that you've already checked and doesn't check on its own (it should, if it doesn't). - Anonymous
July 18, 2004
The stack trace is really handy a lot of the time, in my opinion. One might want to catch an exception and log it, to send to a developer or whatnot, for instance. - Anonymous
July 18, 2004
Actually, a smart thing that could be done WRT the stack trace...
One could have some sort of flag bit on each exception handler down the chain which says if it accessed the stack trace, with the assumption for complex things the JIT doesn't want to investigate being 'yes'... most exception handling is pretty simple stuff, at least within the catch { } block, so this might be feasible. Though, exceptions really are supposed to be for exceptional stuff, so there's really no reason to optimize them in my opinion. Like NullReferenceException/ArgumentNullException. If you get those, you're done. The End. Fix your code. I can't really think of any exceptions which aren't like that.
Speaking of exceptions, I must say the liberal use of them is one of the best features of .Net. Using ordinary Win32, one finds that tons of methods don't provide much if any information about what on earth is wrong, leading one to just get mad and do something else unless dealing with the problem is a requirement in the short term. Managed DirectX has this flaw, in that it throws DirectXExceptions which are completely useless and non-descriptive. I hope they fix it at some point. - Anonymous
July 18, 2004
The comment has been removed - Anonymous
July 18, 2004
The comment has been removed - Anonymous
July 19, 2004
The comment has been removed - Anonymous
July 19, 2004
The comment has been removed - Anonymous
July 19, 2004
Note that in many cases signals do not propagate father than the direct caller of the signaling method. Unlike exceptions which often bubble up to the operating system. - Anonymous
February 21, 2008
The comment has been removed