Insidious Coupling

This will get to the topic of code coupling soon enough, but bear with me for a bit of background.

I like to use the 'fail fast' approach to my code. This means that invalid inputs cause a failure in the code as quickly as possible. Perhaps the simplest example of this in the JDK is the collections classes, whose iterators fail as soon as they detect that the underlying collection object has been modified. This allows them to avoid synchronization costs while also detecting unsafe usage.

In my own code, the most common situation where I find fast-fail code is in constructors. If an argument the constructor cannot be null (either because it will be used in the constructor, saved to a field, or both), I check that the object reference is not null. For example:

public class Foo {
 // ...
 public Foo( Bar b ) {
  // Check that b is not null
  // rest of constructor...
 }
}

I started out performing this check with an if statement:

public class Foo {
 // ...
 public Foo( final Bar b ) {
  if ( b == null ) {
   throw new IllegalArgumentException( "Bar b cannot be null" );
  }
  // rest of constructor...
 }
}

This gets us a pretty understandable stack trace:

Exception in thread "main" java.lang.IllegalArgumentException: Bar b cannot be null
 at org.sandbox.Foo.(Foo.java:10)
 at org.sandbox.SimpleDoesStuff.doIt(SimpleDoesStuff.java:11)
 at org.sandbox.Baz.main(Baz.java:10)

But this Java code is pretty verbose, especially if you have multiple arguments to check. After that I tried creating a static method named assureNotNull which would perform the check:

public class Foo {
 // ...
 public Foo( final Bar b ) {
  assureNotNull( b );
  // rest of constructor...
 }
 //...
 private static void assureNotNull( final Object o ) throws IllegalArgumentException {
  if ( o == null ) {
   throw new IllegalArgumentException( "Object cannot be null" );
  }
 }
}

This has the unfortunate consequence that it becomes harder to track down the nature of the problem.

Exception in thread "main" java.lang.IllegalArgumentException: Object cannot be null
 at org.sandbox.Foo.assureNotNull(Foo.java:14)
 at org.sandbox.Foo.(Foo.java:9)
 at org.sandbox.SimpleDoesStuff.doIt(SimpleDoesStuff.java:11)
 at org.sandbox.Baz.main(Baz.java:10)

The stack trace on the exception shows assureNotNull as the source of the exception, which while strictly true doesn't clearly show that the real problem was with someone passing a null Bar to the Foo constructor. This can be mitigated to some degree by taking an description as argument:

public class Foo {
 // ...
 public Foo( final Bar b ) {
  assureNotNull( b, "Bar b cannot be null" );
  // rest of constructor...
 }
 //...
 private static void assureNotNull( final Object o, final String msg ) throws IllegalArgumentException {
  if ( o == null ) {
   throw new IllegalArgumentException( msg );
  }
 }
}

This provides a somewhat more helpful exception message but the method generating the exception is still 'off by one':

Exception in thread "main" java.lang.IllegalArgumentException: Bar b cannot be null
 at org.sandbox.Foo.assureNotNull(Foo.java:14)
 at org.sandbox.Foo.(Foo.java:9)
 at org.sandbox.SimpleDoesStuff.doIt(SimpleDoesStuff.java:11)
 at org.sandbox.Baz.main(Baz.java:10)

And even with this semi-fix to help find the cause of the problem, we're still left with recreating this method in every class. From here the logical next step is to centralize the method. We can do this with an Assertions class:

public class Assertions {
 // ...
 public  static void assureNotNull( final Object o, final String msg ) throws IllegalArgumentException {
  if ( o == null ) {
   throw new IllegalArgumentException( msg );
  }
 }
 // ...
}

Now, our Foo constructor becomes simpler:

import org.acme.Assertions;

public class Foo {
 // ...
 public Foo( final Bar b ) {
  Assertions.assureNotNull( b, "Bar b cannot be null" );
  // rest of constructor...
 }
 //...
}

And with Java 5's static imports, we can even simplify it further:

import static org.acme.Assertions.assureNotNull;

public class Foo {
 // ...
 public Foo( final Bar b ) {
  assureNotNull( b, "Bar b cannot be null" );
  // rest of constructor...
 }
 //...
}

But we still have the wrong method as the first listed in the stack trace:

Exception in thread "main" java.lang.IllegalArgumentException: Bar b cannot be null
 at org.acme.Assertions.assureNotNull(Assertions.java:9)
 at org.sandbox.Foo.(Foo.java:7)
 at org.sandbox.SimpleDoesStuff.doIt(SimpleDoesStuff.java:11)
 at org.sandbox.Baz.main(Baz.java:10)

Since all of us Java developers are big on frameworks, we might even take it to the 'next level'. Why not centralize the Assertions in some common framework? If the Assertions class were in a common framework, the thinking goes, then we'd all be able to use it everywhere. That consistency would really help us read unfamiliar code.

Ignoring the issue of getting any set of developers of size greater than one to agree on anything, especially a basic assertions framework used everywhere, we've just introduced insidious code coupling where it really doesn't belong. In our desire to achieve clarity, brevity, and avoid redundancy, we've coupled code together. If we have a common class in our framework, all the classes in the framework are now coupled to it and indirectly to each other. If we move the class to a generic framework, we externalize the coupling, making it even broader. This coupling introduces many hazards for versioning and incompatibility problems that come with the independent evolution of code.

So, what choice are we left with? Well, we could always depend upon the runtime itself to generate an exception for us. We can do this quite briefly, with less redundancy of code than the other approaches, and with zero coupling:

public class Foo {
 // ...
 public Foo(Bar b) {
  b.getClass(); // Assure not null
 }
}

This code is quite short, and generates a NullPointerException with the top element pointing to exactly the line of code where the problem occurred:

Exception in thread "main" java.lang.NullPointerException
 at org.sandbox.Foo.(Foo.java:7)
 at org.sandbox.SimpleDoesStuff.doIt(SimpleDoesStuff.java:11)
 at org.sandbox.Baz.main(Baz.java:10)

This code also avoids nearly all redundancy, and I think after seeing it once it becomes quite clear. As a bonus, it's easy to create a macro in any modern IDE to help in generating the code.

Sometimes, the right solution to a problem is not abstracting code into a separate method/class/framework.