Hi,
When there is a post-condition, that return value of a method must not be null, what can be done ?
I could do
assert returnValue != null : "Not acceptable null value";
but assertions could be turned off !
So is it okay to do
if(returnValue==null)
{
throw new NullPointerException("return value is null at method AAA");
}
?
Or is it better to use a user-defined exception (like NullReturnValueException ) for such a condition ?
Given that NullPointerException is the idiomatic way to communicate an unexpected null value in Java, I would recommend you throw a standard NullPointerException and not a homegrown one. Also keep in mind that the principle of least surprise would suggest that you don't invent your own exception type for a case where a system exception type exists.
Assertions are good for debugging but not good if you have to handle certain conditions so that's not really a good way to handle the error condition.
A book I have called O'Reilly's Java in A Nutshell which is written by an expert lists this definition for NullPointerException:
Signals an attempt to access a field or invoke a method of a null object.
Since returning null isn't either of those things, I think it'd be more appropriate to write your own exception.
The JavaDoc for NullPointerException states:
Thrown when an application attempts to use null in a case where an object is required. These include:
* Calling the instance method of a null object. * Accessing or modifying the field of a null object. * Taking the length of null as if it were an array. * Accessing or modifying the slots of null as if it were an array. * Throwing null as if it were a Throwable value.Applications should throw instances of this class to indicate other illegal uses of the null object.
I consider violating the post-condition an illegal action. However, I think what exception you use doesn't matter much, because we are talking about a code path that should be (and hopefully is) unreachable, and hence you will not have error handling specific to that exception, and hence the only effect of that name is a different wording of some entry in a log file nobody is ever likely to see.
If in contrast you think the post condition is likely to be violated it might be a good idea to include more debugging information, such as the arguments the method was invoked with.
There certainly isn't a universal law against throwing NullPointerException, but it's tough to answer if you actually should in such an abstracted example. What you don't want to do is put people up the chain in the position of trying to catch NullPointerException. Code like this (real example, I swear):
catch (NullPointerException npe) {
if (npe.getMessage().equals("Null return value from getProdByCode") {
drawToUser("Unable to find a product for the product type code you entered");
}
}
Is a surefire indicator you're doing something wrong. So if the null return value is an indicator of some system state that you're actually able to communicate, use an exception that communicates that state. There aren't many cases I can think of where it makes sense to null check a reference just to chuck a nullpointer. Usually the very next line of code would have chucked the nullpointer (or something more informative) anyway!
I would consider that usage of NullPointerException ok, if you remember the description. That is what the person investigating has work with (line numbers may shift). Also remember to document that your methods throw null pointer exceptions in special cases.
If you check your method parameters right in the beginning, a throw new IllegalArgumentException("foo==null") is acceptable to me too.
I would recommend you never throw NullPointerException by yourself.
If you want to tell to your API user that null is not a valid argument value then throw IllegalArgumentException, it exists just for the case.
Another (more modern imho) option is to use @NotNull annotation near the argument.
IMO you should never manually throw a NullPointerException. The calling routine wouldn't know if the real or manual NullPointerException without checking the description. In this case it looks like you would want to roll your own exception that matches the problem closer, so that the calling method can correctly recover frm this exception. Maybe a PostConditionException would be generic enough for many circumstances.
Throw that exception is not a good practice in some case and I wonder why when you already catch it with the if statement?
if(returnValue==null)
If you describe a method contract where the return value can not be null, then you had better make sure you don't return null. But this isn't a NullPointerException at all. If the value you have to return is null then clearly the caller has either given you bad arguments (IllegalArgumentException), you are not in a valid state (IllegalStateException), or some other much more meaningful exceptional condition has occurred other than NullPointerException (which usually indicates a programming error).
In the Java-verse null is always a valid value when expecting an object. You're better off avoiding such impossible post conditions. If you really can't abide a null then you'll have to rework your method so you can return a primitive.
It's often a really good idea to throw NPE before the logic gets so deep that the calling programmer will have a hard time figuring out what was null. addListener() methods are a good example.
I see no problem with throwing a NPE as early as possible before the JVM does it for you - in particular for null arguments. There seems to be some debate about this, but there are many examples in the Java SE libraries that does exactly this. I cannot see why NPE should be holy in the aspect that you are not able to throw it yourself.
However, I digress. This question is about something different. You are talking about a post-condition stating that the return value mustn't be null. Surely null in this case would mean you have a bug inside the very method?
How would you even document this? "This method throws a NullPointerException if the return value unexpectedly is null"? Without explaining how this could happen? No, I would use an assertion here. Exceptions should be used for errors that can conceivably happen - not to cover things that can happen if there's something wrong inside the method, because that does not help anybody.