Event Handling system in Java

Question

I have written a system for handling the dispatch of events between objects. (I need this functionality for a larger project I am working on. Objects should be able to dispatch events without knowing anything about the objects that will receive them. If this indicates that there is something seriously wrong with my larger design, please tell me.)

I currently have two classes: EventHandler and EventListener. (There's a third class in the package, ResolvableEventObject, but it just adds a resolve() method to the basic EventObject that is called once the event is finished with dispatch.)

The code is as follows. (imports were skipped)

(If you need more explanation, please ask.)

Note: this is the old code, kept for TwoThe's answer. See below for new code.

EventManager.class

public class EventManager //The manager holds all the EventListeners, 
                          //and dispatches events among them.
{
    //Singleton pattern, as multiple managers is nonsensical.
    private static EventManager INSTANCE = null;  
    private HashMap<Class<? extends EventObject>, ArrayList<EventListener>> listeners;
    private int lockCount = 0;
    private ArrayList<ListenerStub> delayedListeners;

    public static EventManager getInstance()
    {
        if (INSTANCE == null) {
            INSTANCE = new EventManager();
        }
        return INSTANCE;
    }

    private EventManager()
    {
        listeners = new HashMap();
        delayedListeners = new ArrayList();
    }

    /*Adds a listener if there's nothing using the set of listeners, or delays it
     * until the usage is finished if something is.
     * Package protected because nothing other than EventListener's registerListener() 
     * should call it. Eqivalent statements apply for removeListener() below.
     */
    void addListener(Class<? extends EventObject> c, EventListener m)
    {
        if (lockCount == 0) {
            realAddListener(c, m);
        } else {
            delayListenerOperation(c, m, true);
        }
    }

    void realAddListener(Class<? extends EventObject> c, EventListener m)
    {
        if (!listeners.containsKey(c)) {
            listeners.put(c, new ArrayList<EventListener>());
        }
        ArrayList<EventListener> list = listeners.get(c);
        list.add(m);
        System.out.println("added");
    }

    void removeListener(Class<? extends EventObject> c, EventListener m)
    {
        if (lockCount == 0) {
            realRemoveListener(c, m);
        } else {
            delayListenerOperation(c, m, false);
        }
    }

    private void realRemoveListener(Class<? extends EventObject> c, EventListener m)
    {
        ArrayList<EventListener> list = listeners.get(c);
        list.remove(m);
        System.out.println("removed");
    }

    private void delayListenerOperation(Class<? extends EventObject> c, EventListener m, boolean add)
    {
        delayedListeners.add(new ListenerStub(c, m, add));
        System.out.println("delayed");
    }

    public void dispatchEvent(EventObject e) 
    {
        ListenerLock l = new ListenerLock();
        //This odd construction seems to force garbage collection. 
        //Security measure to clean up old objects before dispatching events.
        System.gc();
        try {
            Thread.sleep(1);
        } catch (Exception ex) {
        }
        //end of odd construction
        dispatchEvent(e, e.getClass()); //Hiding recursive call from user
        l.release();
        if (e instanceof ResolvableEventObject) {
            ((ResolvableEventObject) e).resolve();
            //ResolvableEventObject is my class, it's just an EventObject with a
            //resolve() method.
        }
    }

    private void dispatchEvent(EventObject e, Class C)
    {
        ArrayList<EventListener> list = listeners.get(C);
        if (list != null) {
            for (int i = 0; i < list.size(); i++) {
                EventListener listener = list.get(i);
                listener.handleEvent(e);
            }
        }
        if (EventObject.class.isAssignableFrom(C.getSuperclass())) {
            dispatchEvent(e, C.getSuperclass());
        }
    }

    private void resolveDelayedListeners()
    {
        for (ListenerStub listenerStub : delayedListeners) {
            if (listenerStub.isAdd()) {
                realAddListener(listenerStub.getC(), listenerStub.getM());
            } else {
                realRemoveListener(listenerStub.getC(), listenerStub.getM());
            }
        }
    }

    private class ListenerStub /*
     * This class is used to hold the data of the listeners to be removed
     * until the lock on the listener set is released.
     */

    {

        private Class<? extends EventObject> c;
        private EventListener m;
        private boolean add;

        ListenerStub(Class<? extends EventObject> c, EventListener m, boolean add)
        {
            this.c = c;
            this.m = m;
            this.add = add;
        }

        public Class<? extends EventObject> getC()
        {
            return c;
        }

        public EventListener getM()
        {
            return m;
        }

        public boolean isAdd()
        {
            return add;
        }
    }

    public class ListenerLock
    /*
     * This is a basic lock that is used to prevent modifications to the list of
     * listeners. It releases when told, or when finalized if forgotten.
     */
    {

        boolean released = false;

        ListenerLock()
        {
            lockCount++;
        }

        public void release()
        {
            if (!released) {
                lockCount = Math.min(0, lockCount - 1);
                released = true;
                if (lockCount == 0) {
                    resolveDelayedListeners();
                }
            }
        }

        protected void finalize() throws Throwable
        {
            super.finalize();
            release();
        }
    }
}

EventListener.class

public class EventListener
{
/*
 * An EventListener pairs a class (EventObject or a subtype) with a method on an object.
 * It can hold either a strong or a weak reference to the object, and defaults weak.
 * It uses reflection to call the specified method.
 */
    Object source;
    Method handler = null;
    Class E;
    WeakReference<Object> sourceReference;

    public EventListener(Class<? extends EventObject> E, Object source,
                         String handlingMethod, boolean weakReference)
    {

        if (weakReference) {
            this.source = null;
            sourceReference = new WeakReference(source);
        } else {
            this.source = source;
            sourceReference = null;
        }
        /*
         * This structure sets up the call to the specified method on the specified
         * object. If no method is found for the specified class to listen to,
         * superclasses are tried until either a valid method is found or EventObject
         * is reached with no match.
         */
        Class C = E;
        while (EventObject.class.isAssignableFrom(C) && handler == null) {
            try {
                this.handler = source.getClass().getMethod(handlingMethod, C);
            } catch (NoSuchMethodException e) {
                C = C.getSuperclass();
            }
        }
        if (handler == null) {
            throw new IllegalArgumentException("No method with the signature: "
                                               + handlingMethod + "("
                                               + E.getSimpleName() + ") found.");
        }
        this.E = E;
    }

    public EventListener(Class<? extends EventObject> E, Object source,
                         String handlingMethod)
    {
        this(E, source, handlingMethod, true);
    }

    void handleEvent(EventObject event) //package protected because it should only be
                                        //called by EventManager's dispatchEvent().
    {
        if (source == null) { //source == null iff using weak references.
            if (sourceReference.get() == null) { //referenced object garbage collected,
                                                 //delete listener.
                deregisterListener();
                return;
            }
            try {
                handler.invoke(sourceReference.get(), event);
            } catch (IllegalAccessException | IllegalArgumentException |
                     InvocationTargetException e) {
                throw new IllegalArgumentException(e.getMessage(), e.getCause());
            }
        } else {
            try {
                handler.invoke(source, event);
            } catch (IllegalAccessException | IllegalArgumentException |
                     InvocationTargetException e) {
                throw new IllegalArgumentException(e.getMessage(), e.getCause());
            }
        }
    }

    public void registerListener() //registers a listener with the manager. 
                                   //Reference to listener is required to deregister later.
    {
        EventManager.getInstance().addListener(E, this);
    }

    public void deregisterListener()
    {
        EventManager.getInstance().removeListener(E, this);
    }
}

New code:

EventManager.java

public final class EventManager //The manager holds all the EventListeners, and dispatches events among them.
{

    private static EventManager INSTANCE = new EventManager();  //Singleton pattern, as multiple managers is nonsensical.
    private HashMap<Class<? extends EventObject>, LinkedList<EventListener>> listeners;
    private int lockCount = 0;
    private LinkedList<ListenerStub> delayedListeners;

    public static EventManager getInstance()
    {
        return INSTANCE;
    }

    private EventManager()
    {
        listeners = new HashMap();
        delayedListeners = new LinkedList();
    }

    /*Adds a listener if there's nothing using the set of listeners, or delays it
     * until the usage is finished if something is.
     * Package protected because nothing other than EventListener's registerListener() 
     * should call it. Eqivalent statements apply for removeListener() below.
     */
    void addListener(Class<? extends EventObject> clazz, EventListener listener)
    {
        if (lockCount == 0) {
            realAddListener(clazz, listener);
        } else {
            delayListenerOperation(clazz, listener, true);
        }
    }

    private void realAddListener(Class<? extends EventObject> clazz, EventListener listener)
    {
        if (!listeners.containsKey(clazz)) {
            listeners.put(clazz, new LinkedList<EventListener>());
        }
        LinkedList<EventListener> list = listeners.get(clazz);
        list.add(listener);
        System.out.println("added");
    }

    void removeListener(Class<? extends EventObject> clazz, EventListener listener)
    {
        if (lockCount == 0) {
            realRemoveListener(clazz, listener);
        } else {
            delayListenerOperation(clazz, listener, false);
        }
    }

    private void realRemoveListener(Class<? extends EventObject> clazz, EventListener listener)
    {
        LinkedList<EventListener> list = listeners.get(clazz);
        list.remove(listener);
        System.out.println("removed");
    }

    private void delayListenerOperation(Class<? extends EventObject> clazz, EventListener listener, boolean add)
    {
        delayedListeners.add(new ListenerStub(clazz, listener, add));
        System.out.println("delayed");
    }

    public void dispatchEvent(EventObject event) 
    {
        ListenerLock l = new ListenerLock();
        dispatchEvent(event, event.getClass()); //Hiding recursive call from user
        l.release();
        if (event instanceof ResolvableEventObject) {
            ((ResolvableEventObject) event).resolve();
            //ResolvableEventObject is my class, it's just an EventObject with a
            //resolve() method.
        }
    }

    private void dispatchEvent(EventObject event, Class clazz)
    {
        LinkedList<EventListener> list = listeners.get(clazz);
        if (list != null) {
            for (int i = 0; i < list.size(); i++) {
                EventListener listener = list.get(i);
                listener.handleEvent(event);
            }
        }
        if (EventObject.class.isAssignableFrom(clazz.getSuperclass())) {
            dispatchEvent(event, clazz.getSuperclass());
        }
    }

    private void resolveDelayedListeners()
    {
        for (ListenerStub listenerStub : delayedListeners) {
            if (listenerStub.isAdd()) {
                realAddListener(listenerStub.getListenerClass(), listenerStub.getListener());
            } else {
                realRemoveListener(listenerStub.getListenerClass(), listenerStub.getListener());
            }
        }
    }

    private class ListenerStub /*
     * This class is used to hold the data of the listeners to be removed
     * until the lock on the listener set is released.
     */

    {

        private Class<? extends EventObject> clazz;
        private EventListener listener;
        private boolean add;

        ListenerStub(Class<? extends EventObject> clazz, EventListener listener, boolean add)
        {
            this.clazz = clazz;
            this.listener = listener;
            this.add = add;
        }

        public Class<? extends EventObject> getListenerClass()
        {
            return clazz;
        }

        public EventListener getListener()
        {
            return listener;
        }

        public boolean isAdd()
        {
            return add;
        }


    }

    public class ListenerLock
    /*
     * This is a basic lock that is used to prevent modifications to the list of
     * listeners. It releases when told, or when finalized if forgotten.
     */
    {

        boolean released = false;

        ListenerLock()
        {
            lockCount++;
        }

        public void release()
        {
            if (!released) {
                lockCount = Math.min(0, lockCount - 1);
                released = true;
                if (lockCount == 0) {
                    resolveDelayedListeners();
                }
            }
        }

        protected void finalize() throws Throwable
        {
            super.finalize();
            release();
        }
    }
}

EventListener.java

public final class EventListener
{
/*
 * An EventListener pairs a class (EventObject or a subtype) with a method on an object.
 * It can hold either a strong or a weak reference to the object, and defaults weak.
 * It uses reflection to call the specified method.
 */
    Object source = null;
    Method handler = null;
    Class clazz;
    WeakReference<Object> sourceReference;

    public EventListener(Class<? extends EventObject> clazz, Object source,
                         String handlingMethod, boolean weakReference)
    {

        if (weakReference) {
            this.source = null;
            sourceReference = new WeakReference(source);
        } else {
            this.source = source;
            sourceReference = null;
        }
        /*
         * This structure sets up the call to the specified method on the specified
         * object. If no method is found for the specified class to listen to,
         * superclasses are tried until either a valid method is found or EventObject
         * is reached with no match.
         */
        Class C = clazz;
        while (EventObject.class.isAssignableFrom(C) && handler == null) {
            try {
                this.handler = source.getClass().getMethod(handlingMethod, C);
            } catch (NoSuchMethodException e) {
                C = C.getSuperclass();
            }
        }
        if (handler == null) {
            throw new IllegalArgumentException("No method with the signature: "
                                               + handlingMethod + "("
                                               + clazz.getSimpleName() + ") found.");
        }
        this.clazz = clazz;
    }

    public EventListener(Class<? extends EventObject> clazz, Object source,
                         String handlingMethod)
    {
        this(clazz, source, handlingMethod, true);
    }

    void handleEvent(EventObject event) //package protected because it should only be
                                        //called by EventManager's dispatchEvent().
    {
        if (source == null) { //source == null iff using weak references.
            if (sourceReference.get() == null) { //referenced object garbage collected, delete listener.
                deregisterListener();
                return;
            }
            try {
                handler.invoke(sourceReference.get(), event);
            } catch (IllegalAccessException | IllegalArgumentException |
                     InvocationTargetException e) {
                throw new IllegalArgumentException(e.getMessage(), e.getCause());
            }
        } else {
            try {
                handler.invoke(source, event);
            } catch (IllegalAccessException | IllegalArgumentException |
                     InvocationTargetException e) {
                throw new IllegalArgumentException(e.getMessage(), e.getCause());
            }
        }
    }

    public void registerListener() //registers a listener with the manager. Reference to listener
                                   //is required to deregister later.
    {
        EventManager.getInstance().addListener(clazz, this);
    }

    public void deregisterListener()
    {
        EventManager.getInstance().removeListener(clazz, this);
    }
}

Could you please tell me whether this is proper practice, or if there is a better way to do it? I am knowledgeable about Java, but this is the first time I have done anything like this.

Also, is there anything you notice that is hurting performance? I haven't had a chance to test it with regards to speed, but my functionality tests completed almost instantly after compile.

If you were wondering, this is intended to be used as part of a library for a larger project, as well as other subsequent projects with complex message passing between unspecified objects that don't reference each other.

Answer 1

In general an event listener is something used often in Java programs, that is why there are already a lot of event managers that you can use, for example the Observable pattern. So what you are basically doing is to reinvent the square wheel. It is a good idea to always look for the work of others first before doing unnecessary work.

private static EventManager INSTANCE = null;  

The proper singleton pattern to use is:

private static final EventManager INSTANCE = new EventManager();  

This reduces the getInstance() Method to:

public static EventManager getInstance() {
    return INSTANCE;
}

Some people discuss whether or not you even need a getInstance() method here, since you could just modify INSTANCE to be public instead vs "the Java way".

private ArrayList<ListenerStub> delayedListeners;

You seem to use ArrayList for all purposes. There are other List-types in Java that suit certain situations better than an ArrayList. You should have a look at those and try to get a feeling when to use which list. In this particular case i.E. a LinkedList is better.

private EventManager()

This should be protected, just in case you want to eventually inherit this class at any point in the future. private should only be used to prevent inheritance on purpose.

delayListenerOperation(c, m, true);

What is the purpose of delaying listeners? I don't really see one in your event system. If you want to make sure some events are processed later, try a PriorityQueue to sort Events based on their order. Working with "global" variables for these purposes is prone to threading issues or coding mistakes.

ListenerLock l = new ListenerLock();

This is a C++-pattern in Java, it won't work as expected and shouldn't be used. You should use a class-global Lock if you really need to lock something. But since this code isn't thread-safe at all, I don't see the reason for a lock.

System.gc();

Never! do that, unless you have a really good reason to and absolutely know and have proven that you must do it. The garbage collector is very smart in the way it works, and those constructs just dumb it down and cause hick-ups or a waste of processing time.

Thread.sleep(1);

Why? What do you try to archive with that? This serves no purpose.

ListenerStub(Class<? extends EventObject> c, EventListener m, boolean add)

c, m or add are not a good choice for variable names. Try to be expressive so you still know what this does should you have to review the code in 2 years.

public class ListenerLock

Reinventing the wheel once more. See Lock for the already existing (and actually working) implementation.

protected void finalize() throws Throwable

Again: never do that. This is prone to many errors as you can never tell when this function is actually called.

public class EventListener

This should be an Interface if you want other classes to be able to implement this. Otherwise every class needs to use this as a base class, and in Java you can only inherit from one class. That would as well simplify a lot of your handling code down to a simple

void handleEvent(EventObject e)

in the actual class that is supposed to handle the event.

if (weakReference) {
    this.source = null;
    sourceReference = new WeakReference(source);
} else {
    this.source = source;
    sourceReference = null;
}

If you use Reference as base class here and write your own StandardReference<T> class you can simplify this to:

this.source = weakReference ? new WeakReference(source) : new StandardReference(source);

This would be more elegant and remove the duplicated variable.

Summary

You wrote a lot of code to duplicate existing functionality but did neither improve nor add to the existing one. I would suggest to remove those classes and work with the existing functionality to solve your problem.

That would remove all bugs and potential bugs from the code you have, and will be future-proof should there be any major changes to Java.