2010-08-04 1 views
6

Ich habe so etwas wie die folgenden im HeaderDesign-Muster switch-Anweisung

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

und wird verwendet als

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

Dies scheint, wie die Art von Situation, Refactoring, die ziemlich häufig sein würde und gut geeignet zum Refactoring. Was wäre der beste Weg, Design-Patterns (oder ein grundlegendes C++ - Sprachen-Feature-Redesign) anzuwenden, um diesen Code umzuformen?

Ich habe gelesen, dass das Befehlsmuster häufig verwendet wird, um Schalteranweisungen zu refaktorieren, aber das scheint nur anwendbar, wenn man zwischen Algorithmen auswählt, um eine Aufgabe zu erledigen. Ist dies ein Ort, wo das Fabrik- oder abstrakte Fabrikmuster anwendbar ist (ich kenne mich auch nicht sehr gut aus)? Doppelter Versand?

Ich habe versucht, so viel belanglosen Kontext wie möglich wegzulassen, aber wenn ich etwas wichtiges verpasste, lass es mich wissen und ich werde es bearbeiten, um es aufzunehmen. Außerdem konnte ich nichts Ähnliches finden, aber wenn das ein Duplikat ist, leite mich einfach zu der entsprechenden SO Frage um.

+0

Besucher Muster ist gut für den Austausch von Schaltern wie diesem. – neoneye

+0

@neoneye: Das Besuchermuster implementiert Double-Dispatch basierend auf den dynamischen Typen von zwei vorhandenen Objekten. In diesem Fall müssen wir bestimmen, welcher Objekttyp erstellt werden soll. –

Antwort

2

Pull-Typen und type_ out von MsgBase, sie gehören nicht dorthin.

Wenn Sie sich etwas ganz Besonderes wünschen, registrieren Sie alle abgeleiteten Typen zusammen mit dem Token (z. B. "type"), das die Fabrik verwenden wird, um zu wissen, was zu tun ist. Dann sucht die Factory in der Tabelle nach dem Deserialisieren dieses Tokens und erstellt die richtige Nachricht.

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

usw. Das Erstellen statische Methode eine neue DerivedMessage zuordnet und deserialisiert es, schreibt die Serialize-Methode das Token (in diesem Fall Hash("DerivedMessage")) und dann serialisiert selbst. Einer von ihnen sollte wahrscheinlich isRegistered testen, so dass er vom Linker nicht vollständig entfernt wird.

(Bemerkenswerterweise benötigt diese Methode keine Aufzählung oder andere "statische Liste von allem, was jemals existieren kann." Zu dieser Zeit kann ich nicht an eine andere Methode denken, die zirkuläre Referenzen in gewissem Maße nicht benötigt.)

5

Sie könnten ein Factory Method Muster verwenden, das die korrekte Implementierung der Basisklasse (abgeleitete Klasse) basierend auf dem Wert erstellt, den Sie aus dem Stream sehen.

+4

Die Factory-Methode, wie von der von Ihnen bereitgestellten Wikipedia-Verknüpfung erklärt, hat immer noch eine 'switch'-Anweisung, sie ist nur in der Factory-Methode versteckt. –

+0

Ja, das stimmt. Der entscheidende Punkt ist, dass es versteckt ist. Irgendwann wirst du entscheiden müssen, was du erschaffen willst. –

+1

Es beseitigt auch die Notwendigkeit, dass die Basisklasse irgendwelche Kenntnisse über die abgeleiteten Implementierungen besitzt (indem sie die Erstellung außerhalb der Klassenstruktur delegiert) und im Allgemeinen ein gutes Design nach Schnittstellengrundsätzen durchsetzt (da es einen Zeiger auf die Basisklasse zurückgeben sollte). –

1

Es ist im Allgemeinen eine schlechte Idee für eine Basisklasse, Wissen über abgeleitete Klassen zu haben, so dass ein Redesign definitiv in Ordnung ist. Ein Fabrikmuster ist wahrscheinlich, was Sie hier wollen, wie Sie bereits bemerkt haben.

3

Der Schalter ist nicht alles schlecht. Es ist eine Möglichkeit, das Fabrikmuster zu implementieren. Es ist leicht zu testen, es macht es einfach, die gesamte Palette der verfügbaren Objekte zu verstehen, und es ist gut für Coverage-Tests.

Eine weitere Technik besteht darin, eine Zuordnung zwischen Ihren Enum-Typen und Factories zu erstellen, um die spezifischen Objekte aus dem Datenstrom zu erstellen. Dies verwandelt den Kompilierungszeitschalter in eine Laufzeitnachschlage. Das Mapping kann zur Laufzeit erstellt werden, sodass neue Typen hinzugefügt werden können, ohne alles neu zu kompilieren.

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