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.

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.