Checking for null parameter

[pek]

So is checking for null pointer a good practice? I mean, 90% of the
times you have any object as a parameter, you really want that object
to be something other than null. Otherwise you have to blot your code
into an unlimited number of lines of try, catch etc (and sometimes you
will fail doing any correct error handling).

Just to make my question more clear, here is an example:

class Test {
private A a;
public Test(A a) throws Exception {
if ( a == null ) throw new Exception("Null!");
this.a = a
}
}

class Hell {
private Devil d;
private Fire f;
public Hell(Devil d, Fire f) throws Exception {
if ( ( d == null ) || ( f == null ) ) throw new
Exception("Null!");
this.d = d; this.f = f;
}
public void burnAngel(Angel a) {
if ( a == null ) throw new Exception("Null!");
a.burn();
}
}

class Main {
public static void main(String[] args) {
try {
Test t = new Test(new A());
} catch (Exception e) {}
try {
Hell h = new Hell(new Devil(), new Fire());
} catch (Exception e){}
}
}

So, is this all necessary? I mean, if A, Devil, Fire and Angel is null
and I know that they should not be, then that means that I have an
error in my code. There is no reason to explicity tell someone that
the object should not be null. If I do, I have to try catch almost 80%
of my code.

What do you think? If checking for null overkill or not?

 

[ttrifonov]

Hi,

It's very bad idea to throw java.lang.Exception for null parameters.
It's better to throw NullPointerException or IllegalArgumentException.
These exception are run-time exception so you don't need to use any
try-catch blocks. It's good idea to add a comment in javadoc what will
happened with null values and who is responsible to check for them.

Regards,
Tanyu

 

[Ulrich Eckhardt]

So is checking for null pointer a good practice? I mean, 90% of the
times you have any object as a parameter, you really want that object
to be something other than null.

If a function is documented to accept null, I would know it does. Otherwise,
I would assume it doesn't accept null.

Otherwise you have to blot your code
into an unlimited number of lines of try, catch etc (and sometimes you
will fail doing any correct error handling).

Cluttering code in microscopic try-catch-clauses is IMHO a typical beginners
misconception about how to use exceptions. Maybe it's also (bad) influence
from programming languages like C (or badly taught C++..) that require the
programmer to always check returnvalues.

class Test {
private A a;
public Test(A a) throws Exception {
if ( a == null ) throw new Exception("Null!");
this.a = a
}
}

Two things:
1. There is a dedicated NullPointerException.
2. Dereferencing ("using") 'a' in above example would throw that exception
if 'a' was 'null'. Depending on the circumstances, you might want to get
that exception early (like when only storing it) or can live with getting
it later (i.e. when finally using it).

class Hell {
private Devil d;
private Fire f;
public Hell(Devil d, Fire f) throws Exception {
if ( ( d == null ) || ( f == null ) ) throw new
Exception("Null!");
this.d = d; this.f = f;
}
public void burnAngel(Angel a) {
if ( a == null ) throw new Exception("Null!");
a.burn();
}
} [...]
try {
Hell h = new Hell(new Devil(), new Fire());
} catch (Exception e){} [...]
So, is this all necessary? I mean, if A, Devil, Fire and Angel is null
and I know that they should not be, then that means that I have an
error in my code. There is no reason to explicity tell someone that
the object should not be null. If I do, I have to try catch almost 80%
of my code.

What do you think? If checking for null overkill or not?

Try to make that code more realistic. First you create hell, then burn some
angels. In either case, if fire is null, you end up with an exception in
the catch handler. The only difference is that in one case it is a bit
earlier and in the other a bit later. I'd go for the 'later', because the
additional effort of checking every reference if it is null just clutters
the code.

Uli

 

[pek]

A method must be coded to handle any input that can be sent to it. Otherwise
it will break when it gets the wrong input.

What's the matter, is it too hard to copy-and-paste a try-catch block? If
that's too much effort, change jobs and be a ditch digger.

The purpose of those try-catch blocks (and other logic to handle anomalous
input) is to make it possible to fix the program when something bad happens.

Generally, design your algorithms so that undesired inputs are never passed in
the first place. Your keywords to google are "algorithm invariants" and the
Java 'assert' keyword. Warning: 'assert' is not for input checking! Its
purpose is to enforce algorithm invariants.

What you have to check in private methods is different from what you have to
check in public methods, on account of you completely control calls into
private methods but not others.

If 'null' is a possible input, and you need to handle it differently from
other values, then you absolutely, inexorably and inarguably must check for it.

Every time.

Without fail.

So, since 90% of the times I need a non-null object in my code, I
should throw an exception. So that means that 80% of any code that
calls a method or constructs an object that needs another object
should be inside a try catch.

The problem isn't the time it takes to write the code, I do, after
all, use an IDE that does it automatically. The problem is the
complexity of the error handling. Consider the following code:

public doSomething() throws Exception {
try {
Test t = new Test(this.aString);
...
} catch (Exception e) { }
// I don't care if Test threw an exception
try {
TestAgain a = new TestAgain();
...
// I do care if TestAgain threw an exception, so continue in the
try
// But I want to handle Tester exception differently
Tester ts = null;
try {
ts = new Tester(a);
} catch (Exception e) { throw e }
...
ts.doSomething();
}
}

Assuming that exceptions are thrown only in case of a null object,
that looks ugly.

Besides, Java it self doesn't excplicity say that a method throw an
Exception for null, it throw a run-time NullPointerException whenever
the object is being used. So, the problem is you, because somewhere in
your code you didn't do something correct. Otherwise all methods and
claases in Java would have a signature of "throws Exception" and then
all your code would be in a try catch.

Oh, have in mind that this is more than I theoritical question than an
opinion. I'm not saying that null checking should be avoided.

 

[pek]

Hi,

It's very bad idea to throw java.lang.Exception for null parameters.
It's better to throw NullPointerException or IllegalArgumentException.
These exception are run-time exception so you don't need to use any
try-catch blocks. It's good idea to add a comment in javadoc what will
happened with null values and who is responsible to check for them.

Regards,
Tanyu

That where exactly my thoughts. And since NullPointerException is
thrown "automatically" when trying to use a null object, you don't
even have to throw it. Just stating in the Javadoc that the objects
should not be null should be enough.

I mean, I consider handling object when they are null a special case,
not standard! I always want my objects not to be null!

 

[pek]

If an exception is thrown, runtime or not, it's a pretty darn good idea that
the calling code have a catch block for it, if one doesn't take the care to
pass only valid values in the first place.

Given a method 'foo()' that can throw an IllegalArgumentException, the best
practice is to pass only valid values to it in the first place:

if ( arg == null )
{
handleNullSituation();
}
else
{
foo( arg );
}

NOW you don't need a try-catch, but if you don't do the pre-check, then you
absolutely would need to catch the exception.

So, what do you consider a better way of coding? Doing a pre-check,
thus avoiding all your methods to have an exception handling a null
state, or having try catch everywhere?

Let me rephrase my question: How often does a method or constructor
that accepts an object in it's parameter list CAN WORK if the object
is null?

 

[pek]

If a function is documented to accept null, I would know it does. Otherwise,
I would assume it doesn't accept null.


Cluttering code in microscopic try-catch-clauses is IMHO a typical beginners
misconception about how to use exceptions. Maybe it's also (bad) influence
from programming languages like C (or badly taught C++..) that require the
programmer to always check returnvalues.


Two things:
1. There is a dedicated NullPointerException.
2. Dereferencing ("using") 'a' in above example would throw that exception
if 'a' was 'null'. Depending on the circumstances, you might want to get
that exception early (like when only storing it) or can live with getting
it later (i.e. when finally using it).

That exactly my point. Trying to figure out when do you have to check
for an object that is null brings complexity in your code. If you
decide that you will throw an exception when you will actually use it,
you have to think "Is it too late? Maybe I should have thrown it when
I was storing it.". Isn't it obvious that a would have SOME use
eventually? Otherwise it wouldn't be in the constructor in the first
place. If the method could work even when a is null, wouldn't it be
more convinient to state it in the javadoc?

class Hell {
private Devil d;
private Fire f;
public Hell(Devil d, Fire f) throws Exception {
if ( ( d == null ) || ( f == null ) ) throw new
Exception("Null!");
this.d = d; this.f = f;
}
public void burnAngel(Angel a) {
if ( a == null ) throw new Exception("Null!");
a.burn();
}
} [...]
try {
Hell h = new Hell(new Devil(), new Fire());
} catch (Exception e){} [...]
So, is this all necessary? I mean, if A, Devil, Fire and Angel is null
and I know that they should not be, then that means that I have an
error in my code. There is no reason to explicity tell someone that
the object should not be null. If I do, I have to try catch almost 80%
of my code.

What do you think? If checking for null overkill or not?

Try to make that code more realistic. First you create hell, then burn some
angels. In either case, if fire is null, you end up with an exception in
the catch handler. The only difference is that in one case it is a bit
earlier and in the other a bit later. I'd go for the 'later', because the
additional effort of checking every reference if it is null just clutters
the code.

Hahahahahhahahah! I didn't think of creating a realistic example. The
word Hell just pop in my head and I named all the other a little
relevant. But I loved the way you looked at it

 

[Robert]

pek a écrit :

So is checking for null pointer a good practice? I mean, 90% of the
times you have any object as a parameter, you really want that object
to be something other than null. Otherwise you have to blot your code
into an unlimited number of lines of try, catch etc (and sometimes you
will fail doing any correct error handling).

Just to make my question more clear, here is an example:

class Test {
private A a;
public Test(A a) throws Exception {
if ( a == null ) throw new Exception("Null!");
this.a = a
}
}

class Hell {
private Devil d;
private Fire f;
public Hell(Devil d, Fire f) throws Exception {
if ( ( d == null ) || ( f == null ) ) throw new
Exception("Null!");
this.d = d; this.f = f;
}
public void burnAngel(Angel a) {
if ( a == null ) throw new Exception("Null!");
a.burn();
}
}

class Main {
public static void main(String[] args) {
try {
Test t = new Test(new A());
} catch (Exception e) {}
try {
Hell h = new Hell(new Devil(), new Fire());
} catch (Exception e){}
}
}

So, is this all necessary? I mean, if A, Devil, Fire and Angel is null
and I know that they should not be, then that means that I have an
error in my code. There is no reason to explicity tell someone that
the object should not be null. If I do, I have to try catch almost 80%
of my code.

What do you think? If checking for null overkill or not?

Burning Angel ? have you lost your mind ?

 

[Robert]

Lew a écrit :

Prevention is better than curing.


Your question is syntactically incomplete, and I am not sure what you
are asking.

Also, an object cannot be 'null'. A reference can be.

You seems a little bit picky.

If you're asking for statistics on how many classes have constructors
that accept 'null' parameters, it's a meaningless and pointless

kind of harsh. I guess pek was thinking about statistics on nullness

question. In the first place, *every* non-private constructor that
takes a reference parameter must work if the argument is null. Failure
to deal with a possible input is a flaw in the code - programmer error.

Lew said

 

[Jack]

Lew a écrit :

Best practice is to prevent errors. Failing that, catch them early.

"Just clutters the code" is not a valid criticism. If it is necessary
to check for an exception, clutter or not, it must be done.

The reason to check early is to facilitate correction of problems. The
later a bug is caught and logged, the harder it is to discern the true
cause.

There are ways to make code easier to read and maintain, but the
sacrifice of good engineering principles is not among them.

To get "uncluttered" code, structure your algorithms so that non-nullity
of a variable is an 'assert'able invariant. NPEs will become
impossible, obviating the need for corresponding catch blocks. Then you
get your lack of "clutter" as a concomitant to good engineering, not a
violation thereof.

Yet some invariant tools sometimes throw NPE (ESCJava2 in my memory).

 

[pek]

Prevention is better than curing.


Your question is syntactically incomplete, and I am not sure what you are
asking.

Also, an object cannot be 'null'. A reference can be.

I thought that this was obvious.

If you're asking for statistics on how many classes have constructors that
accept 'null' parameters, it's a meaningless and pointless question.

No, my question is: How often a null object reference is NOT an
error?

In the
first place, *every* non-private constructor that takes a reference parameter
must work if the argument is null. Failure to deal with a possible input is a
flaw in the code - programmer error.

Exactly! So why do you have to throw an exception or even check if the
object is a null reference since a NullPointerException will be thrown
anyway? And if a NullPointerException is thrown, your code has a
problem. I believe that this is why NullPointerException is a run-time
exception and not a compile time..

 

[Arved Sandstrom]

So, what do you consider a better way of coding? Doing a pre-check,
thus avoiding all your methods to have an exception handling a null
state, or having try catch everywhere?

[ SNIP ]

In Lew's example above it's simply the case that method "foo" expects a
non-null argument by contract. A method like that is under no obligation to
do anything other than throw an NPE if it gets a null argument.

What Lew is pointing out is this: the code that calls "foo" actually has two
valid paths of execution - one, "arg" is null, and two, "arg" is not null.
Let's say that the calling code is in method "bar"...this method considers a
null value of arg" to be legitimate, checks for it, and does
"handleNullSituation" with it. "bar" handles the non-null case by calling
"foo" - as we've said, "foo" does *not* consider a null argument to be
valid.

Let's consider another example, that can see null values in several
different ways. Your method "bar" is using method "get" in interface Map.
This returns null if there is no mapping for the key or the key maps to
null. First note - "get" throws an NPE if the map does not permit null keys
(like Hashtable)...this is an example of a method simply not accepting a
null argument at all. So you potentially have 4 possibilities here:

1) "get" throws an NPE;
2) "get" returns a null and you determine that the key exists;
3) "get" returns a null and the key does not exist;
4) "get" returns a non-null.

Case 1 shouldn't happen, but eventually will. You should be aware of whether
your map does or does not accept null keys - it it doesn't, but *your*
method (namely "bar") could *legitimately* get null keys, check for a null
key and do something else. If your calling code (namely "bar") does *not*
expect to ever get a null value to pass to "get", call "get" all the time,
and if "get" (see below, it's actually "containsKey" that barfs) throws an
NPE just let that percolate up from "bar" as well - whatever called "bar"
wasn't doing what it should have been doing.

Cases 2, 3 and 4 are handled by a construct similar to what Lew has:

if (someMap.containsKey(key)) {
Object obj = someMap.get(key);
if (obj == null) {
// key exists but maps to null
handleNullValue();
} else {
// "foo" expects a non-null argument
foo(key);
}
} else {
// handle situation where key is not mapped at all
handleNonmappedKey();
}

Note that in the above snippet, if the map does not accept null keys, the
snippet itself is behaving like method "foo" - it does not expect null keys
and makes no effort to handle them. In this situation (the map does not
accept null keys) it's up to "bar" to look for them *if* "bar" itself
expects null keys. If even "bar" never expects to see null keys, neither
"bar", the above code snippet, nor "foo" make any effort to check for null
keys...just let the NPE percolate up.

And where the map can accept null keys, the above code snippet behaves
reasonably. Note: if it's for some reason important to you (for a map that
can accept null keys) to differentiate between the key being null and the
key just not being in the map, "bar" can deal with that too (either in the
else block or before the code snippet is reached).

I hope this example helped. To me null handling is a simple question of,
where do you expect to see it, both as arguments and return values? In the
above, depending on the situation, "bar" is potentially doing null checks
both on arguments it gets and on return values from methods it calls, and is
also aware of how the methods it calls handle null arguments. And as I've
indicated, you look for and handle null if you expect to see it...if you
don't, let an NPE happen.

AHS

 

[Tom Anderson]

Also, an object cannot be 'null'. A reference can be.

Ahem:

http://c2.com/cgi/wiki?NullObject

Although of course that's really something quite different!

tom

 

[Tom Anderson]

A method must be coded to handle any input that can be sent to it.
Otherwise it will break when it gets the wrong input.

This is very silly advice.

What's the matter, is it too hard to copy-and-paste a try-catch block?

Around *every* *single* *method* (or code block, or whatever granularity
you fancy)? Yes. Yes it is.

What you have to check in private methods is different from what you
have to check in public methods, on account of you completely control
calls into private methods but not others.

This is the crux of it - some methods do need hardening against duff
parameters, like nulls, NaNs, out-of-range integers, objects that are
invalid in more subtle ways, etc, but not all. You say all public methods,
but i think that's overkill. Firstly, it only needs to be on exported
methods - methods that you expect outside code to call; the main entry
points to the component or package or whatever. If outside code wants to
call your internal methods with duff arguments, fine, let it handle the
consequences. Secondly, i'm generally quite happy to have code screws up
when given a null, as long as it's clear that i shouldn't pass a null.
That again hands the responsibility for not passing in a null to the
caller.

It would be nice if java could express a requirement for non-nullity in
the type system. Some type systems forbid nulls from normal variables, and
if you want a variable that can be null, you have to call it an
"Option<Foo>" or something. I quite like this idea - most variables
shouldn't be null, so make that the default, but provide a way of having
them be null when that's okay. A simple syntax would be a question mark
after the type - implying it might be one of those, or it might not:

String foo = null ; // compile-time error
String? bar = null ; // that is okay
String baz = bar ; // also a compile-time error - can't assign from ? to not-?
String qux = (String)bar ; // okay, albeit icky

That's probably in Scala or something.

Hey, maybe we could use ! to mean const. HHOK!

tom

 

[Tom Anderson]

You might not realize this, but angels put out more BTU than good
hickory.

The fumes are dreadful, though. Make sure your stovepipe is in good
working order.

tom

 

[Ulrich Eckhardt]

If an exception is thrown, runtime or not, it's a pretty darn good idea
that the calling code have a catch block for it, if one doesn't take the
care to pass only valid values in the first place.

I disagree. You should not catch exceptions you can not handle. Maybe it's
not what you meant, but if I read the above I imagine code that calls each
function that could throw in a microscopic try-catch clause, i.e. a code
that is completely cluttered with error handling code. Coding like that
completely obliterates the whole advantage of exceptions in the first
place, then you can go back to just using returncodes like in C.

Given a method 'foo()' that can throw an IllegalArgumentException, the
best practice is to pass only valid values to it in the first place:

if ( arg == null )
{
handleNullSituation();
}
else
{
foo( arg );
}

NOW you don't need a try-catch, but if you don't do the pre-check, then
you absolutely would need to catch the exception.

....or live with the fact that the code calling this code will have an
exception thrown at it, just like this code does.

Uli

 

[Daniel Pitts]

So is checking for null pointer a good practice? I mean, 90% of the
times you have any object as a parameter, you really want that object
to be something other than null. Otherwise you have to blot your code
into an unlimited number of lines of try, catch etc (and sometimes you
will fail doing any correct error handling).

If it is truly an error to pass in null to a method, allow the JVM to
product a null-pointer exception. If you can handle (null) as a special
case, then you should check for null. If you want to be as informative
as possible, check for null, and throw a NullPointerException with a
descriptive message (not just "Null!").

Just to make my question more clear, here is an example:

class Test {
private A a;
public Test(A a) throws Exception {
if ( a == null ) throw new Exception("Null!");
this.a = a
}
}

class Hell {
private Devil d;
private Fire f;
public Hell(Devil d, Fire f) throws Exception {
if ( ( d == null ) || ( f == null ) ) throw new
Exception("Null!");
this.d = d; this.f = f;
}
public void burnAngel(Angel a) {
if ( a == null ) throw new Exception("Null!");
a.burn();
}
}

class Main {
public static void main(String[] args) {
try {
Test t = new Test(new A());
} catch (Exception e) {}
try {
Hell h = new Hell(new Devil(), new Fire());
} catch (Exception e){}
}
}

Why do you have these in two different try/catch blocks, and why do you
just throw the exception away? You have made try/catch blocks both
worthless AND burdensome, when it can easily be useful and relatively
painless.

So, is this all necessary? I mean, if A, Devil, Fire and Angel is null
and I know that they should not be, then that means that I have an
error in my code. There is no reason to explicity tell someone that
the object should not be null. If I do, I have to try catch almost 80%
of my code.

You should put the possible causes of null pointer exceptions in your
JavaDoc. for example.
/**
* Initializes Hell
* @throws a NullPointerException if devil or fire is null.
*/
public Hell(Devil devil, Fire fire) {
if (devil == null || fire == null) {
throw new NullPointerException();
}
}


Note that NullPointerException is an unchecked exception, since it
generally indicates that the programmer has made a mistake.

What do you think? If checking for null overkill or not?

Absolutely not, however, throwing an Exception, rather than an NPE is
overkill, and trying to catch exceptions caused by nulls that could be
avoided is bad practice.

Hope this helps,
Daniel.

 

[Stefan Ram]

Cluttering code in microscopic try-catch-clauses is IMHO a
typical beginners misconception about how to use exceptions.
Maybe it's also (bad) influence from programming languages like
C (or badly taught C++..) that require the programmer to always
check returnvalues.

When a method does not catch checked exceptions it receives
from the operations it invokes, those checked exceptions will
be thrown from this method an therefore they become part of
the interface of this method.

When the implementation of this method will be changed, those
checked exceptions might change, too, because now other
operations will be invoked, so other checked exceptions might
need to be declared, so the interface will need to change, too.

To avoid this, a method might catch checked exceptions and
wrap them in objects of a custom exception type, so that the
interface will not have to change when the implementation
will change.

 

[Daniel Pitts]

Sure, if you don't have any logging or exception handling. But that
possibility should have been prevented in the first place - just letting
an NPE happen is a bad, bad practice.

It depends on where it happens. If a method can't reasonable be
designed to handle null, then it should throw an NPE if it is given a
null. If the calling code can prevent a null from being passed in, than
it should do so, and then no NPE will be generated.

In general, NPE's usually caused by programmer error, not bad input.
Although the practice holds the same. If a precondition is required for
a method to function properly, it must be met by the calling code, or
the method should reasonably throw an exception. If the exception could
have been anticipated and prevented at the client call site, a unchecked
exception should be used, otherwise a suitable checked exception should
be used.

 

[Roedy Green]

So is checking for null pointer a good practice? I mean, 90% of the
times you have any object as a parameter, you really want that object
to be something other than null. Otherwise you have to blot your code
into an unlimited number of lines of try, catch etc (and sometimes you
will fail doing any correct error handling).

The nice thing about Java is it will check for null and throw an
exception without writing any code.. If all that is you are going to
do, there is not much point in manual checking. If however you can
dodge the null and carry on --where null means there is no such thing,
rather than a coding error, then of course you have to check for it.