Java Observer Pattern – Preconditions for Adding/Removing a Listener

design-patternsjavaobserver-pattern

Imagine the following interface:

 interface Service {
   addListener(Listener l)
   removeListener(Listener l)
 }

Should I check for null values while add/remove? Is it a good idea on remove to check if the listener was registered before (e.g. listener B was never registered and should be removed)?

What behaviour helps developers here to identify problems without creating new ones like IllegalArgumentExceptions?

Best Answer

First and foremost, you should be consistent with the framework convention. Java is a big world to be sure. I use Java for Android, and here the convention is rather permissive.

For example, setOnClickListener allows you to pass in null - this is a crude but nonambiguous way of removing the listener.

From Android source code:

public class View implements Drawable.Callback, KeyEvent.Callback, AccessibilityEventSource {
// [snip]

/**
 * Register a callback to be invoked when this view is clicked. If this view is not
 * clickable, it becomes clickable.
 *
 * @param l The callback that will run
 */
  public void setOnClickListener(OnClickListener l) {
      if (!isClickable()) {
          setClickable(true);
      }
      getListenerInfo().mOnClickListener = l;
  }

Null is fine:

  /**
   * Return whether this view has an attached OnClickListener.  Returns
   * true if there is a listener, false if there is none.
   */
  public boolean hasOnClickListeners() {
      ListenerInfo li = mListenerInfo;
      return (li != null && li.mOnClickListener != null);
  }

Where multiple listeners are allowed, removing a listener is a tolerant operation, too:

/**
 * Remove a listener for layout changes.
 *
 * @param listener The listener for layout bounds change.
 */
public void removeOnLayoutChangeListener(OnLayoutChangeListener listener) {
    ListenerInfo li = mListenerInfo;
    if (li == null || li.mOnLayoutChangeListeners == null) {
        return;
    }
    li.mOnLayoutChangeListeners.remove(listener);
}

mOnLayoutChangeListeners is an ArrayList, and ArrayList.remove(Object object) doesn't crash on an inexistent item - it merely returns false.

The bottom line is the robustness principle - be conservative in what you do, be liberal in what you accept from others.

When in doubt, I'd go with liberal, unless you have a solid reason not to. It's simplier and less surprising. Consistency is the king, and the principle of astonishment is based on consistency.

Any preconditions should be clearly documented. Your definition of interface Service doesn't by itself promise to throw exceptions when null is passed in.

@NonNull and @Nullable annotations may help to remove ambiguity, and they are supported by good IDEs such as IntelliJ Idea or Android Studio.

Related Topic