Java Listener inheritance

Question

I have a java class which fires custom java events. The structure of the code is the following:

public class AEvent extends EventObject {
...
}

public interface AListener extends EventListener {

  public void event1(AEvent event);

}

public class A {

  public synchronized void addAListener(AListener l) {
  ..
  }

  public synchronized void removeAListener(AListener l) {
  ..
  }

  protected void fireAListenerEvent1(AEvent event) {
  ..
  }
}

Everything works correctly, but I'd like to create a new subclass of A (call it B), which may fire a new event. I'm thinking of the following modification:

public class BEvent extends AEvent {
...
}

public interface BListener extends AListener {

  public void event2(BEvent event);
}

public class B extends A {

  public synchronized void addBListener(BListener l) {
  ..
  }

  public synchronized void removeBListener(BListener l) {
  ..
  }

  protected void fireBListenerEvent2(AEvent event) {
  ..
  }

}

Is this the correct approach? I was searching the web for examples, but couldn't find any.

There are a few things I don't like in this solution:

1. BListener has two methods one uses AEvent the other uses BEvent as a parameter.
2. B class both has addAListener and addBListener methods. Should I hide addAListener with private keyword? [UPDATE: it's not possible to hide with private keyword]
3. Similar problem with fireAListenerEvent1 and fireBListenerEvent1 methods.

I'm using Java version 1.5.

Answer 1

I don't see a reason why BListener should extend AListener.

Do you really want to force everyone interested in B events to also implement event1()?

Also you can't add addAListener(), since a derived class can not reduce the visibility of a method that's present in the parent class. Also, you shouldn't need to, or you would violate the Liskov substitution principle (every B must be able to do everything an A can do).

And as a last remark, I'd make the fire*() methods protected. There's usually no reason at all to keep them public and reducing the number of public members keeps your public interface clean.

Answer 2

Don't use inheritence, it's not what you want and will lead to a brittle and difficult to change design. Composition is a more flexible and a better approach for the design. Always try to design interfaces as granular as possible because they should not be changed event. They are your contract with the rest of the system. If new functionality needs to be added the first option is to add more information to the event. If that's not appropriate, then you should design a new interface for delivering that event. This prevents having to change any existing code which isn't affected.

Here's my favorite pattern for this, I beleive it's commonly referred to as an Observer.

Make a new interface defining a methods for that event type (fooEvent() addFooEventListener() removeFooEventListener()). Implement this interface in the concrete class which generates these events. (I usually calls this something like SourcesFooEvent, FiresFooEvent, FooEventSource, etc)

If you want to reduce code duplication you can construct a helper class which handles registration of the listeners, stores them in a collection, and provides a fire method for publishing the events.

Generics can help here. First, a generic listener interface:

public interface Listener<T> {
  void event(T event);
}

Next, a matching EventSource interface:

public interface EventSource<T> {
    void addListener(Listener<T> listener);
}

Finally an abstract base class to quickly construct a helper class to handle registration of listeners and event dispatch:

public abstract class EventDispatcher<T> {
    private List<Listener<T>> listeners = new CopyOnWriteArrayList<T>();

    void addListener(Listener<T> listener) {
      listeners.add(listener);
    }    

    void removeListener(Listener<T> listener) {
      listeners.remove(listener);
    }

    void fireEvent(T event) {
      for (Listener<T> listener : listeners) {
        listener.event(event);
      } 
    }
}

You'd make use of the abstract EventDispatcher through encapsulation, allowing any other class to easily implement EventSource while not requiring it to extend any particular class.

public class Message {
}

public class InBox implements EventSource<Message> {

  private final EventDispatcher<Message> dispatcher = new EventDispatcher<Message>();

  public void addListener(Listener<Message> listener) {
    dispatcher.addListener(listener);
  }

  public void removeListener(Listener<Message> listener) {
    dispatcher.removeListener(listener);
  }

  public pollForMail() {
    // check for new messages here...
    // pretend we get a new message...

    dispatcher.fireEvent(newMessage);
  }
}

Hopefully this illustrates the nice balance between type safety (important), flexibility and code reuse.