Vogel612 covered most of the style points, as well as the very appropriate challenge "is this worth doing".
But if you are going to do it....
An important idea to understand in writing this code well is that you have two very different concerns embedded in AEventPublisher. The first of these is a Publisher; there is also a manager of EventListeners. Things will be much clearer if we expose these abstractions
public interface Publisher<E> {
public void publish(E sender);
}
public interface Observable<L> {
void subscribe(L listener);
void unsubscribe(L listener);
}
Notice that these generics are unconstrained - the concepts we are representing here are generic, so the types should express that. The complexity that is specific to this use case will be represented in the classes that implement the use case.
Speaking of which
interface EventListener<E, P> {
void actionPerformed(P publisher, E event);
}
We simplify the EventListener as well.
As a side note, I'm not too keen on actionPerformed. "Action Performed" is a great (excusing the vagueness) name for an event, but it's wrong for an object method. I'd strongly prefer either onEvent -- for a generic handler -- or onEventHappened. The fact that we're anticipating generics does make good naming harder; but I think onActionPerformed would be more consistent with best practices.
At this point, we have the interfaces that we need to do something scary....
abstract static class EventPublisher
< E
, P extends EventPublisher<E,P>>
implements
Publisher<E>
, Observable<EventListener<E,P>> {
List<EventListener<E,P>> listeners;
public void subscribe(EventListener<E,P> listener) {
listeners.add(listener);
}
public void unsubscribe(EventListener<E,P> listener) {
listeners.remove(listener);
}
@Override
public void publish(E event) {
for(EventListener<E,P> listener : listeners){
listener.actionPerformed(self(),event);
}
}
// NOTE: this should only be implement in final classes!
abstract protected P self();
}
Disclaimer: crazy formatting for readability is a code smell -- a good code review should challenge you hard on whether this introduces unnecessary complexity.
So what the heck is this? Breaking it down, we're defining a class with two generic parameters. E is an event, which can be anything; P is... well, it's anything that extends the EventPublisher we are currently defining. The class also promises to implement the nice clean Publisher and Observable interfaces we defined earlier.
Through the generics, we're able to resolve the circular dependency; it's all syntactic sugar, so although we get turtled all the way down, the types are all going to be erased when the compiler goes to work.
Why are we doing this crazy thing?
The publish method was made protected and abstract, because I couldnt think of a way to get this in AEventPublisher to be casted as the Publisher extends AEventPublisher
Right -- we simply cannot use this in the base class and preserve the specialized behavior that we want. We beat with the seemingly self referential P parameter in the signature -- our specialized EventPublisher is going to provide an implementation of self() with the exact signature we need it to have for the EventListeners to do the right thing.
Let's try an example - I wanted my IDE to show me that things actually worked, so first I created some implementations to be used in the template specialization.
interface Joystick {
int getDirection();
}
static class Game {
static void updatePlayerDirection(Joystick joystick) {
joystick.getDirection();
}
}
That's enough to prove that the EventHandler is really handling the right type of event. But I also want to see that it can really distinguish the right kind of EventPublisher, so I add an extra interface that the JoystickEventPublisher itself is expected to implement.
interface BobNotifier {
void notifyBob();
}
Clearly, I'm running low on creativity here.
OK, showtime - what does the JoystickEventPublisher look like?
final static class JoystickEventPublisher extends EventPublisher<Joystick, JoystickEventPublisher>
implements BobNotifier {
@Override
public void notifyBob() {
//...
}
// We're "allowed" to implement self(), because this class is final.
protected JoystickEventPublisher self() {
return this;
}
}
JoystickEventPublisher extends EventPublisher<E,JoystickEventPublisher<E,P>> which means it really is a P that extends EventPublisher<E,P> and the compiler is satisfied!
A JoystickEventListener which subscribes to the JoystickEventPublisher really gets to use methods that are not part of the EventPublisher interface:
static class JoystickEventListener implements EventListener<Joystick, JoystickEventPublisher> {
public void actionPerformed(JoystickEventPublisher publisher, Joystick joystick)
{
publisher.notifyBob();
Game.updatePlayerDirection(joystick);
}
};
And both subscription and publication "just work".
JoystickEventPublisher p = new JoystickEventPublisher();
p.subscribe(new JoystickEventListener());
p.publish(new Joystick() {
@Override
public int getDirection() {
return 0;
}
});
A couple of further points. The "abstract self()" pattern shows up in other contexts -- it's very useful for implementing fluent builders. So you might consider teasing that piece out.
abstract static class AbstractSelf<T extends AbstractSelf<T>> {
// NOTE: this should only be implement in final classes!
abstract protected T self();
}
abstract static class EventPublisher
< E
, P extends EventPublisher<E,P>>
extends
AbstractSelf<P>
implements
Publisher<E>
, Observable<EventListener<E,P>> {
//...
}
Which is nice, clean, re-usable.... It works great when you are building a new class from scratch. However, if you are trying to extend some other class, the language won't allow you to extend a second class, and you'll have to re-write the self() declaration anyway. It may not be worth the effort.
Second - you may not want to restrict the kinds of Listeners are attached to
EventPublishers. For instance, you might have a generic listener that wants to subscribe to joystick events and keyboard events. We can support that here by loosening the constraints on the listeners. That's going to change the Observer part of the interface while leaving everything else alone -- another hint that observe and publish are two separate concerns.
abstract static class EventPublisher
< E
, P extends EventPublisher<E,P>>
implements
Publisher<E>
, Observable<EventListener<? super E, ? super P>> {
List<EventListener<? super E, ? super P>> listeners;
public void subscribe(EventListener<? super E, ? super P> listener) {
listeners.add(listener);
}
public void unsubscribe(EventListener<? super E, ? super P> listener) {
listeners.remove(listener);
}
@Override
public void publish(E event) {
for(EventListener<? super E, ? super P> listener : listeners){
listener.actionPerformed(self(),event);
}
}
// NOTE: this should only be implement in final classes!
abstract protected P self();
}
The compiler is happy enough....
p.subscribe(new EventListener<Object, Object>() {
@Override
public void actionPerformed(Object publisher, Object event) {
//...
}
});
Finally, you may find that you want to extend JoystickEventPublisher. In the implementation above, the class was deliberately declared final to close that door, because the definition of self() provided by JoystickEventPublisher is going to prevent the event listeners from accessing any new methods in ExtendedJoystickEventPublisher. The right answer is to refactor the JoystickEventPublisher into two pieces - an abstract and extendable piece that has all of the interesting parts of the interface, and a final concrete piece that implements self()
abstract static class AbstractJoystickEventPublisher<P extends AbstractJoystickEventPublisher<P>> extends EventPublisher<Joystick, P>
implements BobNotifier {
@Override
public void notifyBob() {
//...
}
}
final static class JoystickEventPublisher extends AbstractJoystickEventPublisher<JoystickEventPublisher> {
// We're "allowed" to implement self(), because this class is final.
protected JoystickEventPublisher self() {
return this;
}
}
This implementation allows us to create a kind of JoystickEventPublisher which shares the implementation of the BobNotifier interface in addition to providing new methods....
abstract static class AbstractEnhancedJoystickEventPublisher<P extends AbstractEnhancedJoystickEventPublisher<P>> extends AbstractJoystickEventPublisher<P> {
public void notifyAlice() {
// ...
}
}
final static class EnhancedJoystickEventPublisher extends AbstractEnhancedJoystickEventPublisher<EnhancedJoystickEventPublisher> {
protected EnhancedJoystickEventPublisher self() {
return this;
}
}
