8

I have an implementation of a State Pattern where each state handles events it gets from a event queue. Base State class therefore has a pure virtual method void handleEvent(const Event*). Events inherit base Event class but each event contains its data that can be of a different type (e.g. int, string...or whatever). handleEvent has to determine the runtime type of the received event and then perform downcast in order to extract event data. Events are dynamically created and stored in a queue (so upcasting takes place here...).

I know that downcasting is a sign of a bad design but is it possible to avoid it in this case? I am thinking of Visitor Pattern where base class State would contain virtual handlers for each event but then again downcast will need to take place in the piece of code which dequeues event from a queue and passes it to the current state. (At least in this case big switch(eventID) would be only at one place...). Is Visitor Pattern the best way (best practice) to avoid downcasting?

Here is the pseudo-code (I am passing boost::shared_ptr in this example but downcasting happens anyway):

enum EventID
{
   EVENT_1,
   EVENT_2,
   ...
};

class Event
{
   EventID id;
public:
   Event(EventID id):id(id){}
   EventID id() const {return id;}
   virtual ~Event() = 0;
};

class Event1 : public Event
{
   int n;
public:
   Event1(int n):Event(EVENT_1), n(n){}
   int getN() const {return n;}
};

class Event2 : public Event
{
   std::string s;
public:
   Event2(std::string s):Event(EVENT_2), s(s){}
   std::string getS() const {return s;}
};

typedef boost::shared_ptr<Event> EventPtr;

class State
{
   ...
public:
   ...
   virtual ~State() = 0;
   virtual void handleEvent(const EventPtr& pEvent) = 0;
};

class StateA : public State
{
   ...
public:
   void handleEvent(const EventPtr& pEvent)
   {
      switch(pEvent->id())
      {
         case EVENT_1:        
            int n = boost::static_pointer_cast<Event1>(pEvent)->getN();
            ...
            break;
         case EVENT_2:
            std::string s = boost::static_pointer_cast<Event2>(pEvent)->getS();
            ...
            break;
         ... 

      }
   }   
}

2 Answers 2

9

The typical visitor pattern performs no downcast, thanks to a double-dispatch strategy:

// Visitor.hpp
class EventBar;
class EventFoo;

class Visitor {
public:
    virtual void handle(EventBar const&) = 0;
    virtual void handle(EventFoo const&) = 0;
};

// Event.hpp
class Visitor;

class Event {
public:
    virtual void accept(Visitor&) const = 0;
};

And the implementations:

// EventBar.hpp
#include <Event.hpp>

class EventBar: public Event {
public:
    virtual void accept(Visitor& v);
};

// EventBar.cpp
#include <EventBar.hpp>
#include <Visitor.hpp>

void EventBar::accept(Visitor& v) {
    v.handle(*this);
}

The key point here is that in v.handle(*this) the static type of *this is EventBar const&, which selects the correct virtual void handle(EventBar const&) = 0 overload in Visitor.

8
  • 4
    The downcast is made in calling Event::accept. It's resolved via vtable to EventBar::accept and this is cast from Event to EventBar in the process.
    – Agent_L
    Commented May 23, 2012 at 11:49
  • So there are no other magic patterns/idioms to avoid downcasting? My only concern with Visitor is the amount of repetitive code that needs to be written. But it seems that is the price of not having downcasts. I wouldn't have this issue if I had only a single Event class and no need to store derived Event classes in the queue but that is unavoidable. Commented May 23, 2012 at 11:49
  • @BojanKomazec: at least one other way is to use boost::variant: typedef boost::variant<EventBar, EventFoo> Event;. By removing the hierarchy you remove the downcasts :) Commented May 23, 2012 at 11:52
  • @BojanKomazec Are you sure about that? Visitor is usually less code than switch and casting manually.
    – Agent_L
    Commented May 23, 2012 at 11:53
  • @Agent_L: Even though technically true, I would argue that we only worry about manual downcasts. Commented May 23, 2012 at 11:54
3

The idea of events is to pass detailed objects through generalized (and agnostic) interface. Downcast is inevitable and part of the design. Bad or good, it's disputable.

Visitor pattern only hides the casting away from you. It's still performed behind the scenes, types resolved via virtual method address.

Because your Event already has the id, it's not completely agnostic of the type, so casting is perfectly safe. Here you're watching the type personally, in visitor pattern you're making compiler take care of that.

"Whatever goes up must go down".

4
  • I disagree with the use of safe. It can work, but it is highly susceptible to errors because it is manual. Use of the language mechanisms guarantee that a) no mistargetted downcast will occur and b) all "target" types will be handled correctly (switching on ID you could forget a new ID...) Commented May 23, 2012 at 11:55
  • 1
    @MatthieuM. The less you have to done manually, the less room for error, I agree. However questioning competency of a programmer is a straight way to "You are to stupid to write in C++" argument. C and C++ are for people who know what they're doing. That's my assumption. Mistaking syntactic sugar with functionality is too common.
    – Agent_L
    Commented May 23, 2012 at 12:03
  • 1
    The less you have to done manually, the less room for error - this is my credo :) I prefer such design. And with Visitor there is no need for EventIDs. Commented May 23, 2012 at 12:25
  • @Agent_L Yeah, and compiler has less chances to make mistake when looking up the vtable than me fluffing about with enumerators :) Commented May 23, 2012 at 12:38

Not the answer you're looking for? Browse other questions tagged or ask your own question.