Checking all function parameter in all the functions

Question

When I first moved from c to Java I thought that I've finished will all the annoying parameter checks in the begining of every function . (Blessed exceptions)

Lately I've realized that I'm slowly moving back to that practice again , and I'm starting to get really annoyed with all the

if (null == a || null == b || null == a.getValue() || ...) 
{
  return null;
}

For example , I have a utility class that analyses web pages and extract specific elements from them. Any call to dom object function with null elements usually results in an exception - So in almost any function I write in this class has countless null checks :

private URL extractUrl(Element element) throws Exception {
if (null == element) {
  return null;
} ... 

public List<Object> getConcreteElements(String xpath) throws Exception {
if (null == xpath) {
  return Collections.emptyList();
}...

public String getElementsAsXML(String xpath) throws Exception {
if (null == xpath) {
 return null;
}...

in the beginning of every function. Is this something I should get used too or is there some coding practice I'm unaware of that can simplify my life?

Answer 1

At the public boundary of your component, you're always going to have to do data validation.

For internal methods, asserting that the values passed are correct is better.

Returning null is something that it's better to avoid if you can. Either fail then and there to indicate that there is an error upstream of you (internal case) or return an empty or default object if there is such a thing available (public case).

Answer 2

Only check-null-return-null if that makes sense for your application. It may be that returning null simply causes problems for the client code, which doesn't expect a null back. It may be that there's no sensible return value in response to bad input, in which case use exceptions.

As to whether or not you should check the arguments, that depends on who's using that function. if it's a private function, it may be overkill to check. If it's a public API function, you absolutely should check.

Answer 3

Returning null means the caller must ALWAYS check the result from your method. Meaning MORE miserable null checks.

Consider using the NullObject pattern instead, where you return a valid object but having empty operations. I.e. an empty list if a collection is returned or similar.

Answer 4

Yes, they're absolutely annoying. A few small things I've found that at least minimize this:

• Clearly document in internal api's functions that can't return null, or under what conditions you can expect null. If you can trust your documentation, at least, this allows you to avoid null checks in certain places.

• Some idioms minimize null checks - e.g.

  if (string!=null && string.equals("A") --> if ("A".equals(string))

• eclipse (as well as findbugs, I think, and probably many other ide's) can be set to flag redundant null checks, or missing null checks. Not perfect, but it helps.

But overall, you're correct. If my code didn't have null checks and logging statements, there wouldn't be anything there at all;)

Answer 5

In my experience, Java apps are easier to maintain if the default 'contract' for any API is that null parameter values and null results are not allowed. If a method is designed to allow a null argument or returns a null, the meaning and circumstances should be clearly specified in the Javadoc.

If an null is passed where the API does not specifically allow this, it is a bug and the best thing is to fail-fast with an NPE. (IMO, there is no need to document the possibility of an NPE ... it can be assumed.) If null is not a meaningful (documented) argument value, the practice of testing for null and returning null only serves to spread bad data and make it harder to debug your code. If your method uses the supplied null argument and that causes an NPE, so be it. If does not use the argument but it saves the value for future use, then it is worth testing for null and explicitly throwing a NPE.

I also try to avoid the (usually false) optimizations of using null to represent empty lists or empty arrays, at least at the API level.

Now there are situations where it makes sense for an API design to use nulls; e.g. in APIs where an object parameter implements some kind of plugin behavior / policy, or the null is used when an optional method parameter is not supplied. But you need to think about it and document it.

Answer 6

If you check for illegal values, don't hesitate to throw IllegalArgumentExceptions to notify the caller that he made an error. You can hide the boundary/illegal value checking in private methods, that increases the readability of the code, like - just taking one of your examples:

private URL extractUrl(Element element) throws Exception {

  validateExactUrlParameter(element);

  // the real url extraction code

}

private static validateExactUrlParameter(Element element) 
                              throws IllegalArgumentException {

  if (null == element) {
    throw new IllegalArgumentException("Null value not allowed for exactUrl method");
  }

  // do some more checks here, if required

}

If you want to avoid try/catch blocks, then consider throwing unchecked exceptions (RuntimeException or custom subclasses).