5

There are many different events, all implementing the same interface:

interface Event {}
class FooEvent implements Event {}
class BarEvent implements Event {}

Every event has a dedicated handler:

interface EventHandler<T extends Event> {
    void handle(T event);
}
class FooEventHandler implements EventHandler<FooEvent> {
    @Override
    public void handle(FooEvent event) { }
}
class BarEventHandler implements EventHandler<BarEvent> {
    @Override
    public void handle(BarEvent event) { }
}

All event handlers are created once and added to a map. Whenever an event occurs, this map should be used to find the proper event hander.

class Main {
    Map<Class<? extends Event>, EventHandler<? extends Event>> eventHandlerRegistry = Map.of(
            FooEvent.class, new FooEventHandler(),
            BarEvent.class, new BarEventHandler()
    );

    void handleEvent(Event event) {
        EventHandler<? extends Event> handler = this.eventHandlerRegistry.get(event.getClass());
        handler.handle(event); // DOES NOT COMPILE: needed=capture<? extends Event>, given=Event
    }
}

Unfortunately this last line does not compile. I can make it compile by leaving out the type parameter of EventHandler like this:

EventHandlerhandler = this.eventHandlerRegistry.get(event.getClass());
handler.handle(event); // WARNING: unchecked call to 'handle(T)' as a member of raw type 'EventHandler'

But this does not quite feel right... I am aware of PECS, but I feel kind of trapped because I produce AND consume my EventHandlers.

How can I implement this cleanly?

Patric
  • 1,489
  • 13
  • 28
  • Is this all the uses of your event handlers? If so, why is `EventHandler` generic? In other words, what's the value added by `handle(FooEvent event)` that is not present in `handle(Event event)`? – ernest_k Dec 13 '19 at 12:29
  • Every event can have different properties. E.g. `FooEvent` can have a property `String myString` whereas `BarEvent` could have a property `List myBars`. The concrete handlers know exactly what type of event they get and they need the properties of that event to handle it. Obviously I could omit the generic parameter of `EventHandler`, but then I would need to cast the `event` in every `handle()` method, which also does not feel clean... – Patric Dec 13 '19 at 12:41
  • Is `Event` a singleton? Can you have many events of the same type? If not, you can try using `Map> eventHandlerRegistry` to map events and handlers. Otherwise, I'd suggest providing constants (or enums) for each type of event and use them in the map, instead of `Class extends Event>` – Pavel Smirnov Dec 13 '19 at 12:54
  • Can you please share the code that calls `void handleEvent(Event event)` (and can this method be made generic?) – ernest_k Dec 13 '19 at 12:59
  • No, `Event` is not a singleton, there can be an arbitrary amount of events of the same type (e.g. for a sandwich store there can be 100s of `OrderSandwichEvent`). But I cannot use `Event` as the map key, because not all `OrderSandwichEvent`s are equal (e.g. one has `boolean ham=true` and another one can have `boolean ham=false`). And for using constants, I don't see how that wold solve my problem? Using `Class extends Event>` as map key is not a problem as far as I see, but the generic map value is... – Patric Dec 13 '19 at 13:00
  • @ernest_k You can assume it is called exactly as I illustrated in the example. The method could be made generic, but then I just receive a `T extends Event` as a method parameter, which I would need to cast to a concrete Event first before being able to access the events properties... Which is what I wanted to avoid in the first place. – Patric Dec 13 '19 at 13:05

2 Answers2

4

You can't have type safety if you're going to mix your (generic) handlers in the same map. As far as I can see, the way to make your code type-safe is to get rid of the generic type parameter on EventHandler; but this is one thing you want to avoid.

If you may sacrifice type safety, knowing that your handlers will always match the specified classes, then you can try something like:

private <T extends Event> EventHandler<T> getHandler(Class<?> eventClass) {
    return (EventHandler<T>) 
              this.eventHandlerRegistry.get(eventClass); //Unchecked cast
}

Then make your handleEvent method generic:

<T extends Event> void handleEvent(T event) {
    EventHandler<T> handler = this.getHandler(event.getClass());
    handler.handle(event);
}

This method would then compile successfully, without warning. The only thing you'd need to make sure of is that eventHandlerRegistry never gets polluted with something like this:

put(FooEvent.class, new BarEventHandler())); //this can happen
ernest_k
  • 44,416
  • 5
  • 53
  • 99
  • Thanks, seems that this would indeed work. I'm not really concerned about the `eventHandlerRegistry` getting polluted, since this is created only once, and I might as well make it immutable and write a test for it to assure it is not polluted. However, with your solution there is an unchecked cast and with my solution there is an unchecked call... Not sure which one is better/worse...? – Patric Dec 13 '19 at 13:37
  • 1
    @Tagas Your code is using a raw type, which is one of the first things to avoid if you're dealing with generic code. However, my code too is not type-safe, as I stated, but you do guarantee that the warning is taken care of (by making sure that `eventHandlerRegistry` has only correct *mapping*) – ernest_k Dec 13 '19 at 13:45
  • 1
    Still thinking... What's the advantage of making `handleEvent()` generic? I might as well cast the return value of `this.eventHandlerRegistry.get()` to `EventHandler` directly, without making the `handleEvent()` method generic. This would also only result in 1 unchecked cast warning. – Patric Dec 13 '19 at 14:24
  • 1
    The only reason for making `handleEvent` generic is to support `EventHandler` inside the method. Without the parameter `T`, you have to declare `handler` as `EventHandler`, which is worse than `EventHandler` even if `T` isn't used safely; it at least sticks to `T` being a subtype of `Event` rather than to `Event` itself (or worse, the raw type `EventHandler` as in the original code). So, yes, you can declare the method as `void handleEvent(Event event)`, but then that defeats the point of having a parameterized `EventHandler` (which made me ask the first question in comments) – ernest_k Dec 13 '19 at 14:35
1

Here's an example of what you can do using a new EventType enum:

Declare the enum:

public enum EventType {
    FOO_EVENT, BAR_EVENT
}

Declare Event interface:

interface Event {
    EventType getType();
}

class FooEvent implements Event {
    EventType getType() {
        return FOO_EVENT;
    }
}

class BarEvent implements Event {
    EventType getType() {
        return BAR_EVENT;
    }
}

and EventHandlers:

interface EventHandler {
    void handle(Event event);
}

class FooEventHandler implements EventHandler {
    @Override
    public void handle(Event event) {
        //cast Event to FooEvent when processing
    }
}

class BarEventHandler implements EventHandler {
    @Override
    public void handle(Event event) {
        //cast Event to BarEvent when processing
    }
}

Declare a Map between EventType and EventHandler:

Map<EventType, EventHandler> eventHandlerRegistry = //... fill the map here

And finally, when an event occurs, simply do:

eventHandlerRegistry.get(event.getType()).handle(event);

P.S. Note, for enums it's better to use java.util.EnumMap.

Pavel Smirnov
  • 4,611
  • 3
  • 18
  • 28
  • Thanks for the suggestion. But unfortunately it does not really solve my problem, because when I use `EventType` as a generic parameter, then I would still need to cast the `Event` i receive in the `handle()` method based on that type, which is what I tried to avoid in the first place... – Patric Dec 13 '19 at 13:39
  • 1
    @Tagas, updated the answer. I'm afraid you can not do it without casting, but what you can do is to ensure that casting is safe (which is protected by `EventType` and the map). – Pavel Smirnov Dec 13 '19 at 13:45