Software development is all about decisions. Developers make many tiny decisions every day that add up to seriously impact the overall quality of a code base. In this example, I will discuss decisions made while attempting to solve a seemingly simple problem - How do we eliminate duplication in argument validation?
(The class used in this article is available on GitHub as part of the kblaney-assertions project, which I hope to eventually publish to the Maven Central Repository.)
The Problem
It is often recommended that public methods validate their arguments (for example, see javapractices.com). Consistently doing so leads to systems that have the admirable quality of being fail-fast. For example, suppose I have a method that accepts two Strings, neither of which are allowed to be null:
public void foo(final String s1, final String s2)
{
if (s1 == null)
{
throw new IllegalArgumentException("s1 is null");
}
if (s2 == null)
{
throw new IllegalArgumentException("s2 is null");
}
// Remainder of method elided for brevity.
}
Before we even get into the duplication problem, we’ve made some decisions. What exception should we throw if an argument is null? I prefer to throw an IllegalArgumentException when an argument is invalid in any way (including when it is null), but others prefer to throw a NullPointerException. I prefer to leave NullPointerExceptions to indicate specifically that a null pointer was dereferenced. (Perhaps this confusion could have been avoided if NullPointerException was named NullDereferenceException? Naming really is so important to maintainability!) We’ve also made a minor decision that we should have a blank line after parameter validation. Blank lines in software should mean something and I use them in cases like this to indicate a different “paragraph” of code. Code above the blank line is parameter validation; code below the blank line is the method body that assumes that parameters are valid.
There is clearly duplication here and as we all know, duplication is the root of all evil so as professional software developers, we should remove it. But how? Time for more decisions!
The easiest solution is to introduce a private method in the same class. However, it is highly likely that the same parameter validation exists in many classes, so a private method won’t do. Therefore, we need a new class. But what should it be called?
For now, the new class will only have methods related to null-checking, so should we call it NullCheck? What about ArgNullChecker? For this decision, I consider the Single Responsibility Principle. What is the one thing our new class is going to be really good at? Well, the class is going to assert certain conditions about method arguments. For now, the only methods are related to nullness, but there are many other criteria that could conceivably become methods in the same class:
- numbers that must be positive or negative
- numbers that must be greater than a minimum
- numbers that must be smaller than a maximum
- collections that must be non-empty
- strings that must be non-empty or non-blank
With this in mind (even though we don’t write these methods yet), let’s name the new class ArgAssert. This name uses a common shortform (‘arg’ for ‘argument’) and makes it clear what the class is responsible for - asserting on arguments. With that decision aside, what should we name the method that checks whether a specified argument is null?
For this decision, I consider how I would write a short description of the method (for example, the opening sentence in the method’s Javadoc). In this case, the description would be “Asserts that a specified argument is not null.”. With that in mind, let’s name the method assertNotNull. What arguments should this method have?
Well, all the method does is check whether an argument is null, so the naive approach is for the method to have only one parameter:
public final class ArgAssert
{
public static void assertNotNull(final String arg)
{
if (arg == null)
{
throw new IllegalArgumentException("Argument is null");
}
}
}
Note other decisions made here. What should we name the method’s argument? I considered stringToCheck, valueToCheck, value, and s before settling on arg. We want to remain consistent in our use of the “arg” shortform that we use in the class name. Should the method be static? I’m generally not a fan of static methods, because they can make in-memory unit testing a pain. (Yes, I know about the heavyweight mocking frameworks that allow one to mock static method calls. I’m not a fan of those either, but we don’t have time to get into that discussion here.) However, in this case we have a method that runs entirely in memory that nobody will ever need to mock. Therefore, it makes sense to make the method static.
Now we can remove the duplication and make our original method easier to read:
public void foo(final String s1, final String s2)
{
ArgAssert.assertNotNull(s1);
ArgAssert.assertNotNull(s2);
// Remainder of method elided for brevity.
}
Note what’s missing if the assertNotNull method only has one parameter. If either argument is null, the exception’s message does not indicate the name of the argument. With an exception message alone, one can’t determine whether s1 or s2 was null. This is a significant debugging hurdle if we don’t have a traceback that shows which lines of code were executed. Therefore, let’s allow calling classes to pass in a String that indicates the name of the argument being validated:
public final class ArgAssert
{
public static void assertNotNull(final String arg, final String argName)
{
if (arg == null)
{
throw new IllegalArgumentException(argName + " is null");
}
}
}
Now our method allows calling classes to validate that strings are not null. But wait! What about methods that accept other types of objects? Shouldn’t we be able to use the same method to validate those objects? Of course! Java generics to the rescue!
public final class ArgAssert
{
public static <T> void assertNotNull(final T arg, final String argName)
{
if (arg == null)
{
throw new IllegalArgumentException(argName + " is null");
}
}
}
Note that we made the method generic, not the class. That’s so that different invocations of the method can have T represent different types:
public void foo(final String s, final Bar bar)
{
ArgAssert.assertNotNull(s, "s");
ArgAssert.assertNotNull(bar, "bar");
// Remainder of method elided for brevity.
}
I guess we’re done now, right? Nope. Not yet. Our method can still be significantly improved. Consider a constructor that needs to validate its parameters before storing them in members:
public final class A
{
private final B b;
private final C c;
public A(final B b, final C c)
{
ArgAssert.assertNotNull(b, "b");
ArgAssert.assertNotNull(c, "c");
this.b = b;
this.c = c;
}
}
Wouldn’t it be convenient if this constructor could combine argument validation and storing the argument in a member? It can if the assertNotNull method returns the validated parameter:
public final class A
{
private final B b;
private final C c;
public A(final B b, final C c)
{
this.b = ArgAssert.assertNotNull(b, "b");
this.c = ArgAssert.assertNotNull(c, "c");
}
}
That looks pretty nice, so let’s make that change. Here is the final result, including Javadoc:
/**
* Provides static utility methods that make assertions about arguments.
* When assertions fail, an {@code IllegalArgumentException} is thrown.
*/
public final class ArgAssert
{
/**
* Asserts that a specified argument is not null. If the argument is null,
* this method throws an {@code IllegalArgumentException}. If the argument
* is not null, this method returns it.
*
* @param <T> the type of the argument to check
* @param arg the argument to check
* @param argName the name of the argument (that gets included in the
* {@code IllegalArgumentException} that is thrown if {@code arg} is null)
*
* @return the non-null argument
*/
public static <T> T assertNotNull(final T arg, final String argName)
{
if (arg == null)
{
throw new IllegalArgumentException(argName + " is null");
}
return arg;
}
}
With this class in place, we are able to write concise parameter validation early in our public methods.
Summary
To summarize, think of all the decisions we had to make just for this simple method:
- What should we name the class?
- What should we name the method?
- How many parameters should the method accept?
- What should be the type of the method’s parameters?
- What should we name the method parameters?
- Should the method return a value?
- What should the method’s return type be?
- Should the method be static?
- Should the method be generic?
- Should the class be generic?
- What type of exception should the method throw if it encounters a null object?
- What should the exception’s message be if the method encounters a null object?
- Where should I put blank lines to make the method easier to comprehend?
We didn’t discuss the following decisions, but they have to be made too:
- What package does the class belong in?
- What project does the class belong in? In other words, should the class be part of a separate artifact?
- What should the method validate its parameters?
Now, consider that me must make these decisions again for every new method. Further, we must be open to future decisions impacting the decisions we have already made.
When you write software, don’t just dump some code into an editor, get things working and then think you’re done. Question the decisions you made while writing the code and get an outside opinion on your significant decisions, even before a formal code review. Doing so will lead to more maintainable software. Future maintainers will thank you!
Sunday, February 3, 2013 at 5:50PM