Passing null to a method [closed]

Question

I am in the middle of reading the excellent Clean Code

One discussion is regarding passing nulls into a method.

public class MetricsCalculator {
    public double xProjection(Point p1, Point p2) {
        return (p2.x - p1.x) * 1.5;
    }
}
...
calculator.xProjection(null, new Point(12,13));

It represents different ways of handling this:

public double xProjection(Point p1, Point p2) {
    if (p1 == null || p2 == null) {
        throw new IllegalArgumentException("Invalid argument for xProjection");
    }
    return (p2.x - p1.x) * 1.5;
}

public double xProjection(Point p1, Point p2) {
    assert p1 != null : "p1 should not be null";
    assert p2 != null : "p2 should not be null";
    return (p2.x - p1.x) * 1.5;
}

I prefer the assertions approach, but I don't like the fact that assertions are turned off by default.

The book finally states:

In most programming languages there is no good way to deal with a null that is passed by a caller accidentally. Because this is the case, the rational approach is to forbid passing null by default.

It doesn't really go into how you would enforce this restriction?

Do any of you have strong opinions either way.

Answer 1

General rule is if your method doesn't expect null arguments then you should throw System.ArgumentNullException. Throwing proper Exception not only protects you from resource corruption and other bad things but serves as a guide for users of your code saving time spent debugging your code.

Also read an article on Defensive programming

Answer 2

Both the use of assertions and the throwing of exceptions are valid approaches here. Either mechanism can be used to indicate a programming error, not a runtime error, as is the case here.

• Assertions have the advantage of performance as they are typically disabled on production systems.
• Exceptions have the advantage of safety, as the check is always performed.

The choice really depends on the development practices of the project. The project as a whole needs to decide on an assertion policy: if the choice is to enable assertions during all development, then I'd say to use assertions to check this kind of invalid parameter - in a production system, a NullPointerException thrown due to a programming error is unlikely to be able to be caught and handled in a meaningful way anyway and so will act just like an assertion.

Practically though, I know a lot of developers that don't trust that assertions will be enabled when appropriate and so opt for the safety of throwing a NullPointerException.

Of course if you can't enforce a policy for your code (if you're creating a library, for example, and so are dependent on how other developers run your code), you should opt for the safe approach of throwing NullPointerException for those methods that are part of the library's API.

Answer 3

Also not of immediate use, but related to the mention of Spec#... There's a proposal to add "null-safe types" to a future version of Java: "Enhanced null handling - Null-safe types".

Under the proposal, your method would become

public class MetricsCalculator {
    public double xProjection(#Point p1, #Point p2) {
        return (p2.x - p1.x) * 1.5;
    }
}

where #Point is the type of non-null references to objects of type Point.

Answer 4

Spec# looks very interesting!

When something like that isn't available, I generally test non-private methods with a run-time null-check, and assertions for internal methods. Rather than code the null check explicitly in each method, I delegate that to a utilities class with a check null method:

/**
 * Checks to see if an object is null, and if so 
 * generates an IllegalArgumentException with a fitting message.
 * 
 * @param o The object to check against null.
 * @param name The name of the object, used to format the exception message
 *
 * @throws IllegalArgumentException if o is null.
 */
public static void checkNull(Object o, String name) 
    throws IllegalArgumentException {
   if (null == o)
      throw new IllegalArgumentException(name + " must not be null");
}

public static void checkNull(Object o) throws IllegalArgumentException {
   checkNull(o, "object");
} 

// untested:
public static void checkNull(Object... os) throws IllegalArgumentException {
   for(Object o in os) checkNull(o);  
}

Then checking turns into:

public void someFun(String val1, String val2) throws IllegalArgumentException {
   ExceptionUtilities.checkNull(val1, "val1");
   ExceptionUtilities.checkNull(val2, "val2");

   /** alternatively:
   ExceptionUtilities.checkNull(val1, val2);
   **/

   /** ... **/
} 

That can be added with editor macros, or a code-processing script. Edit: The verbose check could be added this way as well, but I think it's significantly easier to automate the addition of a single line.