Avoiding an unverified act to translate to a common interface collection in Java for an event publisher

Question

I am trying to create an easy, thread-safe publishing / subscribing mechanism in an application for the Android application that I am creating. My main approach is to keep track of the IEventSubscriber<T> list for each type of event T, and then be able to publish events for the objects being signed, passing a payload of type T.

I use general method parameters to (I think) ensure that subscriptions are created with a safe type. Thus, I am sure that when I get the list of subscribers from my subscription card, when it comes time to publish an event that I am fine by listing it in the IEventSubscriber<T> list, however, this generates a warning without warning.

My questions:

• Is the unchecked box really safe here?
• How can I check if elements in the IEventSubscriber<T> subscriber list are implemented?
• Assuming that (2) involves some unpleasant thoughts, what would you do here?

Code (Java 1.6):

 import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArraySet; public class EventManager { private ConcurrentMap<Class, CopyOnWriteArraySet<IEventSubscriber>> subscriptions = new ConcurrentHashMap<Class, CopyOnWriteArraySet<IEventSubscriber>>(); public <T> boolean subscribe(IEventSubscriber<T> subscriber, Class<T> eventClass) { CopyOnWriteArraySet<IEventSubscriber> existingSubscribers = subscriptions. putIfAbsent(eventClass, new CopyOnWriteArraySet<IEventSubscriber>()); return existingSubscribers.add(subscriber); } public <T> boolean removeSubscription(IEventSubscriber<T> subscriber, Class<T> eventClass) { CopyOnWriteArraySet<IEventSubscriber> existingSubscribers = subscriptions.get(eventClass); return existingSubscribers == null || !existingSubscribers.remove(subscriber); } public <T> void publish(T message, Class<T> eventClass) { @SuppressWarnings("unchecked") CopyOnWriteArraySet<IEventSubscriber<T>> existingSubscribers = (CopyOnWriteArraySet<IEventSubscriber<T>>) subscriptions.get(eventClass); if (existingSubscribers != null) { for (IEventSubscriber<T> subscriber: existingSubscribers) { subscriber.trigger(message); } } } } 

Answer 1

Is this checkbox absolutely safe here?

Enough. Your code will not cause heap contamination because the signature of the signature ensures that you only put IEventSubscribers of the correct type of compilation time on the map. It can spread heap contamination caused by an unsafe uncontrolled throw elsewhere, but there is little you can do about it.

How can I check if the items in the list of subscribers actually implement IEventSubscriber?

Retrieve each item before IEventSubscriber . Your code already does this on the following line:

 for (IEventSubscriber<T> subscriber: existingSubscribers) { 

If existingSubscribers contains an object not assigned by IEventSubscriber , this line will throw a ClassCastException. It is standard practice to avoid a warning when iterating through a list of an unknown type parameter is to explicitly click each element:

 List<?> list = ... for (Object item : list) { IEventSubscriber<T> subscriber = (IEventSubscriber<T>) item; } 

This code explicitly checks that each element is an IEventSubscriber , but cannot verify that it is an IEventSubscriber<T> .

To really check for a parameter of type IEventSubscriber , IEventSubscriber should help you. This is due to erasure, in particular, subject to declaration

 class MyEventSubscriber<T> implements IEventSubscriber<T> { ... } 

The following expression will always be true:

 new MyEventSubscriber<String>.getClass() == new MyEventSubscriber<Integer>.getClass() 

Assuming that (2) includes some unpleasant reflection, what would you do here?

I would leave the code as is. It’s easy enough to understand that the cast is correct, and I wouldn’t consider it necessary to rewrite it for compilation without warning. If you want to rewrite it, the following idea may come up:

 class SubscriberList<E> extends CopyOnWriteArrayList<E> { final Class<E> eventClass; public void trigger(Object event) { E event = eventClass.cast(event); for (IEventSubscriber<E> subscriber : this) { subscriber.trigger(event); } } } 

and

 SubscriberList<?> subscribers = (SubscriberList<?>) subscriptions.get(eventClass); subscribers.trigger(message); 

Answer 2

Not really. It will be safe if all clients of the EventManager class always use generics and never use rawtypes; that is, if the client code is compiled without warnings related to the generic.

However, for client code, it’s easy to ignore them and insert IEventSubscriber waiting for the wrong type:

 EventManager manager = ...; IEventSubscriber<Integer> integerSubscriber = ...; // subscriber expecting integers // casting to a rawtype generates a warning, but will compile: manager.subscribe((IEventSubscriber) integerSubscriber, String.class); // the integer subscriber is now subscribed to string messages // this will cause a ClassCastException when the integer subscriber tries to use "test" as an Integer: manager.publish("test", String.class); 

I don’t know how to compile to avoid this scenario, but you can check the general parameter types of instances of IEventSubscriber<T> at runtime if the generic type T was bound to the class at compile time. Consider:

 public class ClassA implements IEventSubscriber<String> { ... } public class ClassB<T> implements IEventSubscriber<T> { ... } IEventSubscriber<String> a = new ClassA(); IEventSubscriber<String> b = new ClassB<String>(); 

In the above example for ClassA , String bound to the T parameter at compile time. All instances of ClassA will have a String for T in IEventSubscriber<T> . But in ClassB String bound to T at runtime. ClassB instances can have any value for T If your IEventSubscriber<T> implementations bind the T parameter at compile time, as noted above, to ClassA , you can get this type at run time as follows:

 public <T> boolean subscribe(IEventSubscriber<T> subscriber, Class<T> eventClass) { Class<? extends IEventSubscriber<T>> subscriberClass = subscriber.getClass(); // get generic interfaces implemented by subscriber class for (Type type: subscriberClass.getGenericInterfaces()) { ParameterizedType ptype = (ParameterizedType) type; // is this interface IEventSubscriber? if (IEventSubscriber.class.equals(ptype.getRawType())) { // make sure T matches eventClass if (!ptype.getActualTypeArguments()[0].equals(eventClass)) { throw new ClassCastException("subscriber class does not match eventClass parameter"); } } } CopyOnWriteArraySet<IEventSubscriber> existingSubscribers = subscriptions.putIfAbsent(eventClass, new CopyOnWriteArraySet<IEventSubscriber>()); return existingSubscribers.add(subscriber); } 

This will cause the types to be checked when the subscriber is registered using the EventManager , which will make it easier for you to track bad code, and not just if the types will just be checked much later when the event is published. However, it does make some focal reflection and can only check those types if T bound at compile time. If you can trust the code that will send EventManager subscribers, I would just leave the code as it is, because it is much simpler. However, type checking using reflection, as mentioned above, will make you a bit more secure IMO.

One more note, you might want to reorganize the way you initialize your CopyOnWriteArraySet s, because the subscribe method is currently creating a new set for each call, regardless of whether it is needed or not. Try the following:

 CopyOnWriteArraySet<IEventSubscriber> existingSubscribers = subscriptions.get(eventClass); if (existingSubscribers == null) { existingSubscribers = subscriptions.putIfAbsent(eventClass, new CopyOnWriteArraySet<IEventSubscriber>()); } 

This avoids creating a new CopyOnWriteArraySet each time the method is called, but if you have a race condition and two threads try to install immediately, putIfAbsent will still return the first set created in the second thread, so there is no danger of overwriting it.

Answer 3

Since your subscribe implementation ensures that every Class<?> Key in ConcurrentMap displays the correct IEventSubscriber<?> , It is safe to use @SuppressWarnings("unchecked") when retrieving from a map in publish .

Just make sure you document the reason the warning is suppressed so that any future developer who makes changes to the class knows what is happening.

See also these related posts:

Shared key / value map with associated types

Java map with values ​​limited by key type parameter