Design pattern to refactor switch statement

Question

I have something like the following in the header

class MsgBase
{
  public:
    unsigned int getMsgType() const { return type_; }
    ...
  private:
    enum Types { MSG_DERIVED_1, MSG_DERIVED_2, ... MSG_DERIVED_N };
    unsigned int type_;
    ...
};

class MsgDerived1 : public MsgBase { ... };
class MsgDerived2 : public MsgBase { ... };
...
class MsgDerivedN : public MsgBase { ... };

and is used as

MsgBase msgHeader;
// peeks into the input stream to grab the
// base class that has the derived message type
// non-destructively
inputStream.deserializePeek( msgHeader ); 
unsigned int msgType = msgHeader.getMsgType();

MsgDerived1 msgDerived1;
MsgDerived2 msgDerived2;
...
MsgDerivedN msgDerivedN;

switch( msgType )
{
  case MSG_DERIVED_1:
    // fills out msgDerived1 from the inputStream
    // destructively
    inputStream.deserialize( msgDerived1 );
    /* do MsgDerived1 processing */
    break;
  case MSG_DERIVED_2:
    inputStream.deserialize( msgDerived2 );
    /* do MsgDerived1 processing */
    break;
  ...
  case MSG_DERIVED_N:
    inputStream.deserialize( msgDerivedN );
    /* do MsgDerived1 processing */
    break;
}

This seems like the type of situation which would be fairly common and well suited to refactoring. What would be the best way to apply design patterns (or basic C++ language feature redesign) to refactor this code?

I have read that the Command pattern is commonly used to refactor switch statements but that seems only applicable when choosing between algorithms to do a task. Is this a place where the factory or abstract factory pattern is applicable (I am not very familiar with either)? Double dispatch?

I've tried to leave out as much inconsequential context as possible but if I missed something important just let me know and I'll edit to include it. Also, I could not find anything similar but if this is a duplicate just redirect me to the appropriate SO question.

Answer 1

You could use a Factory Method pattern that creates the correct implementation of the base class (derived class) based on the value you peek from the stream.

Answer 2

It's generally a bad idea for a base class to have knowledge about derived classes, so a redesign is definitely in order. A factory pattern is probably what you want here as you already noted.

Answer 3

The switch isn't all bad. It's one way to implement the factory pattern. It's easily testable, it makes it easy to understand the entire range of available objects, and it's good for coverage testing.

Another technique is to build a mapping between your enum types and factories to make the specific objects from the data stream. This turns the compile-time switch into a run-time lookup. The mapping can be built at run-time, making it possible to add new types without recompiling everything.

// You'll have multiple Factories, all using this signature.
typedef MsgBase *(*Factory)(StreamType &);

// For example:
MsgBase *CreateDerived1(StreamType &inputStream) {
    MsgDerived1 *ptr = new MsgDerived1;
    inputStream.deserialize(ptr);
    return ptr;
}

std::map<Types, Factory> knownTypes;
knownTypes[MSG_DERIVED_1] = CreateDerived1;

// Then, given the type, you can instantiate the correct object:
MsgBase *object = (*knownTypes[type])(inputStream);

...

delete object;

Answer 4

Pull Types and type_ out of MsgBase, they don't belong there.

If you want to get totally fancy, register all of your derived types with the factory along with the token (e.g. 'type') that the factory will use to know what to make. Then, the factory looks up that token on deserialize in its table, and creates the right message.

class DerivedMessage : public Message
{
public:
   static Message* Create(Stream&);
   bool Serialize(Stream&);

private:
   static bool isRegistered;
};

// sure, turn this into a macro, use a singleton, whatever you like
bool DerivedMessage::isRegistered =
      g_messageFactory.Register(Hash("DerivedMessage"), DerivedMessage::Create);

etc. The Create static method allocates a new DerivedMessage and deserializes it, the Serialize method writes the token (in this case, Hash("DerivedMessage")) and then serializes itself. One of them should probably test isRegistered so that it doesn't get dead stripped by the linker.

(Notably, this method doesn't require an enum or other "static list of everything that can ever exist". At this time I can't think of another method that doesn't require circular references to some degree.)