Best way to check function parameters: Check for null or try/catch

Question

Hello everyone,

when implementing/using methods that return or work with instances of objects, what is the most elegant approach to check the function parameters ?

Method to call:

someType GetSomething(object x)
{
    if (x == null) {
        return;
    }

    //
    // Code...
    //
}

or better:

someType GetSomething(object x)
{
    if (x == null) {
        throw new ArgumentNullException("x");
    }

    //
    // Code...
    //
}

Calling Method:

void SomeOtherMethod()
{
    someType myType = GetSomething(someObject);

    if (someType == null) {
        return;
    }

}

or better:

void SomeOtherMethod()
{
    try {
        someType myType = GetSomething(someObject);
    } catch (ArgumentNullException) {
    }
}

When browsing through similar questions, the reason not to use try/catch is performance. But IMHO the try-catch just looks better :).

So, which way is more "elegant"?

Answer 1

If passing in a null is not valid, throw an exception (i.e. - this is an exceptional situation that should never happen).

If a null parameter is valid, return a corresponding object.

In general, accepting null parameters is bad practice - it goes against the principle of least surprise and requires the called to know it is valid.

Answer 2

You should only use exceptions for exceptional cases. If you expect the argument might be (legitimately) null, you should check it -- do not use exceptions for that. IMO you should check for null at the calling site (before the invocation) if it doesn't make sense to pass null to your method.

Answer 3

1- if your writing a framework then option 2 would be an ideal choice because it's better to inform client code.

Answer 4

Check for null and throw an ArgumentNullException. Performance would be a consideration based on the sample code. Your first example (where you do not throw an exception) is valid only if the caller of the method expects no side effects from the method call; which would seem to be an unlikely scenario.

Answer 5

In your example GetSomthing is private. This means you can track down all of the callers and make sure that Null values don't get passed in e.g.

 if (x != null)
   someType myType = GetSomthing(x)
 else
   // Initialize  x or throw an InvalidOperation or return whatever is correct

However if its not really private then you should do as Oded and others said. Check to see if its null and throw an ArguementExecption in most cases.

Answer 6

As far as elegance is concerned, it's hard to top Code Contracts.

Contract.Requires(x != null);