Avoiding Defensive Checks

Defensive Checks

An API's contract (specifically its pre-conditions) defines things that must be true to call the API. A robust API verifies these pre-conditions through a layer of checks that reject invalid input and prevent further processing of the request.

Defensive checks are an absolute must on the public boundaries of your system and are an essential part of building robust, secure software - consider them a firewall against abuse; accidental and deliberate. Here's an example from the core Java API:

public class ArrayList {

    /**
     * Inserts the specified element at the specified position in this
     * list. Shifts the element currently at that position (if any) and
     * any subsequent elements to the right (adds one to their indices).
     *
     * @param index index at which the specified element is to be inserted
     * @param element element to be inserted
     * @throws IndexOutOfBoundsException {@inheritDoc}
     */
    public void add(int index, E element) {
        rangeCheckForAdd(index);

        ensureCapacityInternal(size + 1);  // Increments modCount!!
        System.arraycopy(elementData, index, elementData, index + 1, size - index);
        elementData[index] = element;
        size++;
    }

    private void rangeCheckForAdd(int index) {
        if (index > size || index < 0)
            throw new IndexOutOfBoundsException(outOfBoundsMsg(index));
    }

The important call here is rangeCheckForAdd(index) - that's the defensive check. Should the user get it wrong and supply an invalid index then an IndexOutOfBoundsException is thrown.

Getting it wrong means there is a defect in the calling code, and under normal circumstances, of course, the exception should never fire, not if the calling code is doing its job properly. Calling code shouldn’t handle or use these kind of exceptions as a way of managing flow of control. Instead, all effort should be put into preventing the invalid call in the first place.

This is a simple principle that can applied at all levels of your software, both when enforcing contracts through defensive checks or assertions, as we will see.

Defensive Checks as Overhead

Below the public API things become a little more complicated. Clearly we need to defend at the public boundaries because external clients cannot necessarily be trusted to do the right thing. But what if we own and control both sides, i.e. the calling code (client) and the callee (supplier)? Do we really need to create defences in what are essentially private implementation classes, below the public interface?

Obviously, when you control both client and supplier in any relationship then it's within your power to ensure that all supplier-defined entry conditions are met. So, in the same way you wouldn't place defensive checks in your private methods, there is absolutely no need to use them on internal classes in this situation. In effect, they are redundant and really should be viewed as overhead (cost), put in place to capture defects in your calling code - defects that should be discovered through rigorous automated testing, code reviews, etc.

Assertions

One long standing way of checking a method's pre-conditions is to use assertions. Like a defensive check, an assertion checks that a condition is true and throws an exception if it isn't (e.g. an AssertionFailure in Java). But unlike defensive checks, assertions can be (and normally are) disabled at runtime.

Assertions can be used to approximate Design by Contract but in practice they are better suited for documenting intent rather than being used as a defensive firewall. If anything tools like Javadoc actively encourage a more defensive style of programming because of how pre-conditions are often documented against the thrown exception via the @throws tag. This is great on public classes and interfaces (like Java's List interface below) but less so on implementation classes:

public interface List<E> extends Collection<E> {

    /**
     * Appends the specified element to the end of this list (optional
     * operation).
     *
     * <p>Lists that support this operation may place limitations on what
     * elements may be added to this list.  In particular, some
     * lists will refuse to add null elements, and others will impose
     * restrictions on the type of elements that may be added.  List
     * classes should clearly specify in their documentation any restrictions
     * on what elements may be added.
     *
     * @param e element to be appended to this list
     * @return <tt>true</tt> (as specified by {@link Collection#add})
     * @throws UnsupportedOperationException if the <tt>add</tt> operation
     *         is not supported by this list
     * @throws ClassCastException if the class of the specified element
     *         prevents it from being added to this list
     * @throws NullPointerException if the specified element is null and this
     *         list does not permit null elements
     * @throws IllegalArgumentException if some property of this element
     *         prevents it from being added to this list
     */
    boolean add(E e);

When using assertions to validate pre-conditions, method contracts tend to specify what must be true about the supplied parameters and the state of the system, but not what exceptions will be thrown should the conditions fail. Assertions are not part of the defined behaviour of the method.

You should therefore never test assertions. They will typically be disabled anyway and since stepping outside a method's contract indicates a defect in the client code, that is where you should be focusing your energy, i.e. on testing and ensuring that client code meets supplier pre-conditions. Remember you control both the calling and the called code, so all things are possible. Conversely, defensive checks are very much part of a method's documented behaviour and as such should be fully tested and verified.

One thing you should never do is use assertions in place of defensive checks on your public boundaries. If you are going to use them, use them on your internal classes only.

Verifying Integrity through Tests

One of the perceived dangers of not using defensive checks or assertions is that when a client fails to meet the pre-conditions of an underlying supplier class, the defect can be very hard to trace. Indeed, depending on the nature of the bug, the issue may manifest itself far away from the original sin, which begs the question, why not fail fast through a well positioned defensive check?

A better approach is simply to test your code thoroughly. A unit test not only tests a class's defined behaviour but it also provides documentation for would-be users of that class. It should be entirely possible to look at a class's associated tests and gain a strong understanding of how the class should be used.

If you are testing a class that collaborates purely with 'concrete' suppliers then you will only need to verify that it is behaving correctly according to its contracted behaviour - if it is behaving correctly then it follows that all underlying pre-conditions will have been met. Arguably, there may be pathological cases where this is not actually the case, but as a general rule of thumb, it is a reasonable safe assumption. If on the other hand a class interacts with an interface, then you will need to confirm that the interface's contract is being met. The best way to do this is using a dynamic mocking library like Mockito.

Asserting Invariants

It is unlikely you will see pre-conditions being asserted in modern Java/C# code, but you should hopefully have copious amounts of automated tests to compensate. Still assertions are extremely useful, particularly when it comes to asserting invariants.

As a basic example, consider a simple traffic light enumeration:

enum Color {
    RED,
    AMBER,
    GREEN
}

And a naive and not very object-oriented use of the enum:

public static String flowOf(Color color) {
    if (color == Color.RED) {
        return "stop";
    }
    else if (color == Color.AMBER) {
        return "stopping";
    }
    else if (color == Color.GREEN) {
        return "go";
    }
    return null;  // unreachable!

Assuming that the color parameter cannot be null (and why would it ever be!?), then the last return statement is basically redundant - it should never be reached. Nonetheless, this one line will probably trigger a whole series of questions to the first-time reader - can null be passed as a parameter, what happens if null is returned, etc.

A better way of phrasing this is to assert that once red and amber have been handled, the only other possible option is green and to assert this. This assertion is important in that it documents an important intent and invariant. It also verifies a pre-condition of method. And even though the assertion will most likely be disabled, documenting this intent in the code is hugely beneficial to future readers of the code.

public static String flowOf(Color color) {
    if (color == Color.RED) {
        return "stop";
    }
    else if (color == Color.AMBER) {
        return "stopping";
    }
    assert (color == Color.GREEN);
    return "go";
}

Conclusion

Defensive checks play a vital role in building robust software, especially at the public boundaries of the software. Anytime you sense the client of a piece of software cannot be trusted then the mantra is simple: defend, defend, defend. This applies to software at all levels; libraries, frameworks, and entire systems.

However, below the veneer of the API, you should really try to avoid duplicating the same defensive checks all the way down through the layers of your software. Think in terms of client / supplier contractual relationships and enforce these contracts though a mix of rigorous testing and judicious use of assertions, especially for invariants.