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?

6
  • Is this all the uses of your event handlers? If so, why is EventHandler<T extends Event> 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, 2019 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<String> 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, 2019 at 12:41
  • Is Event a singleton? Can you have many events of the same type? If not, you can try using Map<Event, EventHandler<? extends Event>> 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> Dec 13, 2019 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, 2019 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 OrderSandwichEvents 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, 2019 at 13:00

2 Answers 2

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
4
  • 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, 2019 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, 2019 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<Event> directly, without making the handleEvent() method generic. This would also only result in 1 unchecked cast warning.
    – Patric
    Dec 13, 2019 at 14:24
  • 1
    The only reason for making handleEvent generic is to support EventHandler<T> inside the method. Without the parameter T, you have to declare handler as EventHandler<Event>, which is worse than EventHandler<T> 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, 2019 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.

2
  • 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, 2019 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). Dec 13, 2019 at 13:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.