views:

30

answers:

3

I'm refactoring an existing state machine that heavily used case statements and emums. I've broken it down to Events and eventhandler objects, each corresponding to a state machine. The eventHandlers return new Events to propagate through the state machine.

So, my code looks something like this:

public class Event {
    //common fields and methods
}

public class SpecificEvent extends Event {
    //fields specific to this event
}

public class AnotherEvent extends Event {
    //fields specific to this event
}

public class EventHandler {
    public Event handleEvent(SpecificEvent evt) {
        //do default something
    }
    public Event handleEvent(AnotherEvent evt) {
        //do default something else
    }
}

public class StateOneEventHandler extends EventHandler {
    public Event handleEvent(SpecificEvent evt) {
        //do State1 things
    }
    public Event handleEvent(AnotherEvent evt) {
        //do other State1 things
    }
}

public class StateTwoEventHandler extends EventHandler {
    public Event handleEvent(SpecificEvent evt) {
        //do State2 things
    }
    public Event handleEvent(AnotherEvent evt) {
        //do other State2 things
    }
}

My problem and question is: I'm only passing generic Event references around in my state machine, so how can I call the correct handler for an event?

Event evt = new SpecificEvent();
EventHandler handler = new StateOneEventHandler();

//... later

handler.handleEvent(evt); //compiler error

What is the best way of accomplishing this runtime event "dispatch" ?

+3  A: 

You're right that overloaded methods won't be able to resolve this (because the method to invoke is determined at compile time).

I would suggest that you'd need to refactor this, either to add a method to the event that takes the handler as a parameter (something like public void act(EventHandler handler)), or alternatively rewrite your handler so that it doesn't need to be strongly aware of the type of the event. If Event is a sensible interface/superclass, it will expose enough functionality itself such that the EventHandler doesn't need to know about the specific type.

You can of course always cast if you really really need to, but in general you should follow the Law Of Demeter and only interact with the event objects via the Event interface.

Andrzej Doyle
I like the adding the act() method to the event. My goal was to reduce boilerplate code, but I'd rather put it in the more-or-less fixed number of events(about 15 so far) than in every event handler (about 60 so far). Thanks!
Mark
+1  A: 

The decision which message is called is done at compile time. So in order to select a specific method variant, you need to add a cast.

You might want to have a look at the Visitor Pattern which seems to fit your use case: http://en.wikipedia.org/wiki/Visitor_pattern for inspiration of a way to organize your class hierarchy so that no casts are needed.

Another alternative might be to put the logic into the Events so that you can use the normal late binding mechanism of inheritance.

nhnb
+1  A: 

How about having another method in EventHandler:

public Event handleEvent(Event evt) {
    if (evt instanceof SpecificEvent) {
        return handleEvent((SpecificEvent)evt);
    }
    if (evt instanceof AnotherEvent) {
        return handleEvent((AnotherEvent)evt);
    }
    // code for unknown type
}

If you have a variable of type Event this will be called and will try to call one of the other defined methods. If it's a type extending Event that has no method defined, it contains code for that case. In the classes extending EventHandler the overwritten methods will be called.

I know it's not exactly pretty, but it should work.

Edit:

You could also try doing it reflectively:

public Event handleEvent(Event evt) throws InvocationTargetException{
    try {
        Method m = this.getClass().getMethod("handleEvent", evt.getClass());
        return (Event) m.invoke(this, evt);
    } catch (NoSuchMethodException nsme) {
        nsme.printStackTrace();
    } catch (IllegalAccessException iae) {
        iae.printStackTrace();
    } catch (InvocationTargetException ite) {
        ite.getCause().printStackTrace();
        throw ite;
    }
    // code for unknown type
}

This would work better if you have a lot of Event types. The only problem is that now you have to take care of the exceptions. My guess is that if NoSuchMethodException or IllegalAccessException occur, you can ignore them since it means that no method is defined for that event (or that it's inaccessible), so you need to revert to default handling of unknown types. You might not want to ignore InvocationTargetException since that means that the method was called but the method itself threw an exception, so it means there's a problem with the code in the method.

Andrei Fierbinteanu
Ick.... thats what I got away from with the giant switch statements. I'll do that if I can't come up with anything else, but I'd rather not.
Mark
I had actually been doing it with reflection before I made this post, but wanted to get away from it for performance reasons. It is what I first came up with, though.
Mark