How to ensure a method handling each event in an event system?

Question

I have several types of events, for example:

abstract class Event {
    static class KeyPress extends Event { ... }
    static class KeyRelease extends Event { ... }
    static class KeyHold extends Event { ... }
    // ...
}

And many listeners which respond to some of the above events by registering them in an event handler. It looks like this currently:

abstract class AbstractListener {
    Set<Class<? extends Event>> eventTypes;
    protected abstract boolean respond(Event event);
}

class DownKeyListener exteneds AbstractListener {
    DownKeyListener () {
        // just prepares to register to receive these events. doesn't matter how exactly.
        eventTypes.add(KeyPress.class);
        eventTypes.add(KeyHold.class);
        //no KeyRelease e.g.
    }

    boolean respond(Event event) {
        if (event instanceof KeyPress)
            return handleKeyPress(event);
        else if (event instanceof KeyHold)
            return handleKeyHold(event);
        return false;
    }

    private boolean handleKeyPress(KeyPress e) { ... }
    private boolean handleKeyHold(KeyHold e) { ... }
}

What I don't like about this is that there is nothing forcing or checking at least the relation between the registered events and the checks for them and handling in the respond method. This keeps leading to developer bugs. it's also a lot of code with instanceofs for little benefit (I would say).

So I though about doing something "smart" like this: create a map between the event types and the handlers so each event that is registered for handling will have a handler:

abstract class AbstractListener {
    Map<Class<? extends Event>, Function<? extends Event, Boolean> map = new ...
    //             ^ doesn't ensure these event ^: are the same but at least
    //                                             that someone responds
    // to ensure same event i can do
    protected <T extends Event> void register(Class<T> event, Function<T, Boolean> function) {
        map.put(event, function);
    }

    protected abstract boolean respond(Event event);
}

And then:

class DownKeyListener exteneds AbstractListener {
    DownKeyListener () {
        Map.put(KeyPress.class, keyPressFunction);
        Map.put(KeyHold.class, keyHoldFunction);
    }

    boolean respond(Event event) {
        Function<? extends Event, Boolean> f = map.get(event.getClass());
        return f == null ? false : f.apply(event);
    }

    Function<KeyPress, Boolean> keyPressFunction = event -> ...;
    Function<KeyHold, Boolean> keyHoldFunction = event -> ...;
}

Of course this doesn't work because of generics. apply gives an error and I understand why

The method apply(capture#4-of ? extends Event) in the type Function<capture#4-of ? extends Event,Boolean> is not applicable for the arguments (capture#5-of ? extends Event)

I don't know how to get what I want working correctly. Some things I had in mind:

• Cast the result of map.get to something that will ensure it works properly
• Change the respond method to be generic protected abstract <T extends Event> boolean respond(T event); which give the similar error:

The method apply(capture#3-of ? extends Event) in the type Function<capture#3-of ? extends Event,Boolean> is not applicable for the arguments (T)

Anyone has a suggestion on achieving what I want in any way?

Answer 1

You could use a visitor pattern, like so:

abstract class Event {
    boolean accept(EventVisitor visitor);

    KeyPress extends Event {
        boolean accept(EventVisitor visitor)
        {
            return visitor.process(this);
        }
    }
    KeyRelease extends Event { ... }
    KeyHold extends Event { ... }
    // ...
}

abstract class EventVisitor
{
    boolean process(KeyPress event)
    {
        return false;
    }

    boolean process(KeyRelease event)
    {
        return false;
    }

    // One default process() method for each subclass of Event.

    boolean respond(Event event)
    {
        return event.accept(this);
    }
}

and then

class DownKeyListener extends EventVisitor
{
    void process(KeyPress event)
    {
        return handleKeyPress(event);
    }

    void process(KeyHold event)
    {
        return handleKeyHold(event);
    }
}

This solution ensures that

1. You avoid all the instanceof needed to dispatch on the event's type.
2. All events that are not explicitly handled by a visitor implementation have a default handling in the abstract class EventVisitor.

Answer 2

Another options is to stop "fighting" the limited type system. My understanding is you want to ensure that anything you want to listen to has a handler, and that the right handler gets called. Then those two steps should be combined. You haven't shown what exactly you are listening to, but I am guessing you have something observable.

observable.register<KeyPress>(new EventListener() {
    void handleEvent(Event e) {
        assert e instanceof KeyPress;
        KeyPress kpe = (KeyPress)e;
        //Do stuff with kpe
    }
});

This does involve a cast, but you can use assert to check during development, and the cast is right next to the registration. Of course you do not have to use an anonymous class:

observable.register<KeyPress>(new MyKeyPressHandler());

You can modify the return type to boolean if you need to.

---

An example of how it might work:

// Could also be abstract class
interface Event {
}

interface EventListener {
    void handleEvent(Event e);
}

interface Observable {
    public void register(Class<? extends Event> event, EventListener handler);
}

// An example of something observable - here how a keyboard might give events.
class KeyDownEvent {
    private Key key;
    KeyDownEvent(Key k) {
        key = k;
    }
    public Key getKey() {
        return key;
    }
}

class Keyboard implements Observable {
    private Map<Class<? extends Event>, Set<EventListener>> handlers = new HashMap<>();

    public void register(Class<? extends Event> event, EventListener handler) {
        // not handling if the set has not been defined yet, but it should be, (or in constructor).
        handlers.put(event, handler);
    }

    private notifyListeners(Event event) {
        Class eventClass = event.getClass();
        for (EventListener listener : handlers.get(eventClass)) {
            listener.handleEvent(event);
        }
    }

    private void updateKeyboardState(KeyboardState ks) {
        //Suppose this is called by some code to update the keyboard
        // get state
        for (Key k : keys) {
            if (ks.isPressed(k) && lastState.get(k) == KeyState.UP) {
                notifyListeners(new KeyDownEvent(k));
            }
        }
    }
}

class LogKeyPressesToConsole {
    LogKeyPressesToConsole(Keyboard keyboard) {
        keyboard.register(KeyDownEvent, new EventListener() {
            void handleEvent(Event event) {
                assert event instanceof KeyDownEvent;
                KeyDownEvent kde = (KeyDownEvent)event;
                System.out.printf(kde.getKey().toString());
            }
        });
    }
}

Answer 3

You might consider making AbstractListener be parameterized by the concrete type of Event that you want to handle.

abstract class AbstractListener<T extends Event> {
    ...
    protected abstract boolean respond(T event);
}

Then have:

class KeyPressListener extends AbstractListener<KeyPressEvent> {
    ....
}

You'd need separate concrete handlers, but you could have them both dispatch to something that could handle processing from different types of events.

Answer 4

I figured it out using my generics approach. My second point about changing respond to a generic <T extends Event> boolean respond(T event) was indeed the right approach. The compiler error for incompatible types can be solved with a dirty but safe cast. Here is the full working code

public class Test {

    public static void main(String[] args) {
        DownKeyListener dkl = new DownKeyListener();
        System.out.println(dkl.respond(new KeyPress()));
        System.out.println(dkl.respond(new KeyHold()));
        System.out.println(dkl.respond(new KeyRelease()));
    }
}

abstract class Event {
    static class KeyPress extends Event {}
    static class KeyHold extends Event {}
    static class KeyRelease extends Event {}
}

abstract class AbstractListener {

    private Map<Class<? extends Event>, Function<? extends Event, Boolean>> map = new HashMap<>();

    protected <T extends Event> void register(Class<T> event, Function<T, Boolean> function) {
        map.put(event, function);
    }

    <T extends Event> boolean respond(T event) {
        Function<? extends Event, Boolean> f = map.get(event.getClass());
        return f == null ? false : ((Function<T, Boolean>) f).apply(event);
    }
}

class DownKeyListener extends AbstractListener {

    DownKeyListener() {
        register(KeyPress.class, keyPressHandler);
        register(KeyHold.class, keyHoldHandler);
    }

    Function<KeyPress, Boolean> keyPressHandler = e -> true;
    Function<KeyHold, Boolean> keyHoldHandler = e -> true;
}

The output of running this is

true
true
false

as required.

There are 3 things i want to point out about this approach.

The first is the ease of use and compile time safety. It couples the event registering (the list of event to listen to which in this cases is the map's keys) and the event handling. For every event type which is registered a handler for that event type must be given and when the listener needs to respond to an event that same handler is used. Creating a new event type does not entail any additional work like visitor methods or new interfaces. It is created and if some listener wants to use it it's ready.

The second thing is that the respond method can be moved to the abstract superclass because it has the same behavior for all listeners: get the handler for the event, call it and return the result. If a subclass listener wants to change this for a specific event type they can override it, check the event type and for the rest call super.

The third thing is that cast and the loss of information between putting the event-function in the map and getting the function from the event. The map is declared with 2 wildcards with no way to force a relation between them as far as i know. this is because of the lack of generic fields in java. if such a thing ever came to exist it would simplify this approach. The forcing of the same types for the event and its handler is done in the register method which adds to the map but that enforcement is lost in the field declaration. That means that when getting the value from the key there is no more compile time correspondence and a cast has to be made to the same type. We know this cast is safe if we only used the register method to populate the map otherwise we're doomed. a generic field would eliminate the need for a "safe" cast.

That's it. I learned a lot from the different answers and my fighting with Java generics and I post here verbosely for the benefit of everyone else. Hopefully this helps someone else in the future.