Null parameter checking in C#

Yes, there are good reasons:

It identifies exactly what is null, which may not be obvious from a NullReferenceException
It makes the code fail on invalid input even if some other condition means that the value isn't dereferenced
It makes the exception occur before the method could have any other side-effects you might reach before the first dereference
It means you can be confident that if you pass the parameter into something else, you're not violating their contract
It documents your method's requirements (using Code Contracts is even better for that of course)

Now as for your objections:

It's slower: Have you found this to actually be the bottleneck in your code, or are you guessing? Nullity checks are very quick, and in the vast majority of cases they're not going to be the bottleneck
It makes the code harder to maintain: I think the opposite. I think it's easier to use code where it's made crystal clear whether or not a parameter can be null, and where you're confident that that condition is enforced.

And for your assertion:

Obviously, the code that uses s will throw an exception anyway.

Really? Consider:

void f(SomeType s)
{
  // Use s
  Console.WriteLine("I've got a message of {0}", s);
}

That uses s, but it doesn't throw an exception. If it's invalid for s to be null, and that indicates that something's wrong, an exception is the most appropriate behaviour here.

Now where you put those argument validation checks is a different matter. You may decide to trust all the code within your own class, so not bother on private methods. You may decide to trust the rest of your assembly, so not bother on internal methods. You should almost certainly validate the arguments for public methods.

A side note: the single-parameter constructor overload of ArgumentNullException should just be the parameter name, so your test should be:

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

Alternatively you can create an extension method, allowing the somewhat terser:

s.ThrowIfNull("s");

In my version of the (generic) extension method, I make it return the original value if it's non null, allowing you to write things like:

this.name = name.ThrowIfNull("name");

You can also have an overload which doesn't take the parameter name, if you're not too bothered about that.

Update .NET 6

There is a new method in .NET API which simplifies null check syntax.

 ArgumentNullException.ThrowIfNull(someParameter);

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.

I've been using this for a year now:

_ = s ?? throw new ArgumentNullException(nameof(s));

It's a oneliner, and the discard (_) means there's no unnecessary allocation.

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>

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.

Null parameter checking in C#

Update .NET 6

Related

Recent Posts