Removing duplication on null checks

This article describes two ways to remove duplication when classes check that methods of an interface do not return null. I'm not sure which way is better, so let me know what you think!

Suppose we have an interface with two methods that return objects:

public interface Athlete
{
  Height getHeight();
  Weight getWeight();
}

Further suppose that Athlete instances are passed as parameters to many different classes in a system. The classes that accept Athlete instances conform to the fail-fast design philosophy, so they not only check if the instance is null, they also check if the athlete's height and weight are null:

public class AthleteUser
{
  public AthleteUser(final Athlete a)
  {
    if (a == null) throw new IllegalArgumentException();
    if (a.getHeight() == null) throw new IllegalArgumentException();
    if (a.getWeight() == null) throw new IllegalArgumentException();
    // Use athlete instance
  }
}

When more than one class performs this same validation, we have duplication. We don't want to repeat ourselves, so how do we remove this duplication?

One option is to introduce a helper class for dealing with Athelete instances. If we follow the naming conventions of Google Guava, we name the helper class Athletes:

public final class Athletes
{
  private Athletes(){}
  public static void assertNoNullReturnValues(final Athlete a)
  {
    if (a == null) throw new IllegalArgumentException();
    if (a.getHeight() == null) throw new IllegalArgumentException();
    if (a.getWeight() == null) throw new IllegalArgumentException();
  }
}

An alternative approach, one I have seldom seen in production, is to put the helper class inside the interface:

public interface Athlete
{
  Height getHeight();
  Weight getWeight();
  public final class Helper
  {
    private Helper(){}

    public void assertNoNullReturnValues(final Athlete a)
    {
      if (a == null) throw new IllegalArgumentException();
      if (a.getHeight() == null) throw new IllegalArgumentException();
      if (a.getWeight() == null) throw new IllegalArgumentException();
    }
  }
}

Which method do you prefer?

The separate class is used more frequently, but the class-inside-the-interface approach has its advantages:

• It makes it much easier to find the helper class.
• It reduces the number of files.

Comments

[Nick Gerakines]

I've been using the @Nonnull and @Nullable annotations lately and found them to be really useful for this sort of thing.