32.

About This Bug Pattern

Although Java programmers quickly learn the rules governing which method will be called on an invocation, it's not hard to accidentally break one method by overloading another method that it relies on.

The Broken Dispatch pattern can be described as follows:

• The arguments to an overloaded method (such as foo()) are passed to another method (such as bar()) that takes more general types.

• bar() then invokes foo() with these arguments.

• Because the static types of these arguments inside the scope of bar() are more general, the wrong version of method foo() might be invoked.

Errors like these can be difficult to diagnose because they can be introduced simply by adding new methods (as opposed to modifying existing ones). Also, the program execution may continue for some time before problems are discovered.

The Symptoms

To illustrate the nature of this pattern, let's consider the following example code (originally Chapter 9:

Listing 14-1: Implementing the Immutable Lists

 interface List {} class Empty implements List {   public boolean equals(Object that) {     return (that != null) &&            (this.getClass() == that.getClass());   } } class Cons implements List {   Object first;   List rest;   Cons(Object _first) {     this.first = _first;     this.rest = new Empty();   }   Cons(Object _first, List _rest) {     this.first = _first;     this.rest = _rest;   }   public Object getFirst() { return this.first; }   public List getRest() { return this.rest; }   public boolean equals(Object that) {     // The semantics for equals specified in the Sun JDK 1.3 API require that     // we return false for x.equals(null), x non-null.     if (that == null) { return false; }     if (this.getClass() != that.getClass()) { return false; }     else { // that instanceof Cons       Cons _that = (Cons)that;       return (this.getFirst().equals(_that.getFirst())) &&              (this.getRest().equals(_that.getRest()));     }   } } 

Recall that we implemented linked lists as containers for these immutable lists. But now let's suppose that we're implementing linked lists in a separate package in which we know that all instances of class LinkedList will be lists of Strings. We could write the constructors to enforce this invariant as follows:

Listing 14-2: Defining Enforcement Parameters for Linked Lists

 public class LinkedList {   private List value;   /**    * Constructs an empty LinkedList.    */   public LinkedList() { this.value = new Empty(); }   /**    * Constructs a LinkedList containing only the given element.    */   public LinkedList(String _first) { this.value = new Cons(_first); }   /**    * Constructs a LinkedList consisting of the given Object followed by    * all the elements in the given LinkedList.    */   public LinkedList(String _first, LinkedList _rest) {     this.value = new Cons(_first, _rest.value);   }   public Object getFirst() { return this.value.getFirst(); }   public LinkedList getRest() {     return new LinkedList(this.value.getRest());   }   public void push(String s) { this.value = new Cons(s, this.value); }   public String pop() {     String result = (String)this.value.getFirst();     this.value = this.value.getRest();     return result;   }   public boolean isEmpty() { return this.value instanceof Empty; }   public String toString() { ... }   ... } 

Suppose we write this code and all the test cases work. Or, more realistically, let's suppose that the code doesn't work at first, but that we manage to get it working after a few debugging cycles.

Perhaps several months later you develop a new constructor on class Cons that takes a String representation of the list as its only argument (Appendix A contains some sample code that illustrates this). Such a constructor is quite useful—it allows us to construct new lists with expressions such as the following:

Listing 14-3: New Constructor Takes Only String Representation of List as Argument

 // equivalent to new Cons("this", new Cons("is", new Cons("a", new Cons("list", new Empty())))) new Cons("(this is a list)") // equivalent to new Cons("so", new Cons("is", new Cons("this", new Empty()))) new Cons("(so is this)") // equivalent to new Cons("this", //                  new Cons("list", //                    new Cons("contains", //                       new Cons("a", //                          new Cons(new Cons("nested", //                                new Empty()), //                             new Cons("list", //                                new Empty()))))))                 new Cons("(this list contains a (nested) list)") 

So, we write this constructor and all the test cases for it work. Great! But then we notice that some of the tests for methods on class LinkedList suddenly break. What's going on?

The Cause

The problem is with the constructor on class LinkedList that takes a single String as its argument. This constructor previously called the underlying constructor on class Cons. But now that we've overloaded that constructor with a more specific method, one that takes a String as an argument, this more specific method is invoked.

Unless the String passed to the LinkedList constructor is a valid representation of a Cons, the program will crash when trying to parse it. Worse yet, if that String does happen to be a valid representation of a Cons, the program will continue to execute with corrupt data. In that case, we would have introduced a saboteur into the data (see Chapter 12).

The Broken Dispatch bug, like other bug patterns, is easiest to diagnose in code that is test infested (to borrow from the terminology of extreme programming), in which all but the most trivial methods have corresponding unit tests. (For more on XP, see "Building Cost-Effective Specifications with Stories" in Chapter 3.)

In such an environment, the most common symptom will be a test case for code you haven't touched that suddenly breaks. When this happens, it's possibly a case of the Broken Dispatch pattern. If the test case breaks immediately after you overload another method, it almost certainly is.

If the code isn't loaded with tests, things become more difficult. The bug might produce symptoms such as method invocations that return much more quickly than expected (and with incorrect results). Alternatively, you might find that certain events that were supposed to occur never did (because the appropriate method was never executed).

Remember that symptoms like these can also be attributed to other bug patterns. If you encounter such symptoms, your best bet is to begin writing more unit tests, starting with the methods in which the error was discovered and working back through the program execution.

Cures and Preventions

The nice thing about this bug pattern is that there are some simple solutions. The most straightforward cure is to upcast the arguments in the method invocation. In our example, this would mean rewriting the relevant LinkedList constructor as follows:

Listing 14-4: Upcasting the Arguments in the Method Invocation

   public LinkedList(String _first) {     this.value = new Cons((Object)_first);   } 

Of course, this approach solves the problem only for this one call to the Cons constructor. There may be other calls where we have to upcast, and this technique might become cumbersome for clients of the Cons class. In cases like these, you have to weigh the advantage you gain from having this convenient constructor against the potential for errors it introduces.

This dilemma is yet another example of the tension that exists between the expressiveness and the robustness of an interface. One way to get the best of both worlds would be to replace the String constructor on Cons with a static method that takes a String and returns a new Cons object. In fact, this is a strictly better solution in examples like this one, where we want to perform sophisticated object initialization based on the constructor arguments. Including static methods that return new instances of a class is a design pattern known as the Factory Method pattern. See Resources for more information on design patterns.

Note 

Argument structure is a key component to making sure that when you overload a method, the call invokes the intended method.

A good preventative measure for this bug pattern is to avoid overloading methods entirely. Unlike method overriding, overloading is seldom needed or justified.

A Brisk Walk Through the Problem

This is a speedy summary of the steps we took surrounding the code in Listings 14-1 and 14-2 in this chapter. Starting with our example, we've implemented LinkedLists as containers for immutable lists.

We write a constructor to enforce the invariant that "all instances of class LinkedList will be lists of Strings."

Later, we write a new constructor that takes a String representation of the list as its only argument.

Test cases for the new constructor work, but some method tests on class LinkedList break.

Why? We've overloaded the constructor with a more specific method.

Results: If the String passed to the LinkedList constructor isn't a valid representation, it won't parse. If the String is coincidentally valid, the program will continue, but with corrupt data.