Ist es ein Codegeruch, generische Objekte in einem Container zu speichern und dann das Objekt abzurufen und die Objekte aus dem Container zu entfernen?

Zum Beispiel habe ich ein Spiel, das einige Werkzeuge hat, um die Fähigkeit des Spielers zu erhöhen:

Tool.h

class Tool{
public:
    std::string name;
};

Und einige Tools:

Schwert.h

class Sword : public Tool{
public:
    Sword(){
        this->name="Sword";
    }
    int attack;
};

Shield.h

class Shield : public Tool{
public:
    Shield(){
        this->name="Shield";
    }
    int defense;
};

MagicCloth.h

class MagicCloth : public Tool{
public:
    MagicCloth(){
        this->name="MagicCloth";
    }
    int attack;
    int defense;
};

Und dann kann ein Spieler einige Werkzeuge für den Angriff halten:

class Player{
public:
    int attack;
    int defense;
    vector<Tool*> tools;
    void attack(){
        //original attack and defense
        int currentAttack=this->attack;
        int currentDefense=this->defense;
        //calculate attack and defense affected by tools
        for(Tool* tool : tools){
            if(tool->name=="Sword"){
                Sword* sword=(Sword*)tool;
                currentAttack+=sword->attack;
            }else if(tool->name=="Shield"){
                Shield* shield=(Shield*)tool;
                currentDefense+=shield->defense;
            }else if(tool->name=="MagicCloth"){
                MagicCloth* magicCloth=(MagicCloth*)tool;
                currentAttack+=magicCloth->attack;
                currentDefense+=magicCloth->shield;
            }
        }
        //some other functions to start attack
    }
};

Ich denke, es ist schwierig, es durch if-elsevirtuelle Methoden in den Werkzeugen zu ersetzen , da jedes Werkzeug unterschiedliche Eigenschaften hat und sich auf den Angriff und die Verteidigung des Spielers auswirkt, wofür die Aktualisierung des Angriffs und der Verteidigung des Spielers im Player-Objekt durchgeführt werden muss.

Aber ich war mit diesem Entwurf nicht zufrieden, da er mit einer langen if-elseAussage downcasting enthält . Muss dieses Design "korrigiert" werden? Wenn ja, was kann ich tun, um das Problem zu beheben?

Ja, es ist ein Codegeruch (in vielen Fällen).

Ich denke, es ist schwierig, if-else in Tools durch virtuelle Methoden zu ersetzen

In Ihrem Beispiel ist es ganz einfach, das if / else durch virtuelle Methoden zu ersetzen:

class Tool{
 public:
   virtual int GetAttack() const=0;
   virtual int GetDefense() const=0;
};

class Sword : public Tool{
    // ...
 public:
   virtual int GetAttack() const {return attack;}
   virtual int GetDefense() const{return 0;}
};

Jetzt brauchen Sie Ihren ifBlock nicht mehr , der Anrufer kann ihn einfach so verwenden

       currentAttack+=tool->GetAttack();
       currentDefense+=tool->GetDefense();

Natürlich ist eine solche Lösung für kompliziertere Situationen nicht immer so offensichtlich (aber dennoch fast jederzeit möglich). Wenn Sie jedoch nicht wissen, wie Sie den Fall mit virtuellen Methoden lösen können, können Sie hier unter "Programmierer" (oder, wenn es sprach- oder implementierungsspezifisch wird, unter Stackoverflow) erneut eine Frage stellen.

Das Hauptproblem bei Ihrem Code besteht darin, dass Sie bei jeder Einführung eines neuen Artikels nicht nur den Code des Artikels schreiben und aktualisieren müssen, sondern auch Ihren Player (oder den Ort, an dem der Artikel verwendet wird) modifizieren müssen, was das Ganze zu einem macht viel komplizierter.

Als allgemeine Faustregel halte ich es immer für faul, wenn man sich nicht auf normale Unterklassen / Vererbung verlassen kann und das Upcasting selbst durchführen muss.

Ich könnte mir zwei mögliche Ansätze vorstellen, um das Ganze flexibler zu machen:

• Verschieben Sie, wie bereits erwähnt, die Elemente attackund defensein die Basisklasse und initialisieren Sie sie dort 0. Dies kann auch als Kontrolle dienen, ob Sie den Gegenstand tatsächlich für einen Angriff schwingen oder ihn zum Blockieren von Angriffen verwenden können.

• Erstellen Sie eine Art Rückruf- / Ereignissystem. Hierfür gibt es verschiedene mögliche Ansätze.

  Wie wäre es einfach zu halten?

  • Sie können Basisklassenmitglieder wie virtual void onEquip(Owner*) {}und erstellen virtual void onUnequip(Owner*) {}.
  • Ihre Überladungen würden aufgerufen und die Statistiken beim (Un-) Ausrüsten des Gegenstandes modifizieren, zB virtual void onEquip(Owner *o) { o->modifyStat("attack", attackValue); }und virtual void onUnequip(Owner *o) { o->modifyStat("attack", -attackValue); }.
  • Auf die Statistiken kann auf dynamische Weise zugegriffen werden, z. B. mithilfe einer kurzen Zeichenfolge oder einer Konstante als Schlüssel, sodass Sie sogar neue ausrüstungsspezifische Werte oder Boni eingeben können, die Sie nicht unbedingt in Ihrem Spieler oder "Besitzer" speziell behandeln müssen.

  • Verglichen damit, nur die Angriffs- / Verteidigungswerte rechtzeitig anzufordern, wird das Ganze dadurch nicht nur dynamischer, es erspart Ihnen auch unnötige Anrufe und Sie können sogar Elemente erstellen, die Ihren Charakter dauerhaft beeinflussen.

    Stellen Sie sich zum Beispiel einen verfluchten Ring vor, der nur einen versteckten Wert setzt, sobald er ausgerüstet ist, und Ihren Charakter dauerhaft als verflucht markiert.

Obwohl @DocBrown eine gute Antwort gegeben hat, geht es nicht weit genug, imho. Bevor Sie mit der Bewertung der Antworten beginnen, sollten Sie Ihre Anforderungen bewerten. Was brauchst du wirklich ?

Im Folgenden zeige ich zwei mögliche Lösungen, die unterschiedliche Vorteile für unterschiedliche Bedürfnisse bieten.

Das erste ist sehr simpel und speziell auf das zugeschnitten, was Sie gezeigt haben:

class Tool {
    public:
        std::string name;
        int attack;
        int defense;
}

public void attack() {
    int attack = this->attack;
    int defense = this->defense;
    for (Tool* tool : tools){
        attack += tool->attack;
        defense += tool->defense;
    }
}

Dies ermöglicht eine sehr einfache Serialisierung / Deserialisierung von Tools (z. B. zum Speichern oder Vernetzen) und erfordert überhaupt keinen virtuellen Versand. Wenn Ihr Code alles ist, was Sie gezeigt haben, und Sie nicht erwarten, dass er sich wesentlich weiterentwickelt, außer dass es mehr verschiedene Tools mit unterschiedlichen Namen und Statistiken gibt, nur in unterschiedlichen Mengen, dann ist dies der richtige Weg.

@DocBrown hat eine Lösung angeboten, die sich immer noch auf den virtuellen Versand stützt. Dies kann von Vorteil sein, wenn Sie die Tools auf Teile Ihres Codes spezialisieren, die nicht angezeigt wurden. Wenn Sie jedoch wirklich ein anderes Verhalten benötigen oder ändern möchten, würde ich die folgende Lösung vorschlagen:

Zusammensetzung über Vererbung

Was ist, wenn Sie später ein Tool benötigen, das die Beweglichkeit ändert ? Oder Geschwindigkeit laufen ? Mir scheint, Sie machen ein Rollenspiel. Eine Sache, die für RPGs wichtig ist, ist die Möglichkeit zur Verlängerung . Die bisher gezeigten Lösungen bieten das nicht. Sie müssten die ToolKlasse jedes Mal ändern und neue virtuelle Methoden hinzufügen, wenn Sie ein neues Attribut benötigen.

Die zweite Lösung, die ich zeige, ist die, auf die ich bereits in einem Kommentar hingewiesen habe. Sie verwendet Komposition anstelle von Vererbung und folgt dem Prinzip "Zur Änderung geschlossen, zur Erweiterung offen *" wird vertraut aussehen (ich stelle mir die Komposition gerne als den kleineren Bruder von ES vor).

Beachten Sie, dass das, was ich unten zeige, in Sprachen mit Laufzeitinformationen wie Java oder C # deutlich eleganter ist. Daher muss der C ++ - Code, den ich zeige, eine "Buchhaltung" enthalten, die einfach notwendig ist, damit die Komposition genau hier funktioniert. Vielleicht kann jemand mit mehr C ++ - Erfahrung einen noch besseren Ansatz vorschlagen.

Zunächst schauen wir uns noch einmal die Seite des Anrufers an . In Ihrem Beispiel attackinteressieren Sie als Aufrufer in der Methode überhaupt nicht für Tools. Was Sie interessiert, sind zwei Eigenschaften - Angriffs- und Verteidigungspunkte. Es ist Ihnen eigentlich egal, woher diese kommen, und Sie interessieren sich nicht für andere Eigenschaften (z. B. Laufgeschwindigkeit, Beweglichkeit).

Als erstes führen wir eine neue Klasse ein

class Component {
    public:
        // we need this, in Java we'd simply use getClass()
        virtual std::string type() const = 0;
};

Und dann erstellen wir unsere ersten beiden Komponenten

class Attack : public Component {
    public:
        std::string type() const override { return std::string("mygame::components::Attack"); }
        int attackValue = 0;
};

class Defense : public Component {
    public:
      std::string type() const override { return std::string("mygame::components::Defense"); }
      int defenseValue = 0;
};

Anschließend lassen wir ein Tool eine Reihe von Eigenschaften enthalten und die Eigenschaften für andere abfragbar machen.

class Tool {
private:
    std::map<std::string, Component*> components;

public:
    /** Adds a component to the tool */
    void addComponent(Component* component) { 
        components[component->type()] = component;
    };
    /** Removes a component from the tool */
    void removeComponent(Component* component) { components.erase(component->type()); };
    /** Return the component with the given type */
    Component* getComponentByType(std::string type) { 
        std::map<std::string, Component*>::iterator it = components.find(type);
        if (it != components.end()) { return it->second; }
        return nullptr;
    };
    /** Check wether a tol has a given component */
    bool hasComponent(std::string type) {
        std::map<std::string, Component*>::iterator it = components.find(type);
        return it != components.end();
    }
};

Beachten Sie, dass in diesem Beispiel nur eine Komponente für jeden Typ unterstützt wird. Dies erleichtert die Arbeit. Theoretisch könnte man auch mehrere Komponenten desselben Typs zulassen, aber das wird sehr schnell hässlich. Ein wichtiger Aspekt: Toolist jetzt für Änderungen geschlossen - wir werden nie wieder die Quelle berühren Tool- aber offen für Erweiterungen - wir können das Verhalten eines Tools erweitern, indem wir andere Dinge modifizieren und einfach andere Komponenten in das Tool übertragen.

Jetzt brauchen wir eine Möglichkeit, Werkzeuge nach Komponententypen abzurufen. Sie können immer noch einen Vektor für Werkzeuge verwenden, genau wie in Ihrem Codebeispiel:

class Player {
    private:
        int attack = 0; 
        int defense = 0;
        int walkSpeed;
    public:
        std::vector<Tool*> tools;
        std::vector<Tool*> getToolsByComponentType(std::string type) {
            std::vector<Tool*> retVal;
            for (Tool* tool : tools) {
                if (tool->hasComponent(type)) { 
                    retVal.push_back(tool); 
                }
            }
            return retVal;
        }

        void doAttack() {
            int attackValue = this->attack;
            int defenseValue = this->defense;

            for (Tool* tool : this->getToolsByComponentType(std::string("mygame::components::Attack"))) {
                Attack* component = (Attack*) tool->getComponentByType(std::string("mygame::components::Attack"));
                attackValue += component->attackValue;
            }
            for (Tool* tool : this->getToolsByComponentType(std::string("mygame::components::Defense"))) {
                Defense* component = (Defense*)tool->getComponentByType(std::string("mygame::components::Defense"));
                defenseValue += component->defenseValue;
            }
            std::cout << "Attack with strength " << attackValue << "! Defend with strenght " << defenseValue << "!";
        }
};

Sie können dies auch in Ihre eigene InventoryKlasse umgestalten und Nachschlagetabellen speichern, die das Abrufen von Tools nach Komponententyp erheblich vereinfachen und das wiederholte Durchlaufen der gesamten Sammlung vermeiden.

Welche Vorteile hat dieser Ansatz? In attack, Sie verarbeiten Tools , die zwei Komponenten haben - Sie kümmern sich nicht um irgendetwas anderes.

Stellen wir uns vor, Sie haben eine walkToMethode, und jetzt entscheiden Sie, dass es eine gute Idee ist, wenn ein Werkzeug die Fähigkeit erhält, Ihre Gehgeschwindigkeit zu ändern. Kein Problem!

Erstellen Sie zuerst das neue Component:

class WalkSpeed : public Component {
public:
    std::string type() const override { return std::string("mygame::components::WalkSpeed"); }
    int speedBonus;
};

Anschließend fügen Sie dem Tool, mit dem Sie die Weckgeschwindigkeit erhöhen möchten, einfach eine Instanz dieser Komponente hinzu und ändern die WalkToMethode, um die soeben erstellte Komponente zu verarbeiten:

void walkTo() {
    int walkSpeed = this->walkSpeed;

    for (Tool* tool : this->getToolsByComponentType(std::string("mygame::components:WalkSpeed"))) {
        WalkSpeed* component = (WalkSpeed*)tool->getComponentByType(std::string("mygame::components::Defense"));
        walkSpeed += component->speedBonus;
        std::cout << "Walk with " << walkSpeed << std::endl;
    }
}

Beachten Sie, dass wir unseren Tools ein gewisses Verhalten hinzugefügt haben, ohne die Tools-Klasse zu ändern.

Sie können (und sollten) die Zeichenfolgen in ein Makro oder eine statische const-Variable verschieben, damit Sie sie nicht immer wieder eingeben müssen.

Wenn Sie diesen Ansatz weiter verfolgen, z. B. Komponenten erstellen, die dem Spieler hinzugefügt werden können, und eine CombatKomponente erstellen, die den Spieler als kampffähig kennzeichnet, können Sie auch die attackMethode loswerden und diese behandeln lassen durch die Komponente oder an anderer Stelle verarbeitet werden.

Der Vorteil, dass der Spieler auch Komponenten erhalten kann, ist, dass Sie dann nicht einmal den Spieler wechseln müssen, um ihm ein anderes Verhalten zu verleihen. In meinem Beispiel könnten Sie eine MovableKomponente erstellen , so dass Sie die walkToMethode nicht auf dem Player implementieren müssen , um ihn in Bewegung zu setzen. Sie erstellen einfach die Komponente, hängen sie an den Player an und lassen sie von einer anderen Person verarbeiten.

Ein vollständiges Beispiel finden Sie in dieser Übersicht: https://gist.github.com/NetzwergX/3a29e1b106c6bb9c7308e89dd715ee20

Diese Lösung ist offensichtlich etwas komplexer als die anderen, die veröffentlicht wurden. Aber je nachdem, wie flexibel Sie sein möchten, wie weit Sie gehen möchten, kann dies ein sehr wirkungsvoller Ansatz sein.

Bearbeiten

Einige andere Antworten schlagen eine direkte Vererbung vor (Schwerter verlängern lassen, Schild verlängern lassen). Ich denke nicht, dass dies ein Szenario ist, in dem die Vererbung sehr gut funktioniert. Was ist, wenn Sie entscheiden, dass das Blockieren mit einem Schild auf eine bestimmte Weise auch den Angreifer schädigen kann? Mit meiner Lösung können Sie einfach eine Angriffskomponente zu einem Schild hinzufügen und dies ohne Änderungen an Ihrem Code feststellen. Mit der Vererbung hätten Sie ein Problem. Gegenstände / Werkzeuge in RPGs sind von Anfang an Hauptkandidaten für die Komposition oder sogar für die direkte Verwendung von Entitätssystemen.

Im Allgemeinen ifist es ein Zeichen dafür, dass etwas stinkendes vor sich geht , wenn Sie jemals die Notwendigkeit haben, in einer beliebigen OOP-Sprache (in Kombination mit der Anforderung des Typs einer Instanz) zu verwenden. Zumindest sollten Sie sich Ihre Modelle genauer ansehen.

Ich würde Ihre Domain anders modellieren.

Für Ihren Anwendungsfall Toolhat a ein AttackBonusund ein DefenseBonus- was beides sein könnte, 0falls es für Kämpfe wie Federn oder ähnliches unbrauchbar ist.

Für einen Angriff hast du dein baserate+ bonusvon der Waffe benutzt. Das gleiche gilt für die Verteidigung baserate+ bonus.

Infolgedessen Toolmüssen Sie eine virtualMethode zur Berechnung der Angriffs- / Verteidigungsboni haben.

tl; dr

Mit einem besseren Design könnten Sie hacky ifs vermeiden .

Wie geschrieben "riecht" es, aber das könnten nur die Beispiele sein, die Sie gegeben haben. Das Speichern von Daten in generischen Objektcontainern und das anschließende Umwandeln, um Zugriff auf die Daten zu erhalten, ist kein automatischer Codegeruch. Sie werden sehen, dass es in vielen Situationen verwendet wird. Wenn Sie es jedoch verwenden, sollten Sie wissen, was Sie tun, wie Sie es tun und warum. Wenn ich mir das Beispiel ansehe, wird anhand von auf Zeichenfolgen basierenden Vergleichen festgestellt, welches Objekt das ist, was meinen persönlichen Geruchsmesser auslöst. Es deutet darauf hin, dass Sie nicht ganz sicher sind, was Sie hier tun (was in Ordnung ist, da Sie die Weisheit hatten, hierher zu kommen, um Programmierern zu helfen mich raus! ").

Das grundlegende Problem beim Umwandeln von Daten aus generischen Containern wie diesem ist, dass der Produzent der Daten und der Konsument der Daten zusammenarbeiten müssen, aber es ist möglicherweise nicht offensichtlich, dass dies auf den ersten Blick der Fall ist. In jedem Beispiel dieses Musters, ob es stinkt oder nicht, ist dies das grundlegende Problem. Es ist sehr wahrscheinlich, dass der nächste Entwickler nicht weiß, dass Sie dieses Muster ausführen, und es versehentlich abbricht. Wenn Sie dieses Muster verwenden, müssen Sie darauf achten, dem nächsten Entwickler zu helfen. Sie müssen es ihm leichter machen, den Code nicht ungewollt zu knacken, da er möglicherweise nicht genau weiß, dass er existiert.

Was wäre zum Beispiel, wenn ich einen Player kopieren wollte? Wenn ich mir nur den Inhalt des Player-Objekts ansehe, sieht es ziemlich einfach aus. Ich habe zu kopieren nur die attack, defenseund toolsVariablen. Einfach wie Torte! Nun, ich werde schnell herausfinden, dass Ihre Verwendung von Zeigern es etwas schwieriger macht (irgendwann lohnt es sich, sich intelligente Zeiger anzusehen, aber das ist ein anderes Thema). Das ist leicht zu lösen. Ich erstelle einfach neue Kopien von jedem Tool und füge diese in meine neue toolsListe ein. Immerhin Toolist eine wirklich einfache Klasse mit nur einem Mitglied. Also erstelle ich ein paar Kopien, einschließlich einer Kopie der Sword, aber ich wusste nicht, dass es ein Schwert ist, also habe ich nur die kopiert name. Später betrachtet die attack()Funktion den Namen, stellt fest, dass es sich um ein "Schwert" handelt, wirft es und es passieren schlimme Dinge!

Wir können diesen Fall mit einem anderen Fall in der Socket-Programmierung vergleichen, der dasselbe Muster verwendet. Ich kann eine UNIX-Socket-Funktion wie folgt einrichten:

int sockfd = socket(AF_INET, SOCK_STREAM, 0);
sockaddr_in serv_addr;
serv_addr.sin_family = AF_INET;
serv_addr.sin_port = htons(portno);
serv_addr.sin_addr.s_addr = INADDR_ANY;
bind(sockfd, (struct sockaddr *) &serv_addr, sizeof(serv_addr));

Warum ist das das gleiche Muster? Weil bindes kein akzeptiert sockaddr_in*, akzeptiert es ein generischeres sockaddr*. Wenn Sie sich die Definitionen für diese Klassen ansehen, sehen wir, dass sockaddrnur ein Mitglied der Familie, der wir sin_family* zugewiesen haben, vorhanden ist . Die Familie sagt, zu welchem ​​Subtyp du die Casts machen solltest sockaddr. AF_INETsagt dir, dass die Adressstruktur tatsächlich a ist sockaddr_in. In diesem AF_INET6Fall wäre die Adresse eine Adresse sockaddr_in6mit größeren Feldern, um die größeren IPv6-Adressen zu unterstützen.

Dies ist identisch mit Ihrem ToolBeispiel, außer dass eine Ganzzahl verwendet wird, um die Familie anzugeben, und nicht ein std::string. Ich behaupte jedoch, dass es nicht riecht und versuche es aus anderen Gründen als "es ist eine Standardmethode, Sockets zu machen, damit es nicht" riecht ". Offensichtlich ist es dasselbe Muster, das ist Warum ich behaupte, dass das Speichern von Daten in generischen Objekten und das Umwandeln von Daten nicht automatisch nach Code riecht, aber es gibt einige Unterschiede in der Vorgehensweise, die die Sicherheit erhöhen.

Bei Verwendung dieses Musters besteht die wichtigste Information darin, die Übermittlung von Informationen über die Unterklasse vom Produzenten zum Verbraucher zu erfassen. Dies tun Sie mit dem nameFeld und UNIX-Sockets mit ihrem sin_familyFeld. Dieses Feld ist die Information, die der Verbraucher benötigt, um zu verstehen, was der Produzent tatsächlich erstellt hat. In allen Fällen dieses Musters sollte es sich um eine Aufzählung handeln (oder zumindest um eine Ganzzahl, die wie eine Aufzählung wirkt). Warum? Überlegen Sie, was Ihr Verbraucher mit den Informationen tun wird. Sie müssen eine große ifErklärung oder eine Erklärung geschrieben habenswitchAnweisung, wie Sie es getan haben, wo sie den richtigen Untertyp bestimmen, ihn umwandeln und die Daten verwenden. Per Definition kann es nur eine kleine Anzahl dieser Typen geben. Sie können es in einer Zeichenfolge speichern, wie Sie es getan haben, aber das hat zahlreiche Nachteile:

• Langsam - std::stringmuss normalerweise einen dynamischen Speicher ausführen, um die Zeichenfolge beizubehalten. Sie müssen auch einen Volltextvergleich durchführen, um den Namen jedes Mal abzugleichen, wenn Sie herausfinden möchten, welche Unterklasse Sie haben.
• Zu vielseitig - Es gibt etwas zu sagen, um sich selbst Einschränkungen aufzuerlegen, wenn Sie etwas äußerst Gefährliches tun. Ich hatte Systeme wie dieses, die nach einem Teilstring suchten, um festzustellen, auf welche Art von Objekt sie schauten. Dies funktionierte hervorragend, bis der Name eines Objekts versehentlich diese Teilzeichenfolge enthielt und einen fürchterlich kryptischen Fehler verursachte. Da wir, wie oben erwähnt, nur eine kleine Anzahl von Fällen benötigen, gibt es keinen Grund, ein stark überlastetes Werkzeug wie Strings zu verwenden. Dies führt zu...
• Fehleranfällig - Sagen wir einfach, Sie wollen auf mörderische Weise versuchen, zu debuggen, warum Dinge nicht funktionieren, wenn ein Verbraucher versehentlich den Namen eines Zaubertuchs festlegt MagicC1oth. Im Ernst, bei solchen Fehlern kann es Tage dauern, bis Sie feststellen, was passiert ist.

Eine Aufzählung funktioniert viel besser. Es ist schnell, günstig und weitaus weniger fehleranfällig:

class Tool {
public:
    enum TypeE {
        kSword,
        kShield,
        kMagicCloth
    };
    TypeE type;

    std::string typeName() const {
        switch(type) {
            case kSword:      return "Sword";
            case kSheild:     return "Sheild";
            case kMagicCloth: return "Magic Cloth";

            default:
                throw std::runtime_error("Invalid enum!");
        }
   }
};

Dieses Beispiel zeigt auch eine switchAussage, die die Aufzählungen mit einbezieht, mit dem wichtigsten Teil dieses Musters: einem defaultFall, der wirft. Sie sollten niemals in diese Situation geraten, wenn Sie die Dinge perfekt machen. Wenn jedoch jemand einen neuen Tooltyp hinzufügt und Sie vergessen, den Code zu aktualisieren, um ihn zu unterstützen, möchten Sie, dass der Fehler behoben wird. Tatsächlich empfehle ich sie so sehr, dass Sie sie hinzufügen sollten, selbst wenn Sie sie nicht benötigen.

Der andere große Vorteil von enumist, dass dem nächsten Entwickler eine vollständige Liste der gültigen Werkzeugtypen zur Verfügung steht. Es ist nicht nötig, den Code zu durchforsten, um Bobs spezielle Flötenklasse zu finden, die er in seinem epischen Bosskampf verwendet.

void damageWargear(Tool* tool)
{
    switch(tool->type)
    {
        case Tool::kSword:
            static_cast<Sword*>(tool)->damageSword();
            break;
        case Tool::kShield:
            static_cast<Sword*>(tool)->damageShield();
            break;
        default:
            break; // Ignore all other objects
    }
}

Ja, ich habe eine "leere" Standardanweisung eingefügt, um dem nächsten Entwickler klar zu machen, was zu erwarten ist, wenn ein neuer unerwarteter Typ auf mich zukommt.

Wenn Sie dies tun, riecht das Muster weniger. Um jedoch geruchsfrei zu sein, müssen Sie als letztes die anderen Optionen in Betracht ziehen. Diese Casts sind einige der mächtigsten und gefährlichsten Werkzeuge, die Sie im C ++ - Repertoire haben. Sie sollten sie nicht verwenden, es sei denn, Sie haben einen guten Grund.

Eine sehr beliebte Alternative ist das, was ich als "Gewerkschaftsstruktur" oder "Gewerkschaftsklasse" bezeichne. Für Ihr Beispiel wäre dies tatsächlich eine sehr gute Passform. Um eine davon zu erstellen, erstellen Sie eine ToolKlasse mit einer Aufzählung wie zuvor, aber statt einer Unterklasse Toolwerden nur alle Felder von jedem Untertyp darauf platziert.

class Tool {
    public:
        enum TypeE {
            kSword,
            kShield,
            kMagicCloth
        };
    TypeE type;

    int   attack;
    int   defense;
};

Jetzt brauchen Sie überhaupt keine Unterklassen mehr. Sie müssen sich nur das typeFeld ansehen, um zu sehen, welche anderen Felder tatsächlich gültig sind. Dies ist viel sicherer und leichter zu verstehen. Es hat jedoch Nachteile. Es gibt Zeiten, in denen Sie dies nicht verwenden möchten:

• Wenn die Objekte zu unterschiedlich sind - Sie erhalten möglicherweise eine Wäscheliste mit Feldern, und es ist unklar, welche für die einzelnen Objekttypen gelten.
• Wenn Sie in einer speicherkritischen Situation arbeiten - Wenn Sie 10 Werkzeuge herstellen müssen, können Sie mit dem Speicher faul sein. Wenn Sie 500 Millionen Werkzeuge herstellen müssen, kümmern Sie sich um Bits und Bytes. Unionsstrukturen sind immer größer als sie sein müssen.

Diese Lösung wird von UNIX-Sockets aufgrund des Unähnlichkeitsproblems, das durch die offene Endlichkeit der API verursacht wird, nicht verwendet. Mit UNIX-Sockets sollte etwas geschaffen werden, mit dem jede UNIX-Variante arbeiten kann. Jede Variante könnte die Liste der Familien definieren, die sie unterstützen AF_INET, und es würde für jede eine kurze Liste geben. Wenn jedoch ein neues Protokoll AF_INET6hinzukommt, müssen Sie möglicherweise neue Felder hinzufügen. Wenn Sie dies mit einer Unionsstruktur tun würden, würden Sie effektiv eine neue Version der Struktur mit demselben Namen erstellen und endlose Inkompatibilitätsprobleme verursachen. Aus diesem Grund haben sich die UNIX-Sockets dafür entschieden, das Casting-Muster anstelle einer Union-Struktur zu verwenden. Ich bin sicher, sie haben darüber nachgedacht, und die Tatsache, dass sie darüber nachgedacht haben, ist ein Teil dessen, warum es nicht riecht, wenn sie es benutzen.

Sie könnten auch eine Union für real verwenden. Gewerkschaften sparen Speicher, indem sie nur so groß sind wie das größte Mitglied, aber sie haben ihre eigenen Probleme. Dies ist wahrscheinlich keine Option für Ihren Code, aber es ist immer eine Option, die Sie in Betracht ziehen sollten.

Eine andere interessante Lösung ist boost::variant. Boost ist eine großartige Bibliothek mit wiederverwendbaren plattformübergreifenden Lösungen. Es ist wahrscheinlich einer der besten C ++ - Codes, die jemals geschrieben wurden. Boost.Variant ist im Grunde die C ++ - Version von Gewerkschaften. Es ist ein Container, der viele verschiedene Typen enthalten kann, aber jeweils nur einen. Sie könnten Ihre machen Sword, Shieldund MagicClothKlassen, dann Werkzeug sein , macht boost::variant<Sword, Shield, MagicCloth>es einen dieser drei Typen enthält Sinn. Dies hat immer noch dasselbe Problem mit der zukünftigen Kompatibilität, das die Verwendung von UNIX-Sockets verhindert (ganz zu schweigen davon, dass UNIX-Sockets älter als C sind)boostvon ziemlich viel!), aber dieses Muster kann unglaublich nützlich sein. Variant wird beispielsweise häufig in Analysebäumen verwendet, die eine Zeichenfolge von Text verwenden und diese anhand einer Grammatik für Regeln auflösen.

Die endgültige Lösung, die ich vor dem Eintauchen und Verwenden des generischen Objektguss-Ansatzes empfehlen würde, ist das Besucher- Entwurfsmuster. Visitor ist ein leistungsfähiges Entwurfsmuster, das die Beobachtung nutzt, dass das Aufrufen einer virtuellen Funktion das von Ihnen benötigte Casting effektiv und für Sie erledigt. Weil der Compiler es tut, kann es niemals falsch sein. Anstatt eine Aufzählung zu speichern, verwendet Visitor daher eine abstrakte Basisklasse mit einer vtable, die den Typ des Objekts kennt. Wir erstellen dann einen hübschen kleinen Aufruf mit doppelter Indirektion, der die Arbeit erledigt:

class Tool;
class Sword;
class Shield;
class MagicCloth;

class ToolVisitor {
public:
    virtual void visit(Sword* sword) = 0;
    virtual void visit(Shield* shield) = 0;
    virtual void visit(MagicCloth* cloth) = 0;
};

class Tool {
public:
    virtual void accept(ToolVisitor& visitor) = 0;
};

lass Sword : public Tool{
public:
    virtual void accept(ToolVisitor& visitor) { visitor.visit(*this); }
    int attack;
};
class Shield : public Tool{
public:
    virtual void accept(ToolVisitor& visitor) { visitor.visit(*this); }
    int defense;
};
class MagicCloth : public Tool{
public:
    virtual void accept(ToolVisitor& visitor) { visitor.visit(*this); }
    int attack;
    int defense;
};

Also, wie sieht dieses gottesfürchtige Muster aus? Nun, Toolhat eine virtuelle Funktion accept. Wenn Sie es einem Besucher übergeben, wird erwartet, dass es sich umdreht und die richtige visitFunktion für diesen Besucher für den Typ aufruft. Dies ist, was das visitor.visit(*this);für jeden Untertyp tut. Kompliziert, aber wir können dies mit Ihrem obigen Beispiel zeigen:

class AttackVisitor : public ToolVisitor
{
public:
    int& currentAttack;
    int& currentDefense;

    AttackVisitor(int& currentAttack_, int& currentDefense_)
    : currentAttack(currentAttack_)
    , currentDefense(currentDefense_)
    { }

    virtual void visit(Sword* sword)
    {
        currentAttack += sword->attack;
    }

    virtual void visit(Shield* shield)
    {
        currentDefense += shield->defense;
    }

    virtual void visit(MagicCloth* cloth)
    {
        currentAttack += cloth->attack;
        currentDefense += cloth->defense;
    }
};

void Player::attack()
{
    int currentAttack = this->attack;
    int currentDefense = this->defense;
    AttackVisitor v(currentAttack, currentDefense);
    for (Tool* t: tools) {
        t->accept(v);
    }
    //some other functions to start attack
}

Also, was passiert hier? Wir erschaffen einen Besucher, der für uns etwas Arbeit leistet, sobald er weiß, welche Art von Objekt er besucht. Anschließend durchlaufen wir die Liste der Tools. Nehmen wir an, das erste Objekt ist ein Shield, aber unser Code weiß das noch nicht. Es ruft t->accept(v)eine virtuelle Funktion auf. Da das erste Objekt ein Schild ist, void Shield::accept(ToolVisitor& visitor)ruft es am Ende , was ruft visitor.visit(*this);. Wenn wir uns nun ansehen, was visitwir anrufen sollen, wissen wir bereits, dass wir einen Schild haben (weil diese Funktion aufgerufen wurde), also werden wir am Ende void ToolVisitor::visit(Shield* shield)unseren anrufen AttackVisitor. Dies führt nun den richtigen Code aus, um unsere Verteidigung zu aktualisieren.

Besucher ist sperrig. Es ist so klobig, dass ich fast denke, es hat einen eigenen Geruch. Es ist sehr einfach, schlechte Besuchermuster zu schreiben. Es hat jedoch einen großen Vorteil, den keiner der anderen hat. Wenn wir einen neuen Werkzeugtyp hinzufügen, müssen wir dafür eine neue ToolVisitor::visitFunktion hinzufügen . In dem Moment, in dem wir dies tun, wird sich jeder ToolVisitor im Programm weigern, zu kompilieren, weil ihm eine virtuelle Funktion fehlt. Dies macht es sehr einfach, alle Fälle zu erfassen, in denen wir etwas verpasst haben. Es ist viel schwieriger zu garantieren , dass , wenn Sie verwenden ifoder switchAussagen , die Arbeit zu tun. Diese Vorteile sind gut genug, dass Visitor in Generatoren für 3D-Grafikszenen eine nette kleine Nische gefunden hat. Sie brauchen genau die Art von Verhalten, die der Besucher anbietet, damit es großartig funktioniert!

Denken Sie daran, dass diese Muster es dem nächsten Entwickler schwer machen. Nehmen Sie sich Zeit, um es ihnen leichter zu machen, und der Code riecht nicht!

* Technisch gesehen hat sockaddr ein Mitglied namens sa_family. Hier auf der C-Ebene gibt es einige knifflige Aufgaben, die für uns keine Rolle spielen. Sie können sich gerne die tatsächliche Implementierung ansehen , aber für diese sa_family sin_familyund andere Antworten werde ich sie vollständig austauschen, wobei Sie die für die Prosa intuitivste verwenden und darauf vertrauen, dass dieser C-Trick die unwichtigen Details berücksichtigt.

Im Allgemeinen vermeide ich es, mehrere Klassen zu implementieren / zu erben, wenn es nur um die Kommunikation von Daten geht. Sie können sich an eine einzelne Klasse halten und von dort aus alles implementieren. Für Ihr Beispiel ist das genug

class Tool{
    public:
    //constructor, name etc.
    int GetAttack() { return attack }; //Endpoints for your Player
    int GetDefense() { return defense };
    protected:
         int attack;
         int defense;
};

Wahrscheinlich erwarten Sie, dass Ihr Spiel mehrere Arten von Schwertern usw. implementiert, aber Sie haben andere Möglichkeiten, dies zu implementieren. Klassenexplosion ist selten die beste Architektur. Halte es einfach.

Wie bereits erwähnt, ist dies ein schwerwiegender Codegeruch. Man könnte jedoch davon ausgehen, dass die Ursache für Ihr Problem in der Verwendung der Vererbung anstelle der Komposition in Ihrem Entwurf liegt.

Angenommen, Sie haben uns drei Konzepte gezeigt:

• Artikel
• Gegenstand, der angegriffen werden kann.
• Gegenstand, der verteidigt werden kann.

Beachten Sie, dass Ihre vierte Klasse nur eine Kombination der letzten beiden Konzepte ist. Deshalb würde ich vorschlagen, dafür Komposition zu verwenden.

Sie benötigen eine Datenstruktur, um die für einen Angriff erforderlichen Informationen darzustellen. Und Sie benötigen eine Datenstruktur, die die Informationen darstellt, die Sie für die Verteidigung benötigen. Zuletzt benötigen Sie eine Datenstruktur, um Dinge darzustellen, die eine oder beide der folgenden Eigenschaften aufweisen können:

class Attack
{
private:
  int attack_;

public:
  int AttackValue() const;
};

class Defense
{
private:
  int defense_

public:
  int DefenseValue() const;
};

class Tool
{
private:
  std::optional<Attack> atk_;
  std::optional<Defense> def_;

public:
  const std::optional<Attack> &GetAttack() const {return atk_;}
  const std::optional<Defense> &GetDefense() const {return def_;}
};

Warum nicht abstrakte Methoden modifyAttackund modifyDefensein der ToolKlasse erstellen ? Dann würde jedes Kind seine eigene Implementierung haben, und Sie nennen das elegant:

for(Tool* tool : tools){
    currentAttack = tool->recalculateAttack(currentAttack);
    currentDefense = tool->recalculateDefense(currentDefense);
}
// proceed with new values for currentAttack and currentDefense

Wenn Sie Werte als Referenz übergeben, werden Ressourcen gespart, wenn Sie:

for(Tool* tool : tools){
    tool->recalculateAttack(&currentAttack);
    tool->recalculateDefense(&currentDefense);
}
// proceed with new values for currentAttack and currentDefense

Wenn man Polymorphismus verwendet, ist es immer am besten, wenn sich der gesamte Code, der sich um die verwendete Klasse kümmert, in der Klasse selbst befindet. So würde ich es codieren:

class Tool{
 public:
   virtual void equipTo(Player* player) =0;
   virtual void unequipFrom(Player* player) =0;
};

class Sword : public Tool{
  public:
    int attack;
    virtual void equipTo(Player* player) {
      player->attackBonus+=this->attack;
    };
    //unequipFrom = reverse equip
};
class Shield : public Tool{
  public:
    int defense;
    virtual void equipTo(Player* player) {
      player->defenseBonus+=this->defense;
    };
    //unequipFrom = reverse equip
};
//other tools
class Player{
  public:
    int baseAttack;
    int baseDefense;
    int attackBonus;
    int defenseBonus;

    virtual void equip(Tool* tool) {
      tool->equipTo(this);
      this->tools.push_back(tool)
    };

    //unequip = reverse equip

    void attack(){
      //modified attack and defense
      int modifiedAttack = baseAttack + this->attackBonus;
      int modifiedDefense = baseDefense+ this->defenseBonus;
      //some other functions to start attack
    }
  private:
    vector<Tool*> tools;
};

Dies hat folgende Vorteile:

• Einfacheres Hinzufügen neuer Klassen: Sie müssen nur alle abstrakten Methoden implementieren und der Rest des Codes funktioniert einfach
• Klassen lassen sich leichter entfernen
• Es ist einfacher, neue Statistiken hinzuzufügen (Klassen, die sich nicht um die Statistik kümmern, ignorieren sie einfach)

Ich denke, eine Möglichkeit, die Fehler in diesem Ansatz zu erkennen, besteht darin, Ihre Idee zu ihrer logischen Schlussfolgerung zu entwickeln.

Das sieht aus wie ein Spiel, also werden Sie sich wahrscheinlich irgendwann Sorgen um die Leistung machen und diese Zeichenfolgenvergleiche gegen ein intoder austauschen enum. Wenn die Liste der Elemente länger wird, wird dies if-elseziemlich unhandlich. Sie können daher in Erwägung ziehen, sie in eine neue Version zu konvertieren switch-case. Sie haben an dieser Stelle auch eine ziemliche Textwand, sodass Sie die Aktion in jeder caseFunktion in eine separate Funktion umwandeln können.

Sobald Sie diesen Punkt erreicht haben, wird die Struktur Ihres Codes vertraut - sie sieht aus wie eine handgerollte Homebrew-Tabelle * - die Grundstruktur, auf der virtuelle Methoden normalerweise implementiert werden. Dies ist jedoch eine Tabelle, die Sie jedes Mal manuell aktualisieren und pflegen müssen, wenn Sie einen Elementtyp hinzufügen oder ändern.

Indem Sie sich an "reale" virtuelle Funktionen halten, können Sie die Implementierung des Verhaltens jedes Elements innerhalb des Elements selbst beibehalten. Sie können zusätzliche Elemente auf eigenständige und konsistente Weise hinzufügen. Und während Sie dies alles tun, ist es der Compiler, der sich um die Implementierung Ihres dynamischen Versands kümmert, und nicht Sie.

Um Ihr spezifisches Problem zu lösen: Sie haben Mühe, ein einfaches Paar virtueller Funktionen zum Aktualisieren von Angriff und Verteidigung zu schreiben, da einige Elemente nur den Angriff und einige Elemente nur die Verteidigung betreffen. Der Trick in einem einfachen Fall wie diesem, beide Verhaltensweisen trotzdem zu implementieren, aber in bestimmten Fällen ohne Wirkung. GetDefenseBonus()könnte zurückkehren 0oder ApplyDefenseBonus(int& defence)einfach gehendefence unverändert bleiben. Wie Sie vorgehen, hängt davon ab, wie Sie mit den anderen Aktionen umgehen möchten, die Auswirkungen haben. In komplexeren Fällen, in denen das Verhalten abwechslungsreicher ist, können Sie die Aktivität einfach zu einer einzigen Methode kombinieren.

_{* (Wird jedoch in Bezug auf die typische Implementierung umgesetzt.)}

Einen Codeblock zu haben, der alle möglichen "Werkzeuge" kennt, ist kein großartiges Design (zumal Sie am Ende viele solcher Blöcke in Ihrem Code haben werden). Aber es gibt auch kein Basic Toolmit Stubs für alle möglichen Werkzeugeigenschaften: Jetzt Toolmuss die Klasse über alle möglichen Verwendungen Bescheid wissen.

Was jedes Werkzeug weiß, ist, was es zu dem Charakter beitragen kann, der es benutzt. Stellen Sie also eine Methode für alle Werkzeuge bereit giveto(*Character owner). Es passt die Statistiken des Spielers an, ohne zu wissen, was andere Werkzeuge können, und was am besten ist, es muss auch nicht über irrelevante Eigenschaften des Charakters informiert sein. Zum Beispiel hat ein Schild nicht einmal über die Attribute muß wissen attack, invisibility, healthusw. Alles , was ein Werkzeug anzuwenden benötigt werden ist für das Zeichen der Attribute zu unterstützen , dass das Objekt erfordert. Wenn Sie versuchen, einem Esel ein Schwert zu geben und der Esel keine attackStatistik hat, wird eine Fehlermeldung angezeigt.

Die Werkzeuge sollten auch eine remove()Methode haben, die ihre Wirkung auf den Eigentümer umkehrt. Dies ist etwas schwierig (es kann vorkommen, dass Werkzeuge vorhanden sind, die einen Effekt ungleich Null hinterlassen, wenn sie ausgegeben und dann entfernt werden), aber zumindest für jedes Werkzeug lokalisiert.

Es gibt keine Antwort, die besagt, dass es nicht riecht. Ich werde derjenige sein, der für diese Meinung steht. Dieser Code ist völlig in Ordnung! Meine Meinung basiert auf der Tatsache, dass es manchmal einfacher ist, weiterzumachen und Ihre Fähigkeiten schrittweise zu verbessern, wenn Sie mehr neue Inhalte erstellen. Man kann tagelang nicht weiterkommen, um eine perfekte Architektur zu erstellen, aber wahrscheinlich wird es niemand jemals in Aktion sehen, weil man das Projekt nie abgeschlossen hat. Prost!