Simple event manager in Java

I have created a simple event manager and I want it to work properly in a multithreaded environment:

```
public enum EventType {
EVENT_TYPE_A,
EVENT_TYPE_B,
EVENT_TYPE_C;
}

public interface Observer {
void onEvent(EventType eventType);
}

public class ObserverManager {
private Set m_weakReferencedObservers;

public ObserverManager() {
m_weakReferencedObservers = Collections.newSetFromMap(new WeakHashMap()); // allows a set of weak references
}

public void add(Observer observer){
if (observer != null) {
m_weakReferencedObservers.add(observer);
}
}

public void remove(Observer observer){
m_weakReferencedObservers.remove(observer);
}

public void callEvent(EventType eventType){
m_weakReferencedObservers.stream()
.filter(Objects::nonNull)
.forEach(observer -> observer.onEvent(eventType));

m_weakReferencedObservers.remove(null);
}
}

public class EventManager {
private Map m_observerManagers = new HashMap<>();

public void registerObserver(EventType eventType,Observer observer){
synchronized (m_observerManagers){
if (!m_observerManagers.containsKey(eventType)){
m_observerManagers.put(eventType,new ObserverManager());
}

ObserverManager observerManager = m_observerManagers.get(eventType);
observerManager.add(observer);
}
}

public void unregisterObserver(EventType eventType,Observer observer){
synchronized (m_observerManagers) {
if (!m_observerManagers.containsKey(eventType)) {
return;
}

ObserverManager observerManager = m_observerManagers.get(eventType);
observerManager.remove(observer);
}
}

public void callEvent(EventType eventType){
if (!m_observerManagers.containsKey(eventType)){
return;
}

ObserverManage

Answers to Your Questions

-
Yes, this usage is acceptable if your intention was to have a Set of Observer objects that can be removed by the garbage collector if they are not referenced from elsewhere.

-
Instead of using the old-school synchronized keyword, I'd recommend to add a ReentrantReadWriteLock field in EventManager and use it in the three methods.

-
I think that your implementation is already quite simple, readable and usable. Making it better would depend on the goal you are looking for. But there are still a few remarks after these answers.

-
If you add another method like onEvent(EventType, Class), it will have drawbacks such as pollution of the APIs of the objects that do not need the second method, ambiguities in the choice of the method to use for the caller, etc. If you want to grant your user a possibility to define custom events, the simplest way that I see here is to replace the EventType argument with String: the API will not be coupled with the rigid enum (which needs to be modified in order to introduce a new event type) and the user will be able to define his own event types easily.

Other Remarks

-
Style: the m_ prefix for the private fields violates standard Java naming conventions.

-
There is no need to call observerManagers.containsKey() to check if the item is already in the map. For Maps, there is the putIfAbsent method that does exactly the same in one line.

-
I'm not sure that .filter(Objects::nonNull) and weakReferencedObservers.remove(null) are necessary in ObserverManager.callEvent. How do you expect null values to be present in the collection, if add method checks the object to be not null?

StackExchange Code Review Q#139895, answer score: 3