Null parameter checking in C#

Question

In C#, are there any good reasons (other than a better error message) for adding parameter null checks to every function where null is not a valid value? Obviously, the code that uses s will throw an exception anyway. And such checks make code slower and harder to maintain.

void f(SomeType s)
{
  if (s == null)
  {
    throw new ArgumentNullException("s cannot be null.");
  }

  // Use s
}

Answer 1

It saves some debugging, when you hit that exception.

The ArgumentNullException states explicitly that it was "s" that was null.

If you don't have that check and let the code blow op, you get a NullReferenceException from some unidentified line in that method. In a release build you don't get line numbers!

Answer 2

I agree with Jon, but I would add one thing to that.

My attitude about when to add explicit null checks is based on these premises:

• There should be a way for your unit tests to exercise every statement in a program.
• throw statements are statements.
• The consequence of an if is a statement.
• Therefore, there should be a way to exercise the throw in if (x == null) throw whatever;

If there is no possible way for that statement to be executed then it cannot be tested and should be replaced with Debug.Assert(x != null);.

If there is a possible way for that statement to be executed then write the statement, and then write a unit test that exercises it.

It is particularly important that public methods of public types check their arguments in this way; you have no idea what crazy thing your users are going to do. Give them the "hey you bonehead, you're doing it wrong!" exception as soon as possible.

Private methods of private types, by contrast, are much more likely to be in the situation where you control the arguments and can have a strong guarantee that the argument is never null; use an assertion to document that invariant.

Answer 3

Original Code:

void f(SomeType s)
{
  if (s == null)
  {
    throw new ArgumentNullException("s cannot be null.");
  }

  // Use s
}

Rewrite it as:

void f(SomeType s)
{
  if (s == null) throw new ArgumentNullException(nameof(s));
}

[Edit] The reason to rewrite using nameof is because it allows for easier refactoring. If the name of your variable s ever changes then the debugging messages will be updated as well, whereas if you just hardcode the name of the variable then it will eventually be outdated when updates are made over time. It's a good practice used in industry.

Answer 4

Without an explicit if check, it can be very difficult to figure out what was null if you don't own the code.

If you get a NullReferenceException from deep inside a library without source code, you're likely to have a lot of trouble figuring out what you did wrong.

These if checks will not make your code noticeably slower.

---

Note that the parameter to the ArgumentNullException constructor is a parameter name, not a message.
Your code should be

if (s == null) throw new ArgumentNullException("s");

I wrote a code snippet to make this easier:

<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippets  xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
    <CodeSnippet Format="1.0.0">
        <Header>
            <Title>Check for null arguments</Title>
            <Shortcut>tna</Shortcut>
            <Description>Code snippet for throw new ArgumentNullException</Description>
            <Author>SLaks</Author>
            <SnippetTypes>
                <SnippetType>Expansion</SnippetType>
                <SnippetType>SurroundsWith</SnippetType>
            </SnippetTypes>
        </Header>
        <Snippet>
            <Declarations>
                <Literal>
                    <ID>Parameter</ID>
                    <ToolTip>Paremeter to check for null</ToolTip>
                    <Default>value</Default>
                </Literal>
            </Declarations>
            <Code Language="csharp"><![CDATA[if ($Parameter$ == null) throw new ArgumentNullException("$Parameter$");
        $end$]]>
            </Code>
        </Snippet>
    </CodeSnippet>
</CodeSnippets>

Answer 5

The main benefit is that you're being explicit with the requirements of your method right from the start. This makes it clear to other developers working on the code that it is truly an error for a caller to send a null value to your method.

The check will also halt the execution of the method before any other code executes. That means you won't have to worry about modifications being made by the method that are left unfinished.

Answer 6

You might want to take a look at Code Contracts if you need a nicer way to make sure you do not get any null objects as a parameter.