Monday, December 14, 2009

Swing Event Handling - Possible Improvements

Though the concept is certainly fine, I've never been thrilled with Java's implementation of event firing and handling.  You define an event class, which is fine and useful, different events may carry different data, this makes perfect sense.  But, by convention at least, you need also define an interface for the EventListener, e.g.

public interface FooListener extends EventListener {
    public void someRandomHelpfulName(FooEvent e);
}

Now, this has some advantages, as we will see later, but it has definite drawbacks.  The first one is that in many shops the name of the method call is something like  "handleFooEvent", which is completely redundant.

This isn't all that big a deal, but Swing's EventListenerList, which many use to hold the listeners, is a lousy design. 
Consider the "standard" way to use is, which is an ugly, non-OO hack with the internals exposed all over the place.  Taken directly from the comments in the EventListenerList.java source code

Object[] listeners = listenerList.getListenerList();
     // Process the listeners last to first, notifying
     // those that are interested in this event
     for (int i = listeners.length-2; i>=0; i-=2) {
         if (listeners[i]==FooListener.class) {
             // Lazily create the event:
             if (fooEvent == null)
                 fooEvent = new FooEvent(this);
             ((FooListener)listeners[i+1]).fooXXX(fooEvent);
         }
     }

Later versions, 1.3, added a cleaner way to do this, via getListeners(Class t), but the true "OO" principal is tell, don't ask.  What EventListenerList really needs a method fireEvent(Event).  But it can't do this, because it doesn't know the name of the method it is calling.  This is the major drawback of writing a separate interface for every event.

Consider this alternative.  Instead of an interface for every listener, create one interface, generified by the type of the event.

public interface GenericListener extends EventListener {
   public void handleEvent(E event);
}

Then, instead of some strange array of pairs of things, we can create a Broadcaster class, using some of the new concurrency classes for thread safety.  What are the requirements for our collection of listeners?
  • The number of listeners is small
  • The number of "read" operations (firing events) outnumbers "write" (adding a listener)
  • You need  to prevent interference among threads during traversal.
This argues for a CopyOnWrite... collection.  Now, one common bug I've seen with EventListenerList is some listener getting added twice, or even more!  Since this is rarely a good thing, I favor CopyOnWriteArraySet, though it's easy to change in the code below.

It is simple then to write a Broadcaster that your class can use can fire a single type of event

public class Broadcaster<E extends EventObject> {
   
   protected Collection<GenericListener<E>> listeners = null;

   public synchronized void addListener(GenericListener<E> listener) {
      if (listeners == null)
         listeners = createCollection();

      listeners.add(listener);
   }

   public void removeListener(GenericListener<E> listener) {
      listeners.remove(listener);
   }

   public void fireEvent(E event) {
      if (listeners != null)
         for (GenericListener<E> listener : listeners)
            listener.handleEvent(event);
   }

   public boolean hasListeners() {
     return (listeners != null) && (listeners.size() > 0);
   }
   
   
   protected Collection<GenericListener<E>> createCollection() {
      return new CopyOnWriteArraySet<GenericListener<E>>();
   }

}

With this framework, it is very easy to fire and listen to a single class of events.  No unnecessary listener classes.  And you tell, don't ask the Broadcaster what to do.  The big limitation is that we can only readily fire and listen to one kind of event.  This limitation will be addressed in future posts.

No comments:

Post a Comment