Programmers are Decision Makers

So yesterday I bumped into this bug: a listener of a tree object (not Swing's JTree. A tree with nodes and labeled edges, a-la Data Structures 101) was referencing a null object:

public class Auditor implements TreeListener {

  private Map<String,Date> map = new HashMap<String,Date>();

  public void nodeAdded(Node n) {
    map.put(n.id(), new Date());
  }

  public void nodeRemoved(Node n) {
    Date d = map.get(n.id());
    String s = d.toString();  // <-- NPE is here!!
    map.removeKey(n.id());

    // some other things ...
  }
}

My unit tests told me that the Auditor and Tree class are working fine - the tree fires the correct events and the Auditor reacts correctly. Of course, a Green bar does not guarantee "no bugs" so I double checked the code and it seemed fine.

Indeed the problem was elsewhere. Turns out that a bug in my app caused each listener to be registered twice. Auditor was not expecting to be called twice for each event, hence the NPE.

So I fixed the bug. Then I went back to the addListener() method of the Tree class. It is a very simple method:

public class Tree {

  private List<TreeListener> listeners
    = new ArrayList<TreeListener>();

  public void addListener(TreeListener arg) {
    listeners.add(arg);
  }

  // Additional methods ...
}

Looking at the code I tried to think if this method should be changed in order to prevent future occurrences of this bug. As I was thinking about it I realized that this simple method is an excellent illustration to one software's most fundamental truths:

Programming is about making (design) decisions.

I am not the first one to say this (see Joakim Holm's "Programming is all Design" or Alistair Cockburn). Here's the (probably partial) list of decision that a programmer has to make when writing a one-liner method that should simply add a listener to a list.

1. Should the listeners field be private? Maybe we want subclasses to manipulate/inspect it in some way, which calls for protected visibility.
   
   
2. What is the order of notification? Should we use a "first-registered first-notified" policy? or maybe "last-registered first-notified". Is order important at all? If not then maybe we would like to force the application not to depend on the order by random shuffling?
   
   
3. Can a listener be registered with more than one Tree object? If so, then the listener interface should probably specify the originating tree, not just the node
   
   
4. Can a listener be registered more than once with a single Tree object? Usually the answer is no, but there are scenarios where multiple registration does make sense.
   
   
5. Assuming each listener may be registered exactly once, how do we deal with multiple registrations?
   
   • Throw an exception?
   • A standard Java assertion? Could be disabled which may be good or bad depending on context.
   • Throw an AssertionError? Has a more dramatic effect than a plain Exception. Also will be reported by JUnit as a failure rather than an error. Again, may be either good or bad.
   • Silently ignore. Return from the method without adding the listener. This seems very natural but it has the drawback that it hides the problem. If I choose to silently ignore then I will get no notification about future problems. How about logging?
   • Return a Boolean value indicating whether the listener was already registered. Puts the responsibility on the client code to check this value. Will not always happen.
     
     
6. Do I need to provide some thread protection?
   
   • Define the method as synchronized?
   • Define the listeners field as a ConcurrentLinkedList?
   
   
7. How should this list of listeners interact with the Garbage Collector? Should a listener that is only accessible from this list be collectible (by storing it as a WeakReference)? A "No" answer puts additional burden on client code: must remove listeners from the Tree object when they are no longer needed. If listeners are dynamically added as the app is running this may become a big problem.
   
   Note that choosing weak references is not always a good idea. Some listeners are only accessible from the listened-to object. Take for example a listener that inspects the tree and updates the title of the main window. It is created, pushed onto the listeners list and all other traces to it are lost. A weak reference will make such a listener disappear with the very first collection cycle.
   
   
8. Do I want to refactor the whole "listeners" concern into a dedicated strategy class. This will improve testability and decoupling.

As I said this is a partial list. Point is simple: a programming task requires much more decision (and mental energy) that what initially seems. In other words: If you want to design everything upfront you'll end up programming upfront but without the assistance of tools such as IDE, Unit tests, Refactoring, and the likes. Good luck.