Should I avoid downcasting by any means when using factory pattern?

Question

I'm working on a server project that implements a proprietary protocol. The server is implement with factory pattern in C++, and we're now facing the problem of downcasting.

The protocol I'm working on is designed for automatic control over slow networks, such as RS485, ZigBee, narrow-band PLC, etc. We designed the main server with factory pattern. When a new frame is received, we first identify the associated device type of that frame, calling factory method to generate a new "parser" instance, and dispatch the frame to the parser instance.

Our proprietary protocol is implement in pure binary, every information we may need is recorded in the frame itself, so the base interface can be defined as simple as possible. We'd also implement auto-registration approach for our factory (detailed code related to std::map operation is omitted here):

// This is our "interface" base-class
class parser
{
public:
    virtual int parse(unsigned char *) = 0;
    virtual ~parser() { }
};

// The next two classes are used for factory pattern
class instance_generator
{
public:
    virtual parser *generate() = 0;
};

class parser_factory
{
private:
    static std::map<int,instance_generator*> classDB;
public:
    static void add(int id, instance_generator &genrator);
    parser *get_instance(int id);
};

// the two template classes are implementations of "auto-regisrtation"
template <class G, int ID> class real_generator : public instance_generator
{
public:
    real_generator()    {   parser_factory::add(ID,this);   }
    parser *generate()  {   return new G;   }
};

template <class T, int N> class auto_reg : virtual public parser
{
private:
    static real_generator<T,N> instance;
public:
    auto_reg() { instance; }
};
template <class T, int N> parser_generator<T,N> auto_reg<T,N>::instance;


// And these are real parser implementations for each device type
class light_sensor : public auto_reg<light_sensor,1>
{
public:
    int parse(unsigned char *str)
    {
        /* do something here */
    }
};

class power_breaker : public auto_reg<power_breaker,2>
{
public:
    int parse(unsigned char *str)
    {
        /* do something here */
    }
};

/* other device parser */

This factory pattern worked very well, and it's easy to expend new device types.

However, recently we're trying to interface with an existed control system that provide similiar functionallity. The target system is quite old, and it only provides an ASCII-based, AT-command-like serial interface. We've managed to solve the communication issue with PTY, but now the problem to be solved is parser implementation.

The command interface of the target system is quite limited. I can't just wait and listen for what's incoming, I've to poll for the state, and I have to poll twice -- first poll for header, and second poll for payload -- to get a full command. That's a problem for our implementation, cause I've to pass TWO frames into the parser instance so it could work:

class legacy_parser : virtual public parser
{
public:
    legacy_parser() { }
    int parse(unsigned char *str)
    {
        /* CAN NOT DO ANYTHING WITHOUT COMPLETE FRAMES */
    }
    virtual int parse(unsigned char *header, unsigned char *payload) = 0;
};

class legacy_IR_sensor : 
    public legacy_parser,
    public auto_reg<legacy_IR_sensor,20>
{
public:
    legacy_IR_sensor(){ }
    int parse(unsigned char *header, unsigned char *payload)
    {
        /* Now we can finally parse the complete frame */
    }
};

In other words, we'll need to call a method of the derived class, and the method is not defined in the base class. And we're using the factory pattern to generate the instance of derived class.

Now we have several chooice:

1. Simply concatenate two string into one does not work. Both string contains some device-specified informations, and they shall be parsed separately. If we take this approach, we shall perform some "pre-parsing" out of parser instance before we can concatenate the string. And we don't think it's a good idea.

2. Downcast the return of parser_factory::get_instance() to legacy_parser.

3. Create another independent factory, which only contains classes derived from legacy_parser.

4. Change the definition of instance_generator and parser_factory so that they may also generate (legacy_parser*), while keep all existed code unaffected :

   class instance_generator
   {
   public:
       virtual parser *generate() = 0;
       virtual legacy_parser *generate_legacy() { return NULL; }
   };
   
   class extended_parser_factory : public parser_factory
   {
   public:
       legacy_parser *get_legacy_instance(int id);
   };
   

5. Implemet "smart pointer" with Visitor pattern to handle instances derived from legacy_parser:

   class smart_ptr
   {
   public:
       virtual void set(parser *p) = 0;
       virtual void set(legacy_parser *p) = 0;
   };
   
   class parser
   {
   public:
       parser() { }
       virtual int parse(unsigned char *) = 0;
       virtual void copy_ptr(smart_ptr &sp)    // implement "Visitor" pattern
       {
           sp.set(this);
       }
       virtual ~parser() { }
   };
   
   class legacy_parser : virtual public parser
   {
   public:
       legacy_parser() { }
       void copy_ptr(smart_ptr &sp)    // implement "Visitor" pattern
       {
           sp.set(this);
       }
       int parse(unsigned char *str)
       {
           /* CAN NOT DO ANYTHING WITHOUT COMPLETE FRAMES */
       }
       virtual int parse(unsigned char *header, unsigned char *payload) = 0;
   };
   
   class legacy_ptr : public smart_ptr
   {
   private:
       parser *miss;
       legacy_parser *hit;
   public:
       legacy_ptr& operator=(parser *rhv)
       {
           rhv->copy_ptr(*this);
           return *this;
       }
       void set(parser* ptr)
       {
           miss=ptr;
           /* ERROR! Do some log or throw exception */
       }
       void set(legacy_parser *ptr)
       {
           hit = ptr;
       }
       legacy_parser& operator*()
       {
           return *hit;
       }
       ~legacy_ptr()
       {
           if(miss) {
               delete miss;
           }
           if(hit) {
               delete hit;
           }
       }
   };
   

It's obvious that Downcasting with dynamic_cast<> is the easiest approach for us, but none of us likes this idea, because we all feel it's "evil" to downcast something. However, no one can exactly explain why it's "evil."

Before we've made a decision, I'd wish to hear more comments about these options.

Answer 1

http://en.wikipedia.org/wiki/Circle-ellipse_problem is your 1st example of evil. If you see that you can do something with violation of base principles, then you should invent another wheel or try another hat: http://en.wikipedia.org/wiki/Six_Thinking_Hats