Generics

[kofa]

Hi,

I have a problem that I have been unable to code cleanly with
generics. I suspect it is not possible because of no run-time generics
info. The problematic code (a cast, highlighted in capitals in
comments) is in dispatchEvent(Event e) below.

Suppose I have a hierarchy of events. XEvent extends Event, YEvent
extends Event etc.

The purpose is to provide a type-safe event listener and an event
dispatcher.

I could create the interface:
public interface <T extends Event> EventListener {
void eventRaised(T event);
}

And also:
public interface EventManager {
<T extends Event> void addListener(EventManager<T> listener,
Class<T> type);
<T extends Event> void removeListener(EventManager<T> listener,
Class<T> type);
void raiseEvent(Event event);
}

I think the fact that I need to specify T in addListener twice (in
listener type and class type) already shows something's wrong...

Now implementing this seems impossible without casts and unchecked
types:
public class EventManagerImpl implements EventManager {
// no way to express the binding between event subclass as Map key
and event type of listener
private final Map<Class<? extends Event>, Set<EventListener<?
extends Event>>> myListenersByType = new HashMap<Class<? extends
Event>, Set<EventListener<? extends Event>>>();

// addListener, removeListener omitted for brevity

public void dispatchEvent(Event event) {
for (Map.Entry<Class<? extends Event>, Set<EventListener<? extends
Event>>> entry : myListenersByType.entrySet()) {
if (entry.getKey().isInstance(event)) {
for (EventListener<? extends Event> listener:
entry.getValue()) {
// UGLY CAST HERE - could this be avoided?
((EventListener<Event>) listener).eventRaised(event);
}
}
}
}
}

TIA,
Kofa

 

[Roedy Green]

I have a problem that I have been unable to code cleanly with
generics. I suspect it is not possible because of no run-time generics
info. The problematic code (a cast, highlighted in capitals in
comments) is in dispatchEvent(Event e) below.

These kind of questions cause migraine headaches, so I doubt you will
get many bites on this toughie. Here is some generic advice.

Read the FAQs pointed to at http://mindprod.com/jgloss/generics.html
so that you understand the common gotchas. The answer to your problem
might then fall in your lap.

 

[Oliver Wong]

Hi,

I have a problem that I have been unable to code cleanly with
generics. I suspect it is not possible because of no run-time generics
info. The problematic code (a cast, highlighted in capitals in
comments) is in dispatchEvent(Event e) below.

Suppose I have a hierarchy of events. XEvent extends Event, YEvent
extends Event etc.

The purpose is to provide a type-safe event listener and an event
dispatcher.

I could create the interface:
public interface <T extends Event> EventListener {
void eventRaised(T event);
}


And also:
public interface EventManager {
<T extends Event> void addListener(EventManager<T> listener,
Class<T> type);
<T extends Event> void removeListener(EventManager<T> listener,
Class<T> type);
void raiseEvent(Event event);
}

Doesn't make sense. EventManager is not generic, so you cannot refer

I think the fact that I need to specify T in addListener twice (in
listener type and class type) already shows something's wrong...

Now implementing this seems impossible without casts and unchecked
types:
public class EventManagerImpl implements EventManager {
// no way to express the binding between event subclass as Map key
and event type of listener
private final Map<Class<? extends Event>, Set<EventListener<?
extends Event>>> myListenersByType = new HashMap<Class<? extends
Event>, Set<EventListener<? extends Event>>>();

// addListener, removeListener omitted for brevity

public void dispatchEvent(Event event) {
for (Map.Entry<Class<? extends Event>, Set<EventListener<? extends
Event>>> entry : myListenersByType.entrySet()) {
if (entry.getKey().isInstance(event)) {
for (EventListener<? extends Event> listener:
entry.getValue()) {
// UGLY CAST HERE - could this be avoided?
((EventListener<Event>) listener).eventRaised(event);
}
}
}
}
}

Is the dispatchEvent(Event) method actually the raiseEvent(Event)
method?

All these inconsistencies is making it more difficult for me to guess
what exactly it is you're trying to do. Anyway, here's my random guess at
what you intended. The key to my solution is to define your own custom Map
class.

<code>
import java.awt.Event;
import java.util.Set;

interface EventListener<T extends Event> {
void eventRaised(T event);
}

interface EventManager {
<T extends Event> void addListener(EventListener<T> listener, Class<T>
type);

<T extends Event> void removeListener(EventListener<T> listener, Class<T>
type);

void raiseEvent(Event event);
}

class EventMap {
public <T extends Event> Set<EventListener<? super T>> get(Class<?
extends T> key) {
// TODO Auto-generated method stub
return null;
}

public Set<Class<? extends Event>> keySet() {
// TODO Auto-generated method stub
return null;
}
}

class EventManagerImpl implements EventManager {

private final EventMap myListenersByType = new EventMap();

@Override
public <T extends Event> void addListener(EventListener<T> listener,
Class<T> type) {
// TODO Auto-generated method stub
}

@Override
public void raiseEvent(Event event) {
for (Class<? extends Event> key : myListenersByType.keySet()) {
if (key.isInstance(event)) {
Set<EventListener<? super Event>> listeners =
myListenersByType.get(key);
for (EventListener<? super Event> listener : listeners) {
listener.eventRaised(event);
}
}
}
}

@Override
public <T extends Event> void removeListener(EventListener<T> listener,
Class<T> type) {
// TODO Auto-generated method stub
}
}
</code>

- Oliver

 

[Owen Jacobson]

Hi,

I have a problem that I have been unable to code cleanly with
generics. I suspect it is not possible because of no run-time generics
info. The problematic code (a cast, highlighted in capitals in
comments) is in dispatchEvent(Event e) below.

Suppose I have a hierarchy of events. XEvent extends Event, YEvent
extends Event etc.

The purpose is to provide a type-safe event listener and an event
dispatcher.

I could create the interface:
public interface <T extends Event> EventListener {
void eventRaised(T event);

}

And also:
public interface EventManager {
<T extends Event> void addListener(EventManager<T> listener,
Class<T> type);
<T extends Event> void removeListener(EventManager<T> listener,
Class<T> type);
void raiseEvent(Event event);

}

I think the fact that I need to specify T in addListener twice (in
listener type and class type) already shows something's wrong...

Now implementing this seems impossible without casts and unchecked
types:

Truth 6a: It is always possible to add another level of indirection.
I've written generic event dispatchers for my own code, and I'm happy
to donate one as an example. The trick to this problem is to add
another class (which I called a "firer") which encapsulates the
knowledge of how to process each listener.

$ cat EventDispatcher.java
package xxxxx;

import java.util.ArrayList;
import java.util.List;
import java.util.ListIterator;

/**
* Event dispatcher and listener management class. Event listeners can
be
* registered with this class, and then subsequently fired. When an
event is
* fired by a dispatcher, all registered listeners are notified in
reverse order
* of registration.
*
* @param <L>
* the event listener type.
*/
public class EventDispatcher<L> {
private final List<L> listeners = new ArrayList<L> ();

/**
* Fire events on this dispatcher using a given event firer.
*
* @param firer
* the event firing mechanism to use.
*/
public void fire (final EventFirer<? super L> firer) {
if (firer == null) {
throw new IllegalArgumentException ("firer");
}

final ListIterator<L> iter = getIteratorAtEnd ();
while (iter.hasPrevious ()) {
firer.fireEvent (iter.previous ());
}
}

/**
* Adds an event listener to the dispatch sequence. Events will be
dispatched
* to the most recently added listener first. The same listener may
be added
* more than once, and will be notified once for each time it's
registered.
*
* @param listener
* the event listener to register.
* @see #removeListener(Object listener)
*/
public void addListener (final L listener) {
if (listener == null) {
throw new IllegalArgumentException ("listener");
}

listeners.add (listener);
}

/**
* Removes an event listener from the dispatch sequence. If the
listener was
* registered more than once, only the most recent occurrence is
removed.
*
* @param listener
* the event listener to remove.
* @see #addListener (Object listener)
*/
public void removeListener (final L listener) {
final ListIterator<L> listenersIterator = getIteratorAtEnd ();
while (listenersIterator.hasPrevious ()) {
final L registered = listenersIterator.previous ();
if (registered == listener) {
listenersIterator.remove ();
return;
}
}
}

private ListIterator<L> getIteratorAtEnd () {
return listeners.listIterator (listeners.size ());
}
}
// --- END ---




$ cat EventFirer.java
package xxxxx;

/**
* Interface for actually firing a single event, used by {@link
EventDispatcher}s.
*
* @param <L>
* the listener type to fire events on.
*/
public interface EventFirer<L> {
/**
* Fire the event on a single listener.
*
* @param listener
* the listener to fire.
*/
public void fireEvent (L listener);
}
// --- END ---

At this point the "Listener" interface can be any event listener with
no particular requirements for descending from any specific interface
or class, which is nice. The code that uses these classes looks like


/**
* Calls the {@link Listener#connected()} method on all attached
listeners.
*/
protected void fireConnected () {
connectorListeners.fire (new EventFirer<Listener> () {
public void fireEvent (Listener listener) {
listener.connected ();
}
});
}

This does tend to litter code with anonymous classes, but they're not
huge -- most of them are one line. As an added bonus the event
listener interfaces can have any combination of arguments; the
dispatcher doesn't worry about argument passing itself and firers are
easy to implement.

Owen

 

[kofa]

Hi Owen,

thanks for your reply.
It seems to me that in your solution there's no central "event
manager" that has the ability to intelligently notify all listeners
for supertypes as well. Could you please show me how that could be
implemented?

TIA,
Kofa

 

[kofa]

Hello Oliver,

thanks for your reply. You are right, my code was all broken - don't
know why I did not just copy-paste the code I had written...

Sorry for being a nuisance: in theory I know what "? super X" means
("X or any supertype"), but haven't used it in practice. You left
EventMap.get() unimplemented, here's what I came up with:
===
private final Map<Class<? extends Event>, Set<EventListener<?>>>
myListenersByType = new HashMap<Class<? extends Event>,
Set<EventListener<?>>>();
public <T extends Event> Set<EventListener<? super T>> get(Class<?
extends T> key) {
Set<EventListener<? super T>> listeners = new HashSet<EventListener<?
super T>>();
for (Entry<Class<? extends Event>, Set<EventListener<?>>> entry :
myListenersByType.entrySet()) {
if (entry.getKey().isAssignableFrom(key)) {
listeners.add((EventListener<? super T>) entry.getValue());
}
}
return listeners;
}
===

Thanks again,
Kofa

ps. tried to post this earlier, with a compilation error message - it
seems that post did't make it, which is good because the solution
seems to be here now.

 

[kofa]

OK, making a fool of myself (or just showing what I fool I am):
my solution above is wrong - Oliver actually does the instanceof check
in EventManagerImpl.raiseEvent. I should have written:
===
public <T extends Event> Set<EventListener<? super T>> get(Class<?
extends T> eventClass) {
Set<EventListener<?>> listenersForType =
myListenersByType.get(eventClass);
Set<EventListener<? super T>> listeners = new HashSet<EventListener<?
super T>>();
listeners.addAll((Collection<? extends EventListener<? super T>>)
myListenersByType.get(eventClass));
return listeners;
}
===

However, this gives the warning:
Type safety: The cast from Set<EventListener<?>> to Collection<?
extends EventListener<? super T>> is actually checking against the
erased type Collection

Copying manually, casting individual elements solves that:
===
public <T extends Event> Set<EventListener<? super T>> get(Class<?
extends T> key) {
Set<EventListener<? super T>> listeners = new HashSet<EventListener<?
super T>>();
for (Entry<Class<? extends Event>, Set<EventListener<?>>> entry :
myListenersByType.entrySet()) {
listeners.add((EventListener<? super T>) entry.getValue());
}
return listeners;
}
===

Is there a nicer way?

Kofa

 

[Roedy Green]

It seems to me that in your solution there's no central "event
manager" that has the ability to intelligently notify all listeners
for supertypes as well.

See http://mindprod.com/jgloss/event11.html
for a description of the inner working of the event system. There are
several places you can place your hooks.

 

[kofa]

I've tried filling in the gaps, and ended up with:
===
public class EventDispatcherImpl implements EventDispatcher {
[...]
public <T> void dispatchEvent(T event) {
for (EventListener<? super T> listener :
myListeners.get(event.getClass())) {
listener.eventRaised(event);
}
}

/**
* Utility class to map events to listeners
*/
private static class EventMap {
/**
* Stores the event type -> listners mapping.
*/
private final Map<Class<?>, Set<EventListener<?>>> myListenersByType
= new HashMap<Class<?>, Set<EventListener<?>>>();

/**
* Gets all listeners that can handle the specified event class.
* @param <T> the event type
* @param eventClass the event class
* @return the listeners that can handles the event class
*/
<T> Set<EventListener<? super T>> get(Class<? extends T> eventClass)
{
Set<EventListener<? super T>> listeners = new
HashSet<EventListener<? super T>>();
synchronized (myListenersByType) {
for (Map.Entry<Class<?>, Set<EventListener<?>>> entry :
myListenersByType.entrySet()) {
if (entry.getKey().isAssignableFrom(eventClass)) {
for (EventListener<?> listener : entry.getValue()) {
/* warning here:
Type safety: The cast from EventListener<capture-of ?> to
EventListener<? super T> is actually checking against the erased type
EventListener
*/
listeners.add((EventListener<? super T>) listener);
}
}
}
}
return listeners;
}
[...]
===

Now at least there's no need for an Event superclass/interface: I can
dispatch any object (that is, I can maintain multiple, unrelated event
hierarchies, and use a single dispatcher).
Is there a way (aside from @SuppressWarning) to eliminate the warning?

TIA,
Kofa

 

[Hendrik Maryns]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

kofa schreef:

I've tried filling in the gaps, and ended up with:
===
public class EventDispatcherImpl implements EventDispatcher {
[...]
public <T> void dispatchEvent(T event) {
for (EventListener<? super T> listener :
myListeners.get(event.getClass())) {
listener.eventRaised(event);
}
}

/**
* Utility class to map events to listeners
*/
private static class EventMap {
/**
* Stores the event type -> listners mapping.
*/
private final Map<Class<?>, Set<EventListener<?>>> myListenersByType
= new HashMap<Class<?>, Set<EventListener<?>>>();

/**
* Gets all listeners that can handle the specified event class.
* @param <T> the event type
* @param eventClass the event class
* @return the listeners that can handles the event class
*/
<T> Set<EventListener<? super T>> get(Class<? extends T> eventClass)
{
Set<EventListener<? super T>> listeners = new
HashSet<EventListener<? super T>>();
synchronized (myListenersByType) {
for (Map.Entry<Class<?>, Set<EventListener<?>>> entry :
myListenersByType.entrySet()) {
if (entry.getKey().isAssignableFrom(eventClass)) {
for (EventListener<?> listener : entry.getValue()) {
/* warning here:
Type safety: The cast from EventListener<capture-of ?> to
EventListener<? super T> is actually checking against the erased type
EventListener
*/
listeners.add((EventListener<? super T>) listener);
}
}
}
}
return listeners;
}
[...]
===

Now at least there's no need for an Event superclass/interface: I can
dispatch any object (that is, I can maintain multiple, unrelated event
hierarchies, and use a single dispatcher).
Is there a way (aside from @SuppressWarning) to eliminate the warning?

By making your map store EventListener<? super T> instead of <?>. I am
unsure whether that won’t cause other warnings/errors though.

H.
- --
Hendrik Maryns
http://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
http://aouw.org
Ask smart questions, get good answers:
http://www.catb.org/~esr/faqs/smart-questions.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGhSSVe+7xMGD3itQRAmh5AJwK8b4Ditbz1OZMzfZkI1m50328hACbBoW+
gCk9EDFx34MkwzIFVChdeT4=
=VuxX
-----END PGP SIGNATURE-----

 

[Oliver Wong]

Hello Oliver,

thanks for your reply. You are right, my code was all broken - don't
know why I did not just copy-paste the code I had written...

Sorry for being a nuisance: in theory I know what "? super X" means
("X or any supertype"), but haven't used it in practice. You left
EventMap.get() unimplemented, here's what I came up with:
===
private final Map<Class<? extends Event>, Set<EventListener<?>>>
myListenersByType = new HashMap<Class<? extends Event>,
Set<EventListener<?>>>();
public <T extends Event> Set<EventListener<? super T>> get(Class<?
extends T> key) {
Set<EventListener<? super T>> listeners = new HashSet<EventListener<?
super T>>();
for (Entry<Class<? extends Event>, Set<EventListener<?>>> entry :
myListenersByType.entrySet()) {
if (entry.getKey().isAssignableFrom(key)) {
listeners.add((EventListener<? super T>) entry.getValue());
}
}
return listeners;
}
===

Note that entry.getValue() returns an object of type
Set<EventListener<? super T>>, and you're casting that to EventListener<?
super T> which probably isn't what you want. Instead, you need to either
use addAll(), or iterate over the set and add each EventListener one by
one. Here's an example implementation of the later:

<code>
public <T extends Event> Set<EventListener<? super T>> get(
Class<? extends T> key) {
Set<EventListener<? super T>> listeners = new HashSet<EventListener<?
super T>>();
for (Entry<Class<? extends Event>, Set<EventListener<?>>> entry :
myListenersByType
.entrySet()) {
if (entry.getKey().isAssignableFrom(key)) {
for (EventListener<?> el : entry.getValue()) {
listeners.add((EventListener<? super T>) el);
}
}
}
return listeners;
}
</code>

- Oliver

 

[Oliver Wong]

Copying manually, casting individual elements solves that:
===
public <T extends Event> Set<EventListener<? super T>> get(Class<?
extends T> key) {
Set<EventListener<? super T>> listeners = new HashSet<EventListener<?
super T>>();
for (Entry<Class<? extends Event>, Set<EventListener<?>>> entry :
myListenersByType.entrySet()) {
listeners.add((EventListener<? super T>) entry.getValue());
}
return listeners;
}
===

Is there a nicer way?

Depends on your metric for "niceness". I haven't been able to fully
get rid of warnings. I can move the warning around to different locations
where they might make more sense. For example, you could have the
listeners provide a way to "cast themselves" to the proper type:

<code>
class EventImpl<T extends Event> implements EventListener<T> {
Class<? extends T> eventType;

@Override
public void eventRaised(T event) {
// TODO Auto-generated method stub

}

@Override
public <U extends Event> EventListener<? super U>
castSelfAsHandlerFor(Class<? extends U> clazz) {
if (eventType.isAssignableFrom(clazz)) {
return (EventListener<? super U>) this;
} else {
return null;
}
}
}
</code>

Or you could have the code in the event manager take advantage of the fact
that it just knows (but can't express this in the Java type system) that
Class<T> is mapped onto EventListener<T>:

<code>
@Override
public <T extends Event> void raiseEvent(T event) {
for (Class<? extends Event> key : myListenersByType.keySet()) {
if (key.isInstance(event)) {
Set<EventListener<? super T>> listeners = myListenersByType
.get((Class<? extends T>)key);
for (EventListener<? super T> listener : listeners) {
listener.eventRaised(event);
}
}
}
}
</code>

- Oliver

 

[kofa]

Here's my final solution. It does not have the EventMap class, and has
the same one typecast warning:

public class EventDispatcherImpl implements EventDispatcher {
/**
* Stores the event type -> listners mapping.
*/
private final Map<Class<?>, Set<EventListener<?>>> myListeners =
new HashMap<Class<?>, Set<EventListener<?>>>();

/**
* Creates an instance.
*/
public EventDispatcherImpl() {
super();
}

/**
* @see EventDispatcher#addListener(EventListener,
java.lang.Class)
*/
public <T> EventDispatcher addListener(EventListener<? super T>
listener, Class<T> eventClass) {
synchronized (myListeners) {
Set<EventListener<?>> listeners = myListeners.get(eventClass);
if (null == listeners) {
listeners = new HashSet<EventListener<?>>();
myListeners.put(eventClass, listeners);
}
listeners.add(listener);
}
return this;
}

/**
* @see EventDispatcher#removeListener(EventListener,
java.lang.Class)
*/
public <T> EventDispatcher removeListener(EventListener<? super T>
listener, Class<T> eventClass) {
synchronized (myListeners) {
Set<EventListener<?>> listeners = myListeners.get(eventClass);
if (null != listeners) {
listeners.remove(listener);
}
}
return this;
}

/**
* @see EventDispatcher#dispatchEvent(java.lang.Object)
*/
public <T> EventDispatcher dispatchEvent(T event) {
Set<EventListener<? super T>> listenersToNotify = new
HashSet<EventListener<? super T>>();
for (Map.Entry<Class<?>, Set<EventListener<?>>> entry :
myListeners.entrySet()) {
if (entry.getKey().isInstance(event)) {
Set<EventListener<?>> listeners = entry.getValue();
for (EventListener<?> listener : listeners) {
listenersToNotify.add((EventListener<? super T>) listener);
}
}
}
for (EventListener<? super T> listener : listenersToNotify) {
listener.eventRaised(event);
}
return this;
}
}

The nasty thing about it is the declaration of dispatchEvent. It uses
a type parameter T, but only once. The method has only one parameter
and a fixed return type, so it looks pretty much useless, at least in
the interface. Quoting Sun's Generics tutorial:
"The return type doesn't depend on the type parameter, nor does any
other argument to the method (in this case, there simply is only one
argument). This tells us that the type argument is being used for
polymorphism; its only effect is to allow a variety of actual argument
types to be used at different invocation sites. If that is the case,
one should use wildcards. Wildcards are designed to support flexible
subtyping, which is what we're trying to express here.
Generic methods allow type parameters to be used to express
dependencies among the types of one or more arguments to a method and/
or its return type. If there isn't such a dependency, a generic method
should not be used."
However, changing the declaration to:
public EventDispatcher dispatchEvent(Object event) {...} breaks the
code. If I replace the <? super T> with <?>, I get a compile error on
the line:
listener.eventRaised(event);
The method eventRaised(capture-of ?) in the type EventListener<capture-
of ?> is not applicable for the arguments (Object)

I'll leave it at that, I think.

Thanks for your help,
Kofa