3

When I code, I often ask myself the same question :

Do I have to verify all arguments are not null ? So, in each method, I will have something like that :

if (arg1 == null) 
{
    throw FooException("...");
}

if (arg2 == null) 
{
    throw FooException("...");
}

If not, in which case is preferable ?

What's the best practices ?

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
Sandro Munda
  • 39,921
  • 24
  • 98
  • 123

6 Answers6

2

As always, it depends.

If you're writing an API to be used by other teams / organizations, such defensive programming with precondition checks on public functions can really help your users; when using an external library, a meaningful error message like 'argument passed to foo() should not be null' is way better than NullPointerException thrown from some inner class.

Outside of API, though, I think such checks clutter the code too much. Thrown NullPointerExceptions are usually pretty easy to trace with debugger anyway. In languages that support them, you can consider using assertions - their syntax is usually less cumbersome, and you can turn them off on production so the checks won't degrade performance.

socha23
  • 10,171
  • 2
  • 28
  • 25
1

Unfortunetly, yes. you should check all arguments. Now ideally, if you code with good design practices one function should not have more than 4 or 5 arguments, at the most.

Having said that, one should always check for null values in function entry and throw appropriate exception or IllegalArgumentException (my fav).

Furhter, one should never pass NULL to a function and should never return a NULL. Sounds simple but it will save lots of code and bugs. Have a look at the NULL Design Pattern too.

Ravi Bhatt
  • 3,147
  • 19
  • 21
  • Good answer mostly. But keep in mind that although the Null Design Pattern is interesting, it is not as widely used as accepting/returning null (I know Qt uses it though). – Thirler Oct 27 '11 at 12:18
0

Depends, if you want different exceptions i guess you would have to do that for all occasions where you might get a null value. Another way would be to user DATATYP.TryParse(). Look that up.

Hope it helps.

Joakim Serholt
  • 403
  • 5
  • 17
0

Since you're throwing an exception anyway, not verifying them would probably just lead to a nullpointerexception or something similar. I'm not entirely sure what the best practices are myself.

Zhanger
  • 1,290
  • 14
  • 19
0

You should ideally always verify any arguments before you perform any action that might modify any state or data associated with said arguments. It's better to fail early and in a manageable way (by throwing a exception) than to end up with an inconsistent state / data which then also has to be resolved.

Your methods are expecting certain data to be there, there are some cases when it should be safe to assume that it is actually there (say inside a private method, which is called from other methods which validate input). However in general I would recommend validating arguments whenever they are:

  • Supplied by a user.
  • Supplied as part of an API.
  • Passed between modules of a system .

It's might be worth taking a look at this previous StackOverflow question.

Community
  • 1
  • 1
Johannes Kommer
  • 6,401
  • 1
  • 39
  • 45
0

I feel it's mostly down to common sense, and a little down to personal preference.

As others have mentioned, if it's a public API then you want to provide clear error messages wherever possible, so it's best to check parameters before they are used and throw exceptions with messages as per your example.

If it's internal code then there are two other options to think about: use assertions, or don't bother with the validation and rely on debugging. As a rule of thumb, I'll put assertions in if it's code that I expect other developers will be calling or if the condition is subtle enough that debugging it might be a pain. Otherwise, I'll just allow it to fail.

Sometimes you can avoid the issue by using the Null Object pattern. If it's a public API I'd still be inclined to include the checks though.

vaughandroid
  • 4,315
  • 1
  • 27
  • 33