Should I check if the argument is null if I am going to use it immediately?

Question

I'm so used to checking if a method argument is null (then go to the exception), which I hardly even think about. If the argument is a reference type, it is there:

if(arg == null) throw new ArgumentNullException(nameof(arg)); 

But what if I am going to use arg immediately? Should I check anyway? I mean, if I don’t do this, the innument will throw for me (a NullReferenceException) anyway.

For instance:

 public int DoStuff(object o) { return o.GetHashCode(); } 

I could easily add a null check:

 public int DoStuff(object o) { if(o == null) throw new ArgumentNullException(nameof(o)); return o.GetHashCode(); } 

But in both cases, an exception will be thrown (almost on the same line, for debugging purposes). The only difference is the type. Question: In public methods with a single argument of a reference type, if I am going to use the argument immediately, should I still check it if it is null ?

Answer 1

I suggest checking because you have two types of exceptions:

• Unexpected NullReferenceException - something went wrong and you need to debug your own procedure
• ArgumentNullException is a null argument, and it throws the wrong one (and not your code that responds correctly)

throwing ArgumentNullException is a kind of contract: I will make the argument argument valid:

  // An exaggerated example public int DoStuff(SomeObject o) { if (o == null) throw new ArgumentNullException(nameof(o)); else if (o.Disposed) throw new ArgumentException(nameof(o), "o must not be disposed") else if (o.Id > 100) throw new ArgumentOutOfRangeException("Id ...", nameof(o)); // o meets the contract, we accept it and thus we guarantee the output return o.SomeMethod(); } 

Such validation is typical of publicly available methods, since they are subject to the outside world and may come up with any arguments; however, in the case of a private method, you can omit the contract: any caller is a class that implements the method.

Answer 2

Well, it depends;) This is my occupation, and the question (subject) is subjective.

IMO, if the code throws a NullReferenceException, it means that the code itself has no internal consistency. This is a sign of laziness and should be avoided at all costs. If a method throws a NullReferenceException, this is a method error.

ArgumentNullException, on the other hand, means that the method cannot "do this" if the argument is null. This becomes the responsibility of invoker to ensure that the parameter is not null, or to kindly handle the exception.

Bottom line: ArgumentNullException is a contract, NullReferenceException is an error.

Answer 3

I assume that if you immediately raffle a variable, you can discuss it anyway, but I would prefer an ArgumentNullException. Much clearer about what is happening. The exception contains the name of the variable that was null, while the NullReferenceException does not.

Answer 4

I highly recommend that you check for null at the top of the method.

Think of it as a “contract” that indicates the following:

For this method to execute, this argument must be non-null.

Performing a zero check is excellent documentation. You can go even further by using XML comments, for example.

 /// <summary> /// /// </summary> /// <param name="arg1"></param> /// <exception cref="ArgumentNullException">If <paramref name="arg1"/> is NULL.</exception> private static void Foo(object arg1) { } 

Answer 5

I would say that it depends more on how you handle errors, and not how they are thrown. An example will help. Imagine this is a method instead of what you have:

 public static int DoStuff(string o) { return Int.Parse(o); } 

If you have it, then it really doesn't matter.

 public static void main(string[] args) { try { Console.Write("Enter a number: "); value = DoStuff(Console.ReadLine()); } catch(Exception ex) { Console.WriteLine("An exception was thrown. Contents: " + ex.ToString()); } } 

In any case, the flow of programs is the same. But if you have below:

 public static void main(string[] args) { try { int value = 0; do { try { Console.Write("Enter a non-zero number to stop: "); value = DoStuff(Console.ReadLine()); } catch(ArgumentException ex) { Console.WriteLine("Problem with input, try again. Exception message was: " + ex.Message); continue; // Or just nothing, will still loop } } while(value == 0); } catch(Exception ex) { Console.WriteLine("An exception was thrown. Contents: " + ex.ToString()); } } 

Basically, if your error handling doesn't care what type of exception it is, then this is just extra code that probably won't help you. But if you handle certain things differently, then it may be useful to check and throw a more specific type of exception.

But I agree with Dmitry’s message above that the public method matters much more than the private one. Your signature for the method is somewhat similar to a contract. It is best to enforce it.

Answer 6

There are several different scenarios that I think you should consider:

• The method is called from your own program. You do not expect a null value. You are using an object somewhere near the line, and if it is null, it will still change state.
• The method is called from your own program. You do not expect a null value. If this value is null, it will not change state, and everything will remain consistent.
• The method is expanded through a library or API. An unknown X developer calls the method with what you want.
• You just want to know that the argument is not null when implementing the material. To protect yourself and your team members.

Points (1) and (2) refer to the safety of exceptions. This basically means that regardless of your input, the state of your data structures should be consistent. In most cases this is allowed by a simple check; in a more complex scenario, it may involve a rollback or more complex technique. So:

• If you added null to the database, you do not want it to corrupt your database. Now you can, of course, solve null values ​​by simply checking your assumption. If the argument is null , throw an ArgumentNullException because this best describes the script.
• If you call a method that does not expect null and it will not affect the rest of your system, I usually put a comment somewhere and let nature control its course. A NullReferenceException sufficient detail about the nature of the error.
• If it is an API, people expect certain behavior (for example, a ArgumentNullException ). Moreover, as a developer, you do not have to trust a lot of bad looks for you to check anyway.
• If you just want to protect yourself and your team members, use Assertion . Statements are usually implemented in such a way that they do not fall into the release code (or at least do not affect your speed), automatically trigger a breakpoint, and will fail with your unit test. They are also easy to recognize so that they don't add too much bloat to your code.

To summarize, I would not just check everywhere; they tend to make your code unreadable and bloated. However, if I check to ensure the safety of exceptions, it is better to use the exception that best describes the error.

Answer 7

Follow Einstein - "make everything as simple as possible, but not simpler." Does this code do the same without a line of code? If so, it is probably best to remove it.