HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Getting a set of subscribers from a subscriberMap

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gettingsubscribermapfromsubscribersset

Problem

In my Java event bus project, I have a private generic method that retrieves a Set> from a private Map, Set>> (the subscriberMap).

/**
 * A map of Event types and Subscriber Sets.
 * 
 * That is, the Set of Subscribers set to an Event type
 * is assumed to be used for that Event type.
 */
private final Map, Set>> subscriberMap = new ConcurrentHashMap<>();

private  void checkSubscriberMap(Class type)
{
    if (!subscriberMap.containsKey(type))
    {
        subscriberMap.put(type, new HashSet<>());
    }
}

/**
 * Gets a Set of Subscribers by an Event.
 *
 * @param event the event
 * @param  the type of event
 * @return a Set of Subscribers
 */
@SuppressWarnings("unchecked")
private  Set> getSubscribers(E event)
{
    return (Set>) (Object) getSubscribers(event.getClass());
}

/**
 * Gets a Set of Subscribers by an Event type.
 *
 * @param type the event class
 * @param  the type of event
 * @return a Set of Subscribers
 */
@SuppressWarnings("unchecked")
private  Set> getSubscribers(Class type)
{
    checkSubscriberMap(type); // checks if subscriberMap.get(type) is null

    return (Set>) (Object) subscriberMap.get(type);
}


Assuming nobody uses reflection to modify subscriberMap, the public methods that modify subscriberMap only work if the Subscriber is the same type as the Class. Therefore, I can be sure that getSubscribers() will always return a Set of the same type that you give it.

/**
 * Registers a single event handler under an explicit Event type.
 * Returns true if the handler was added.
 *
 * @param subscriber the Subscriber
 * @param type the type of Event
 */
public  boolean register(Subscriber subscriber, Class type)
{
    if(type == Event.class)
    {
        throw new IllegalArgumentException("The provided type is not a subclass of Event.");
    }

    return getSubscribers(type).add(subscriber);
}


However, it's bothering me how getSubscribers() looks. Why does it look I'm doing a hacky workaround? Is it necess

Solution

private  void checkSubscriberMap(Class type)


This method checks nothing. It ensures that there's a non-null value. Or it fixes or prepares something.

Note that you're using both contains and get every time. I guess, a method like

private  Set safeGet(Class type) {
   Set result =
           (Set>) (Object) subscriberMap.get(type);
   if (result == null) {
        result = new HashSet<>();
        subscriberMap.put(type, result);
   }
   return result;
}


would be better.

Consider using Guava's Multimap as it's been solved there.

@SuppressWarnings("unchecked")
private  Set> getSubscribers(E event)
{
    return (Set>) (Object) getSubscribers(event.getClass());
}


I'm afraid, that's right. The ugly double cast is needed as there's no way to tell the compiler that it works. Maybe the intermediate cast could be (non-generic) (Set).


Assuming nobody uses reflection

If somebody does, that's not your fault.

What you did is similar in idea to the MutableClassToInstanceMap, except that you need Set> instead of E itself and therefore you can't use type.cast(object) and have to resolve to an unchecked cast.

Code Snippets

private <E extends Event> void checkSubscriberMap(Class<E> type)
private <E extends Event> Set<E> safeGet(Class<E> type) {
   Set<E> result =
           (Set<Subscriber<E>>) (Object) subscriberMap.get(type);
   if (result == null) {
        result = new HashSet<>();
        subscriberMap.put(type, result);
   }
   return result;
}
@SuppressWarnings("unchecked")
private <E extends Event> Set<Subscriber<E>> getSubscribers(E event)
{
    return (Set<Subscriber<E>>) (Object) getSubscribers(event.getClass());
}

Context

StackExchange Code Review Q#91462, answer score: 2

Revisions (0)

No revisions yet.