patternjavaMinor
My EventBus system followup
Viewed 0 times
followupsystemeventbus
Problem
This question is a followup from my previous question My EventBus system, and incorporates most points from @rolfl's answer.
It includes, but is not limited to:
The code:
```
public class SimpleEventBus implements EventBus {
private final static Set EMPTY_SET = new HashSet<>();
private final static EventHandler[] EMPTY_ARRAY = new EventHandler[0];
private final ConcurrentMap, Set> eventMapping = new ConcurrentHashMap<>();
private final Class eventClassConstraint;
public SimpleEventBus() {
this(Object.class);
}
public SimpleEventBus(final Class eventClassConstraint) {
this.eventClassConstraint = Objects.requireNonNull(eventClassConstraint);
}
@Override
public void registerListenersOfObject(final Object callbackObject) {
Arrays.stream(callbackObject.getClass().getMethods())
.filter(this::isEligibleMethod)
.forEach(method -> {
Class eventClass = method.getParameterTypes()[0];
addEventHandler(eventClass, new MethodEventHandler(method, callbackObject, eventClass));
});
}
@Override
@SuppressWarnings("unchecked")
public void r
It includes, but is not limited to:
- Usage of
Collections.synchronizedSetover manualsynchronized { }on trivial methods.
- Minimal locking
- High performance code, but not going into micro-optimizations if it harms the readability of the code.
The code:
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface Event { }public interface EventBus {
void registerListenersOfObject(final Object callbackObject);
void registerListener(final Class eventClass, final Consumer eventListener);
void executeEvent(final Object event);
void removeListenersOfObject(final Object callbackObject);
void removeListener(final Class eventClass, final Consumer eventListener);
void removeAllListenersOfEvent(final Class eventClass);
void removeAllListeners();
}```
public class SimpleEventBus implements EventBus {
private final static Set EMPTY_SET = new HashSet<>();
private final static EventHandler[] EMPTY_ARRAY = new EventHandler[0];
private final ConcurrentMap, Set> eventMapping = new ConcurrentHashMap<>();
private final Class eventClassConstraint;
public SimpleEventBus() {
this(Object.class);
}
public SimpleEventBus(final Class eventClassConstraint) {
this.eventClassConstraint = Objects.requireNonNull(eventClassConstraint);
}
@Override
public void registerListenersOfObject(final Object callbackObject) {
Arrays.stream(callbackObject.getClass().getMethods())
.filter(this::isEligibleMethod)
.forEach(method -> {
Class eventClass = method.getParameterTypes()[0];
addEventHandler(eventClass, new MethodEventHandler(method, callbackObject, eventClass));
});
}
@Override
@SuppressWarnings("unchecked")
public void r
Solution
There are two bugs related to type system:
Currently if all three parameter types,
In the below case I register a listener for events of type
This happens because in the
To fix this;
The second type problem is this:
When I register a handler for
Conversely if I register a
To fix this; you should replace
Currently if all three parameter types,
registerListener's first, and registered Consumer's and executeEvent's, it works as expected.@Test
public void worksAsExpected() {
String expected = "WORKS";
AtomicReference actual = new AtomicReference<>(null);
EventBus bus = new SimpleEventBus(String.class);
bus.registerListener(String.class, (String s)-> {actual.set(s);});
bus.executeEvent(expected);
assertEquals(expected, actual.get());
}In the below case I register a listener for events of type
Number and execute and event of type Number but it fails to execute the handler:@Test
public void shouldHaveWorkedButDoesNot() {
Number expected = 1;
AtomicReference actual = new AtomicReference<>(null);
EventBus bus = new SimpleEventBus(Number.class);
bus.registerListener(Number.class, (Number n)-> {actual.set(n);});
bus.executeEvent(expected);
assertEquals(expected, actual.get());
}This happens because in the
getCopyOfEventHandlers only gets the handlers registered for runtime type of the executed event. It should get handlers registered for the runtime type or any super-type thereof.To fix this;
getCopyOfEventHandlers should be something like:private Set getEventHandlersFor(final Class eventClass) {
return eventMapping.entrySet().stream()
.filter(entry -> entry.getKey().isAssignableFrom(eventClass))
.flatMap(entry -> entry.getValue().stream())
.collect(Collectors.toSet());
}The second type problem is this:
When I register a handler for
Numbers and it will run for any Numbers. Some might be Floats, others might be AtomicLongs. If I register a Consumer to handle Numbers and pass in a Float, which is a Number, I will get a runtime exception.@Test
public void shouldNotCompileButDoes() {
EventBus bus = new SimpleEventBus(Number.class);
// Not all Numbers are AtomicIntegers so this is a type error
bus.registerListener(Number.class,
(AtomicInteger n)-> {System.out.println(n.get());});
Number someNumber = 4f;
bus.executeEvent(someNumber); // e.g. Float does not have .get()
}Conversely if I register a
Consumer to handle Numbers and pass in any Float, I will always run. In fact obviously a Consumer can accept any object.@Test
public void shouldCompileButDoesNot() {
EventBus bus = new SimpleEventBus(Number.class);
// All Numbers are Objects this would work
// bus.registerListener(Number.class,
// (Object o)-> {System.out.println(o);});
Number someNumber = 4f;
bus.executeEvent(someNumber);
}To fix this; you should replace
Consumer eventListener in registerListeners signature with Consumer. In fact because the T only appears a parameter type in Consumer declaration, a parameter of type Consumer should either be a type error or could be replaced by just Consumer. I suspect latter might be the case for removeListener.Code Snippets
@Test
public void worksAsExpected() {
String expected = "WORKS";
AtomicReference<String> actual = new AtomicReference<>(null);
EventBus bus = new SimpleEventBus(String.class);
bus.registerListener(String.class, (String s)-> {actual.set(s);});
bus.executeEvent(expected);
assertEquals(expected, actual.get());
}@Test
public void shouldHaveWorkedButDoesNot() {
Number expected = 1;
AtomicReference<Number> actual = new AtomicReference<>(null);
EventBus bus = new SimpleEventBus(Number.class);
bus.registerListener(Number.class, (Number n)-> {actual.set(n);});
bus.executeEvent(expected);
assertEquals(expected, actual.get());
}private Set<EventHandler> getEventHandlersFor(final Class<?> eventClass) {
return eventMapping.entrySet().stream()
.filter(entry -> entry.getKey().isAssignableFrom(eventClass))
.flatMap(entry -> entry.getValue().stream())
.collect(Collectors.toSet());
}@Test
public void shouldNotCompileButDoes() {
EventBus bus = new SimpleEventBus(Number.class);
// Not all Numbers are AtomicIntegers so this is a type error
bus.registerListener(Number.class,
(AtomicInteger n)-> {System.out.println(n.get());});
Number someNumber = 4f;
bus.executeEvent(someNumber); // e.g. Float does not have .get()
}@Test
public void shouldCompileButDoesNot() {
EventBus bus = new SimpleEventBus(Number.class);
// All Numbers are Objects this would work
// bus.registerListener(Number.class,
// (Object o)-> {System.out.println(o);});
Number someNumber = 4f;
bus.executeEvent(someNumber);
}Context
StackExchange Code Review Q#48970, answer score: 5
Revisions (0)
No revisions yet.