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.
The Switch Statement code smell refers to using switch statements with a type code to get different behavior or data instead of using subclasses and polymorphism. This switch(typeCode) structure is typically spread throughout many methods. This makes the code difficult to extend, and violates the Open-Closed Principle.
Using “switch” is not an anti-pattern.
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.
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;
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With