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;
         ... 

      }
   }   
}
jaco0646
  • 15,303
  • 7
  • 59
  • 83
Bojan Komazec
  • 9,216
  • 2
  • 41
  • 51

2 Answers2

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.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • 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 May 23 '12 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. – Bojan Komazec May 23 '12 at 11:49
  • @BojanKomazec: at least one other way is to use `boost::variant`: `typedef boost::variant Event;`. By removing the hierarchy you remove the downcasts :) – Matthieu M. May 23 '12 at 11:52
  • @BojanKomazec Are you sure about that? Visitor is usually less code than switch and casting manually. – Agent_L May 23 '12 at 11:53
  • @Agent_L: Even though technically true, I would argue that we only worry about *manual* downcasts. – Matthieu M. May 23 '12 at 11:54
  • @MatthieuM. I need to investigate `boost::variant` (and Static Visitor which is mentioned in variant's documentation page). Thanks for this hint. @Agent_L In the case of visitor, I need to implement handler for each event in each state. With downcasting, each state has a single handler with a big `switch` but I can fall to `default` case for events not handled in that state. But yeah, amount of code is probably not much different. – Bojan Komazec May 23 '12 at 12:04
  • @BojanKomazec You can handle defaulting by moving the core Visitor functionality to base classes. OFC in worst case it's one base class for every event type, but it's still better than duplicating code. – Agent_L May 23 '12 at 12:07
  • @MatthieuM. `boost::variant` rocks! That will be my definite choice I think. – Bojan Komazec May 24 '12 at 10:46
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".

Agent_L
  • 4,960
  • 28
  • 30
  • 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...) – Matthieu M. May 23 '12 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 May 23 '12 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 `EventID`s. – Bojan Komazec May 23 '12 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 :) – Bojan Komazec May 23 '12 at 12:38