[LANG-1727] EventListenerSupport doesn't document ordering of events

- Javadoc
- Use longer lines
- Remove some vertical whitespace

Author: garydgregory

src/changes/changes.xml

@@ -94,6 +94,7 @@ The <action> type attribute can be add,update,fix,remove.
     <action issue="LANG-1772" type="fix" dev="ggregory" due-to="Gary Gregory">Reimplement org.apache.commons.lang3.ClassUtils.hierarchy(Class, Interfaces) using an AtomicReference.</action>
     <action                   type="fix" dev="ggregory" due-to="Ken Dombeck">Fix Javadoc code examples in DiffBuilder and ReflectionDiffBuilder #1400.</action>
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory">Fix generics in org.apache.commons.lang3.stream.Streams.toArray(Class) signature.</action>
+    <action issue="LANG-1727" type="fix" dev="ggregory" due-to="Elliotte Rusty Harold, Gary Gregory">EventListenerSupport doesn't document ordering of events.</action>
     <!-- ADD -->
     <action                   type="add" dev="ggregory" due-to="Gary Gregory">Add Strings and refactor StringUtils.</action>
     <action issue="LANG-1747" type="add" dev="ggregory" due-to="Oliver B. Fischer, Gary Gregory">Add StopWatch.run([Failable]Runnable) and get([Failable]Supplier).</action>

src/main/java/org/apache/commons/lang3/event/EventListenerSupport.java

@@ -37,32 +37,28 @@
 import org.apache.commons.lang3.function.FailableConsumer;
 
 /**
- * An EventListenerSupport object can be used to manage a list of event
- * listeners of a particular type. The class provides
- * {@link #addListener(Object)} and {@link #removeListener(Object)} methods
- * for registering listeners, as well as a {@link #fire()} method for firing
- * events to the listeners.
+ * Manages a list of event listeners of a given generic type. This class provides {@link #addListener(Object)} and {@link #removeListener(Object)} methods for
+ * managing listeners, as well as a {@link #fire()} method for firing events to the listeners.
  *
  * <p>
- * To use this class, suppose you want to support ActionEvents.  You would do:
+ * For example, to support ActionEvents:
  * </p>
+ *
  * <pre>{@code
- * public class MyActionEventSource
- * {
- *   private EventListenerSupport<ActionListener> actionListeners =
- *       EventListenerSupport.create(ActionListener.class);
+ * public class MyActionEventSource {
+ *
+ *     private EventListenerSupport<ActionListener> actionListeners = EventListenerSupport.create(ActionListener.class);
  *
- *   public void someMethodThatFiresAction()
- *   {
- *     ActionEvent e = new ActionEvent(this, ActionEvent.ACTION_PERFORMED, "somethingCool");
- *     actionListeners.fire().actionPerformed(e);
- *   }
+ *     public void someMethodThatFiresAction() {
+ *         ActionEvent e = new ActionEvent(this, ActionEvent.ACTION_PERFORMED, "something");
+ *         actionListeners.fire().actionPerformed(e);
+ *     }
  * }
  * }</pre>
- *
  * <p>
- * Serializing an {@link EventListenerSupport} instance will result in any
- * non-{@link Serializable} listeners being silently dropped.
+ * Events are fired
+ * <p>
+ * Serializing an {@link EventListenerSupport} instance will result in any non-{@link Serializable} listeners being silently dropped.
  * </p>
  *
  * @param <L> the type of event listener that is supported by this proxy.
@@ -71,7 +67,7 @@
 public class EventListenerSupport<L> implements Serializable {
 
     /**
-     * An invocation handler used to dispatch the event(s) to all the listeners.
+     * Invokes listeners through {@link #invoke(Object, Method, Object[])} in the order added to the underlying {@link List}.
      */
     protected class ProxyInvocationHandler implements InvocationHandler {
 
@@ -109,6 +105,9 @@ protected void handle(final Throwable t) throws IllegalAccessException, IllegalA
 
         /**
          * Propagates the method call to all registered listeners in place of the proxy listener object.
+         * <p>
+         * Calls listeners in the order added to the underlying {@link List}.
+         * </p>
          *
          * @param unusedProxy the proxy object representing a listener on which the invocation was called; not used
          * @param method the listener method that will be called on all of the listeners.
@@ -156,32 +155,30 @@ public static <T> EventListenerSupport<T> create(final Class<T> listenerInterfac
     }
 
     /**
-     * The list used to hold the registered listeners. This list is
-     * intentionally a thread-safe copy-on-write-array so that traversals over
-     * the list of listeners will be atomic.
+     * Hold the registered listeners. This list is intentionally a thread-safe copy-on-write-array so that traversals over the list of listeners will be atomic.
      */
     private List<L> listeners = new CopyOnWriteArrayList<>();
 
     /**
-     * The proxy representing the collection of listeners. Calls to this proxy
-     * object will be sent to all registered listeners.
+     * The proxy representing the collection of listeners. Calls to this proxy object will be sent to all registered listeners.
      */
     private transient L proxy;
-
     /**
      * Empty typed array for #getListeners().
      */
     private transient L[] prototypeArray;
 
     /**
      * Constructs a new EventListenerSupport instance.
-     * Serialization-friendly constructor.
+     * <p>
+     * This constructor is needed for serialization.
+     * </p>
      */
     private EventListenerSupport() {
     }
 
     /**
-     * Creates an EventListenerSupport object which supports the provided
+     * Constructs an EventListenerSupport object which supports the provided
      * listener interface.
      *
      * @param listenerInterface the type of listener interface that will receive
@@ -197,7 +194,7 @@ public EventListenerSupport(final Class<L> listenerInterface) {
     }
 
     /**
-     * Creates an EventListenerSupport object which supports the provided
+     * Constructs an EventListenerSupport object which supports the provided
      * listener interface using the specified class loader to create the JDK
      * dynamic proxy.
      *
@@ -212,29 +209,31 @@ public EventListenerSupport(final Class<L> listenerInterface, final ClassLoader
         this();
         Objects.requireNonNull(listenerInterface, "listenerInterface");
         Objects.requireNonNull(classLoader, "classLoader");
-        Validate.isTrue(listenerInterface.isInterface(), "Class %s is not an interface",
-                listenerInterface.getName());
+        Validate.isTrue(listenerInterface.isInterface(), "Class %s is not an interface", listenerInterface.getName());
         initializeTransientFields(listenerInterface, classLoader);
     }
 
     /**
-     * Registers an event listener.
+     * Adds an event listener.
+     * <p>
+     * Listeners are called in the order added.
+     * </p>
      *
      * @param listener the event listener (may not be {@code null}).
-     * @throws NullPointerException if {@code listener} is
-     *         {@code null}.
+     * @throws NullPointerException if {@code listener} is {@code null}.
      */
     public void addListener(final L listener) {
         addListener(listener, true);
     }
 
     /**
-     * Registers an event listener. Will not add a pre-existing listener
-     * object to the list if {@code allowDuplicate} is false.
+     * Adds an event listener. Will not add a pre-existing listener object to the list if {@code allowDuplicate} is false.
+     * <p>
+     * Listeners are called in the order added.
+     * </p>
      *
-     * @param listener the event listener (may not be {@code null}).
-     * @param allowDuplicate the flag for determining if duplicate listener
-     * objects are allowed to be registered.
+     * @param listener       the event listener (may not be {@code null}).
+     * @param allowDuplicate the flag for determining if duplicate listener objects are allowed to be registered.
      *
      * @throws NullPointerException if {@code listener} is {@code null}.
      * @since 3.5
@@ -247,7 +246,7 @@ public void addListener(final L listener, final boolean allowDuplicate) {
     }
 
     /**
-     * Creates the {@link InvocationHandler} responsible for broadcasting calls
+     * Creates the {@link InvocationHandler} responsible for calling
      * to the managed listeners. Subclasses can override to provide custom behavior.
      *
      * @return ProxyInvocationHandler
@@ -263,8 +262,7 @@ protected InvocationHandler createInvocationHandler() {
      * @param classLoader the class loader to be used
      */
     private void createProxy(final Class<L> listenerInterface, final ClassLoader classLoader) {
-        proxy = listenerInterface.cast(Proxy.newProxyInstance(classLoader,
-                new Class[] { listenerInterface }, createInvocationHandler()));
+        proxy = listenerInterface.cast(Proxy.newProxyInstance(classLoader, new Class[] { listenerInterface }, createInvocationHandler()));
     }
 
     /**
@@ -311,7 +309,7 @@ private void initializeTransientFields(final Class<L> listenerInterface, final C
     }
 
     /**
-     * Deserializes.
+     * Deserializes the next object into this instance.
      *
      * @param objectInputStream the input stream
      * @throws IOException if an IO error occurs
@@ -326,26 +324,25 @@ private void readObject(final ObjectInputStream objectInputStream) throws IOExce
     }
 
     /**
-     * Unregisters an event listener.
+     * Removes an event listener.
      *
      * @param listener the event listener (may not be {@code null}).
      * @throws NullPointerException if {@code listener} is
      *         {@code null}.
      */
     public void removeListener(final L listener) {
-        Objects.requireNonNull(listener, "listener");
-        listeners.remove(listener);
+        listeners.remove(Objects.requireNonNull(listener, "listener"));
     }
 
     /**
-     * Serializes.
+     * Serializes this instance onto the given ObjectOutputStream.
      *
      * @param objectOutputStream the output stream
      * @throws IOException if an IO error occurs
      */
     private void writeObject(final ObjectOutputStream objectOutputStream) throws IOException {
         final ArrayList<L> serializableListeners = new ArrayList<>();
-        // don't just rely on instanceof Serializable:
+        // Don't just rely on instanceof Serializable:
         ObjectOutputStream testObjectOutputStream = new ObjectOutputStream(new ByteArrayOutputStream());
         for (final L listener : listeners) {
             try {
@@ -356,10 +353,8 @@ private void writeObject(final ObjectOutputStream objectOutputStream) throws IOE
                 testObjectOutputStream = new ObjectOutputStream(new ByteArrayOutputStream());
             }
         }
-        /*
-         * we can reconstitute everything we need from an array of our listeners,
-         * which has the additional advantage of typically requiring less storage than a list:
-         */
+        // We can reconstitute everything we need from an array of our listeners,
+        // which has the additional advantage of typically requiring less storage than a list:
         objectOutputStream.writeObject(serializableListeners.toArray(prototypeArray));
     }
 }

src/test/java/org/apache/commons/lang3/event/EventListenerSupportTest.java

@@ -47,11 +47,13 @@
 import org.junit.jupiter.api.Test;
 
 /**
+ * Tests {@link EventListenerSupport}.
  */
 class EventListenerSupportTest extends AbstractLangTest {
 
     private void addDeregisterListener(final EventListenerSupport<VetoableChangeListener> listenerSupport) {
         listenerSupport.addListener(new VetoableChangeListener() {
+
             @Override
             public void vetoableChange(final PropertyChangeEvent e) {
                 listenerSupport.removeListener(this);
@@ -61,6 +63,7 @@ public void vetoableChange(final PropertyChangeEvent e) {
 
     private VetoableChangeListener createListener(final List<VetoableChangeListener> calledListeners) {
         return new VetoableChangeListener() {
+
             @Override
             public void vetoableChange(final PropertyChangeEvent e) {
                 calledListeners.add(this);
@@ -71,14 +74,12 @@ public void vetoableChange(final PropertyChangeEvent e) {
     @Test
     void testAddListenerNoDuplicates() {
         final EventListenerSupport<VetoableChangeListener> listenerSupport = EventListenerSupport.create(VetoableChangeListener.class);
-
         final VetoableChangeListener[] listeners = listenerSupport.getListeners();
         assertEquals(0, listeners.length);
         assertEquals(VetoableChangeListener.class, listeners.getClass().getComponentType());
         final VetoableChangeListener[] empty = listeners;
-        //for fun, show that the same empty instance is used
+        // for fun, show that the same empty instance is used
         assertSame(empty, listenerSupport.getListeners());
-
         final VetoableChangeListener listener1 = EasyMock.createNiceMock(VetoableChangeListener.class);
         listenerSupport.addListener(listener1);
         assertEquals(1, listenerSupport.getListeners().length);
@@ -108,7 +109,6 @@ void testCreateWithNullParameter() {
     void testEventDispatchOrder() throws PropertyVetoException {
         final EventListenerSupport<VetoableChangeListener> listenerSupport = EventListenerSupport.create(VetoableChangeListener.class);
         final List<VetoableChangeListener> calledListeners = new ArrayList<>();
-
         final VetoableChangeListener listener1 = createListener(calledListeners);
         final VetoableChangeListener listener2 = createListener(calledListeners);
         listenerSupport.addListener(listener1);
@@ -122,14 +122,12 @@ void testEventDispatchOrder() throws PropertyVetoException {
     @Test
     void testGetListeners() {
         final EventListenerSupport<VetoableChangeListener> listenerSupport = EventListenerSupport.create(VetoableChangeListener.class);
-
         final VetoableChangeListener[] listeners = listenerSupport.getListeners();
         assertEquals(0, listeners.length);
         assertEquals(VetoableChangeListener.class, listeners.getClass().getComponentType());
         final VetoableChangeListener[] empty = listeners;
-        //for fun, show that the same empty instance is used
+        // for fun, show that the same empty instance is used
         assertSame(empty, listenerSupport.getListeners());
-
         final VetoableChangeListener listener1 = EasyMock.createNiceMock(VetoableChangeListener.class);
         listenerSupport.addListener(listener1);
         assertEquals(1, listenerSupport.getListeners().length);
@@ -164,47 +162,42 @@ void testSerialization() throws IOException, ClassNotFoundException, PropertyVet
         final EventListenerSupport<VetoableChangeListener> listenerSupport = EventListenerSupport.create(VetoableChangeListener.class);
         listenerSupport.addListener(Function.identity()::apply);
         listenerSupport.addListener(EasyMock.createNiceMock(VetoableChangeListener.class));
-
-        //serialize:
+        // serialize:
         final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
         try (ObjectOutputStream objectOutputStream = new ObjectOutputStream(outputStream)) {
             objectOutputStream.writeObject(listenerSupport);
         }
-
-        //deserialize:
+        // deserialize:
         @SuppressWarnings("unchecked")
-        final
-        EventListenerSupport<VetoableChangeListener> deserializedListenerSupport = (EventListenerSupport<VetoableChangeListener>) new ObjectInputStream(
+        final EventListenerSupport<VetoableChangeListener> deserializedListenerSupport = (EventListenerSupport<VetoableChangeListener>) new ObjectInputStream(
                 new ByteArrayInputStream(outputStream.toByteArray())).readObject();
-
-        //make sure we get a listener array back, of the correct component type, and that it contains only the serializable mock
+        // make sure we get a listener array back, of the correct component type, and that it contains only the serializable mock
         final VetoableChangeListener[] listeners = deserializedListenerSupport.getListeners();
         assertEquals(VetoableChangeListener.class, listeners.getClass().getComponentType());
         assertEquals(1, listeners.length);
-
-        //now verify that the mock still receives events; we can infer that the proxy was correctly reconstituted
+        // now verify that the mock still receives events; we can infer that the proxy was correctly reconstituted
         final VetoableChangeListener listener = listeners[0];
         final PropertyChangeEvent evt = new PropertyChangeEvent(new Date(), "Day", 7, 9);
         listener.vetoableChange(evt);
         EasyMock.replay(listener);
         deserializedListenerSupport.fire().vetoableChange(evt);
         EasyMock.verify(listener);
-
-        //remove listener and verify we get an empty array of listeners
+        // remove listener and verify we get an empty array of listeners
         deserializedListenerSupport.removeListener(listener);
         assertEquals(0, deserializedListenerSupport.getListeners().length);
     }
 
     @Test
     void testSubclassInvocationHandling() throws PropertyVetoException {
-
         final EventListenerSupport<VetoableChangeListener> eventListenerSupport = new EventListenerSupport<VetoableChangeListener>(
                 VetoableChangeListener.class) {
+
             private static final long serialVersionUID = 1L;
 
             @Override
             protected java.lang.reflect.InvocationHandler createInvocationHandler() {
                 return new ProxyInvocationHandler() {
+
                     @Override
                     public Object invoke(final Object proxy, final Method method, final Object[] args)
                             throws IllegalAccessException, IllegalArgumentException, InvocationTargetException {
@@ -214,7 +207,6 @@ public Object invoke(final Object proxy, final Method method, final Object[] arg
                 };
             }
         };
-
         final VetoableChangeListener listener = EasyMock.createNiceMock(VetoableChangeListener.class);
         eventListenerSupport.addListener(listener);
         final Object source = new Date();