Proper exceptions to use for nulls_问答_开发者

In the following 开发者_运维知识库example we have two different exceptions we want to communicate.

//constructor
public Main(string arg){
   if(arg==null)
      throw new ArgumentNullException("arg");

  Thing foo=GetFoo(arg);

  if(foo==null)
     throw new NullReferenceException("foo is null");    
}

Is this the proper approach for both exception types?

The first exception is definitely correct. It's the second one which is tricky.

There are two possibilities here:

GetFoo() isn't meant to return null, ever. In that case we've basically proven a bug in GetFoo(). I'm not sure of the best exception here, leaving ContractException (from Code Contracts) aside. Basically you want something like ContractException - an exception which means "The world has gone crazy: this isn't just an externally unexpected result, there's a bug here."
GetFoo() can legitimately return null, due to arg's value. In this case I would suggest that ArgumentException (but not ArgumentNullException) may be appropriate. On the other hand, it's odd to throw ArgumentException after using the argument.

InvalidOperationException isn't quite appropriate here, but I might be tempted to use it as the closest thing to a contract failure...

EDIT: You should also consider creating your own exception, as per Aaronaught's answer.

You should never explicitly throw a NullReferenceException.

If null was passed as a parameter, you should throw an ArgumentNullException with the name of the parameter.
If some other thing is null, you should probably throw an InvalidOperationException with a descriptive message.

Never throw NullReferenceException. That doesn't mean that a null was passed. It means that there was an attempt to use the null.

The ArgumentNullException is obviously correct and for the second one it depends on your business context.

ArgumentNullException:

The exception that is thrown when a null reference [...] is passed to a method that does not accept it as a valid argument.

NullReferenceException:

The exception that is thrown when there is an attempt to dereference a null object reference.

"foo is null" is a poor error message, since foo is a local variable. A better error message would be "GetFoo returned null for the input " + arg. Also, if an Exception will be thrown whenever GetFoo returns null, make it throw the appropriate exception.

ArgumentNullException is an obvious choice for the first check.

Since it appears that a Thing is derived from an input parameter I would throw an ArgumentException to indicate that a Thing cannot be constructed from the specified input. Afterall, it is (presumably anyway) a problem with the input and not the algorithm used to construct the Thing.

For the first case, I'll pile on and say that ArgumentNullException is correct.

For the second case, I'm really very surprised that nobody else has said this: You should be making your own Exception class for that. None of the built-in system exceptions are really appropriate:

ArgumentException implies that the argument itself was invalid in some way; that's not really the case here. The argument was fine, it's just that something unexpected happened later.
InvalidOperationException is almost correct, but that exception is generally interpreted to mean that an operation was invoked at the wrong time, such as trying to execute a command on a connection that hasn't been opened yet. In other words, it indicates a mismatch between the current state of the object and the specific operation you tried to perform; that's really not applicable here either.
NullReferenceException is right out. That's a reserved exception that means something completely different (that the program actually tried to deference the null reference).

None of these are right. What you really need to be doing is communicated specifically what went wrong, and in order to do that, you should create a MissingThingException. That exception can include the ID of the thing (presumably arg) in its message/detail. This is the best for callers, because it allows them to catch the specific exception if they know how to handle it, and also the best for end users, because it allows you to leave a meaningful error message.

Summary: Create a custom exception class for this.

Per the Code Analysis blog, Exception usage should be as follows:

ArgumentOutOfRangeException if the input value you are testing is not within an expected set of values. (e.g. if the parameter represents a command name, and the provided command name is invalid)
ArgumentNullException if the input value cannot be null in order to execute the function.
ArgumentException if the input value is invalid (e.g. if you pass in an empty string, but empty strings are not allowed)

I think that these are both correct. For me, choosing an exception type is really just about whether or not it will clearly portray the error that has occured. I always think about it from the perspective of another dev that hasn't seen my class before. Will this other dev be provided with enough information to quickly and accurately locate the problem?