Czy to zapach kodu do przechowywania ogólnych obiektów w kontenerze, a następnie pobierania obiektów i spuszczania obiektów z kontenera?

34

Na przykład mam grę, która ma narzędzia do zwiększenia zdolności gracza:

Tool.h

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

I niektóre narzędzia:

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

I wtedy gracz może mieć kilka narzędzi do ataku:

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

Myślę, że trudno jest zastąpić if-elsewirtualne metody w narzędziach, ponieważ każde narzędzie ma inne właściwości, a każde narzędzie wpływa na atak i obronę gracza, dla których aktualizacja ataku i obrony gracza musi zostać wykonana wewnątrz obiektu gracza.

Ale nie byłem zadowolony z tego projektu, ponieważ zawiera on downcasting, z długim if-elsestwierdzeniem. Czy ten projekt wymaga „korekty”? Jeśli tak, co mogę zrobić, aby to poprawić?

grgrr
źródło
4
Standardową techniką OOP do usuwania testów dla określonej podklasy (i późniejszych downcastów) jest utworzenie, aw tym przypadku może dwóch, wirtualnych metod w klasie bazowej do użycia zamiast łańcucha if i rzutowania. Można tego użyć do całkowitego usunięcia if i delegowania operacji do podklas do zaimplementowania. Nie będziesz też musiał edytować instrukcji if za każdym razem, gdy dodasz nową podklasę.
Erik Eidt,
2
Weź również pod uwagę Podwójną wysyłkę.
Boris the Spider
Dlaczego nie dodać właściwości do klasy Tool, która zawiera słownik typów atrybutów (tj. Atak, obrona) i przypisaną do niej wartość. Atak, obrona można wyliczyć wartości. Następnie możesz po prostu wywołać wartość z samego Narzędzia za pomocą wyliczonej stałej.
user1740075,
1
Zobacz także wzór gościa.
JDługosz

Odpowiedzi:

63

Tak, to zapach kodu (w wielu przypadkach).

Myślę, że trudno jest zastąpić if-else metodami wirtualnymi w narzędziach

W twoim przykładzie dość proste jest zastąpienie if / else metodami wirtualnymi:

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

Teraz nie ma już potrzeby ifblokowania, dzwoniący może po prostu go używać

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

Oczywiście w bardziej skomplikowanych sytuacjach takie rozwiązanie nie zawsze jest tak oczywiste (ale prawie zawsze możliwe). Ale jeśli dojdziesz do sytuacji, w której nie wiesz, jak rozwiązać sprawę za pomocą metod wirtualnych, możesz ponownie zadać nowe pytanie tutaj w „Programistach” (lub, jeśli staje się ono specyficzne dla języka lub implementacji, w Stackoverflow).

Doktor Brown
źródło
4
lub, jeśli o to chodzi, na gamedev.stackexchange.com
Kromster mówi o wsparciu Monica
7
Nie potrzebujesz nawet takiej koncepcji Swordw swojej bazie kodu. Możesz po prostu new Tool("sword", swordAttack, swordDefense)np. Z pliku JSON.
AmazingDreams,
7
@AmazingDreams: to prawda (dla części kodu, które widzimy tutaj), ale myślę, że OP uprościł swój prawdziwy kod, aby jego pytanie skupiło się na aspekcie, który chciał omówić.
Doc Brown
3
Nie jest to o wiele lepsze niż oryginalny kod (cóż, jest trochę). Żadne narzędzie, które ma dodatkowe właściwości, nie może zostać utworzone bez dodania dodatkowych metod. Myślę, że w tym przypadku należy preferować kompozycję nad dziedziczeniem. Tak, obecnie jest tylko atak i obrona, ale to nie musi tak pozostać.
Polygnome,
1
@DocBrown Tak, to prawda, choć wygląda jak RPG, w której postać ma pewne statystyki, które są modyfikowane za pomocą narzędzi lub raczej wyposażonych przedmiotów. Zrobiłbym ogólny Toolze wszystkimi możliwymi modyfikatorami, wypełniłem niektóre vector<Tool*>rzeczy odczytane z pliku danych, a następnie po prostu zapętliłem je i zmodyfikowałem statystyki tak jak teraz. Wpadniesz w kłopoty, gdy chcesz, aby jakiś przedmiot dawał np. 10% premii za atak. Być może a tool->modify(playerStats)jest inną opcją.
AmazingDreams,
23

Główny problem z twoim kodem polega na tym, że za każdym razem, gdy wprowadzasz nowy przedmiot, musisz nie tylko pisać i aktualizować kod przedmiotu, ale także modyfikować odtwarzacz (lub gdziekolwiek ten przedmiot jest używany), co sprawia, że ​​całość jest o wiele bardziej skomplikowane.

Zasadniczo myślę, że zawsze jest to trochę podejrzane, kiedy nie możesz polegać na normalnej podklasie / dziedziczeniu i musisz sam wykonać upcasting.

Mógłbym wymyślić dwa możliwe podejścia, które uczynią całość bardziej elastyczną:

  • Jak wspomniano inni, przenieś członków attacki defensedo klasy podstawowej i po prostu zainicjuj ich 0. Może to również podwójnie sprawdzać, czy rzeczywiście jesteś w stanie wymachnąć przedmiotem do ataku lub użyć go do zablokowania ataków.

  • Utwórz system wywołań zwrotnych / zdarzeń. Istnieją różne możliwe podejścia do tego.

    Co powiesz na prostotę?

    • Możesz utworzyć członków klasy podstawowej, takich jak virtual void onEquip(Owner*) {}i virtual void onUnequip(Owner*) {}.
    • Ich przeciążenia byłoby nazwać i modyfikować statystyk kiedy (UN) wyposażanie element, np virtual void onEquip(Owner *o) { o->modifyStat("attack", attackValue); }a virtual void onUnequip(Owner *o) { o->modifyStat("attack", -attackValue); }.
    • Do statystyk można uzyskać dostęp w sposób dynamiczny, np. Za pomocą krótkiego łańcucha lub stałej jako klucza, dzięki czemu można nawet wprowadzić nowe wartości lub bonusy, które niekoniecznie muszą być obsługiwane przez gracza lub „właściciela”.
    • W porównaniu do samego żądania wartości ataku / obrony w samą porę, nie tylko sprawia to, że całość jest bardziej dynamiczna, ale także oszczędza niepotrzebnych połączeń, a nawet pozwala tworzyć przedmioty, które będą miały trwały wpływ na twoją postać.

      Wyobraź sobie na przykład przeklęty pierścień, który po założeniu ukryje statystyki, oznaczając na stałe twoją postać jako przeklętą.

Mario
źródło
7

Chociaż @DocBrown udzieliło dobrej odpowiedzi, nie idzie ono wystarczająco daleko, imho. Zanim zaczniesz oceniać odpowiedzi, powinieneś ocenić swoje potrzeby. Czego naprawdę potrzebujesz ?

Poniżej pokażę dwa możliwe rozwiązania, które oferują różne zalety dla różnych potrzeb.

Pierwszy jest bardzo uproszczony i dostosowany specjalnie do tego, co pokazałeś:

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

To pozwala bardzo łatwą serializację / deserializację narzędzi (np. Do zapisywania lub tworzenia sieci) i nie wymaga wirtualnej wysyłki. Jeśli twój kod jest wszystkim, co pokazałeś, i nie oczekujesz, że będzie ewoluował znacznie bardziej niż posiadanie większej liczby różnych narzędzi o różnych nazwach i statystykach, tylko w różnych ilościach, to jest to właściwy sposób.

@DocBrown oferuje rozwiązanie, które nadal opiera się na wirtualnej wysyłce, i może być zaletą, jeśli w jakiś sposób specjalizujesz się w narzędziach dla części kodu, które nie zostały pokazane. Jeśli jednak naprawdę chcesz lub chcesz zmienić inne zachowanie, sugerowałbym następujące rozwiązanie:

Kompozycja nad dziedziczeniem

Co jeśli później chcesz mieć narzędzie, które modyfikuje zwinność ? Czy prędkość biegania ? Wydaje mi się, że tworzysz RPG. Jedną z ważnych rzeczy w grach RPG jest możliwość rozszerzenia . Rozwiązania pokazane do tej pory tego nie oferują. Będziesz musiał zmienićTool klasę i dodawać do niej nowe metody wirtualne za każdym razem, gdy potrzebujesz nowego atrybutu.

Drugim rozwiązaniem, które pokazuję, jest to, o którym wspomniałem wcześniej w komentarzu - wykorzystuje kompozycję zamiast dziedziczenia i jest zgodne z zasadą „zamknięte dla modyfikacji, otwarte na rozszerzenie *. Jeśli znasz zasady działania systemów encji, niektóre rzeczy będzie wyglądać znajomo (lubię myśleć o kompozycji jako o mniejszym bracie ES).

Zauważ, że to, co pokazuję poniżej, jest znacznie bardziej eleganckie w językach, które zawierają informacje o typie środowiska wykonawczego, takich jak Java lub C #. Dlatego pokazany przeze mnie kod C ++ musi zawierać „księgowość”, która jest po prostu niezbędna, aby kompozycja działała tutaj. Może ktoś z większym doświadczeniem w C ++ jest w stanie zaproponować jeszcze lepsze podejście.

Najpierw ponownie patrzymy na stronę dzwoniącego . W twoim przykładzie jako rozmówcy w attackmetodzie nie przejmujesz się narzędziami. Ważne są dwie właściwości - punkty ataku i obrony. Tak naprawdę nie obchodzi cię, skąd one pochodzą, i nie obchodzą cię inne właściwości (np. Szybkość biegu, zwinność).

Najpierw wprowadzamy nową klasę

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

Następnie tworzymy nasze pierwsze dwa komponenty

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

Następnie tworzymy Narzędzie przechowujące zestaw właściwości i udostępniamy właściwości innym osobom.

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

Zauważ, że w tym przykładzie obsługuję tylko jeden komponent każdego typu - to ułatwia rzeczy. Teoretycznie można również zezwolić na wiele komponentów tego samego typu, ale robi się to brzydko bardzo szybko. Jeden ważny aspekt: Tooljest teraz zamknięty dla modyfikacji - nigdy więcej nie dotkniemy źródła Tool- ale otwarty na rozszerzenie - możemy rozszerzyć zachowanie Narzędzia, modyfikując inne rzeczy i po prostu przekazując do niego inne Komponenty.

Teraz potrzebujemy sposobu na wyszukiwanie narzędzi według typów komponentów. Nadal możesz użyć wektora do narzędzi, tak jak w przykładzie kodu:

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

Możesz również zmienić to na własne Inventory klasę i przechowywać tabele wyszukiwania, które znacznie upraszczają pobieranie narzędzi według typu komponentu i unikają powtarzania całej kolekcji.

Jakie zalety ma to podejście? W attack, ty przetwarzać Narzędzia, które mają dwa składniki - nie dbam o nic innego.

Wyobraźmy sobie, że masz walkTometodę, a teraz zdecydujesz, że dobrym pomysłem byłoby, gdyby jakieś narzędzie zyskało możliwość modyfikacji prędkości chodzenia. Nie ma problemu!

Najpierw utwórz nowy Component:

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

Następnie po prostu dodajesz instancję tego komponentu do narzędzia, które chcesz zwiększyć swoją prędkość budzenia, i zmieniasz WalkTometodę przetwarzania właśnie utworzonego komponentu:

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

Pamiętaj, że dodaliśmy pewne zachowanie do naszych Narzędzi, nie zmieniając w ogóle klasy Tools.

Możesz (i powinieneś) przenieść łańcuchy do makra lub stałej zmiennej statycznej, więc nie musisz wpisywać go wielokrotnie.

Jeśli posuniesz się dalej tym podejściem - np. Stworzysz komponenty, które można dodać do gracza, i stworzysz Combatkomponent, który oznaczać będzie, że gracz może brać udział w walce, możesz także pozbyć się tej attackmetody i pozwolić sobie na to przez Komponent lub przetwarzane w innym miejscu.

Zaletą tego, że gracz będzie mógł również zdobyć Komponenty, byłoby to, że nie trzeba nawet zmieniać gracza, aby nadać mu inne zachowanie. W moim przykładzie możesz stworzyć Movablekomponent, dzięki czemu nie będziesz musiał zaimplementować walkTometody w odtwarzaczu, aby go poruszyć. Po prostu utwórz komponent, podłącz go do odtwarzacza i pozwól, aby ktoś inny go przetworzył.

Kompletny przykład można znaleźć w tej liście: https://gist.github.com/NetzwergX/3a29e1b106c6bb9c7308e89dd715ee20

To rozwiązanie jest oczywiście nieco bardziej skomplikowane niż inne, które zostały opublikowane. Ale w zależności od tego, jak elastyczny chcesz być, jak daleko chcesz to zrobić, może to być bardzo skuteczne podejście.

Edytować

Niektóre inne odpowiedzi proponują bezpośrednie dziedziczenie (Tworzenie miecza przedłużaniem narzędzia, tworzenie tarczy przedłużaniem narzędzia). Nie sądzę, że jest to scenariusz, w którym dziedziczenie działa bardzo dobrze. Co jeśli zdecydujesz, że blokowanie tarczą w określony sposób może również uszkodzić atakującego? Dzięki mojemu rozwiązaniu możesz po prostu dodać składnik Attack do tarczy i zdać sobie sprawę, że bez żadnych zmian w kodzie. Z dziedziczeniem miałbyś problem. Przedmioty / narzędzia w grach RPG są głównymi kandydatami do składu, a nawet od razu używają systemów jednostek od samego początku.

Polygnome
źródło
1

Ogólnie rzecz biorąc, jeśli kiedykolwiek będziesz potrzebować if (w połączeniu z wymaganiem typu instancji) w dowolnym języku OOP, to znak, że dzieje się coś śmierdzącego. Przynajmniej powinieneś przyjrzeć się bliżej swoim modelom.

Zmodelowałbym twoją domenę inaczej.

W twoim przypadku a Toolma a AttackBonusi DefenseBonus- które mogą być oba, 0na wypadek, gdyby były bezużyteczne do walki jak pióra lub coś w tym rodzaju.

Do ataku masz swoją baserate+ bonusz użytej broni. To samo dotyczy obrony baserate+ bonus.

W konsekwencji Toolmusisz mieć virtualmetodę obliczania boni ataku / obrony.

tl; dr

Dzięki lepszemu projektowi można uniknąć włamań if.

Thomas Junk
źródło
Czasami niezbędny jest if, na przykład przy porównywaniu wartości skalarnych. Do przełączania typów obiektów nie tyle.
Andy,
Haha, jeśli jest dość istotnym operatorem i nie można po prostu powiedzieć, że używanie jest zapachem kodu.
tymtam,
1
@ Tymski do pewnego stopnia masz rację. Wyjaśniłem się. Nie bronię ifmniej programowania. Głównie w kombinacjach takich jak if instanceoflub coś takiego. Ale jest pozycja, która twierdzi, ifże są kodową komórką i istnieją sposoby na obejście tego. I masz rację, to niezbędny operator, który ma swoje własne prawa.
Thomas Junk
1

Jak napisano, „pachnie”, ale mogą to być tylko podane przez ciebie przykłady. Przechowywanie danych w ogólnych kontenerach obiektów, a następnie rzutowanie ich w celu uzyskania dostępu do danych nie powoduje automatycznie zapachu kodu. Zobaczysz go używany w wielu sytuacjach. Jednak podczas korzystania z niego powinieneś być świadomy tego, co robisz, jak to robisz i dlaczego. Kiedy patrzę na ten przykład, użycie porównań opartych na łańcuchach, aby powiedzieć mi, jaki obiekt jest tym, co wyzwala mój osobisty miernik zapachu. Sugeruje to, że nie jesteś do końca pewien, co tu robisz (co jest w porządku, ponieważ miałeś mądrość, aby przyjść tutaj do programistów. SE i powiedzieć „hej, nie sądzę, że podoba mi się to, co robię, pomóż odpadam!").

Podstawowym problemem związanym z wzorcem rzutowania danych z takich ogólnych pojemników jest to, że producent danych i konsument danych muszą współpracować, ale może nie być oczywiste, że robią to na pierwszy rzut oka. W każdym przykładzie tego wzoru, śmierdzącego lub nie śmierdzącego, jest to podstawowa kwestia. Jest bardzo możliwe, że następny programista całkowicie nie zdaje sobie sprawy z tego, że robisz ten wzór i łamiesz go przypadkowo, więc jeśli użyjesz tego wzoru, musisz uważać, aby pomóc następnemu deweloperowi. Musisz ułatwić mu niezamierzone złamanie kodu z powodu pewnych szczegółów, o których istnieniu mógł nie wiedzieć.

Na przykład, co jeśli chciałbym skopiować gracza? Jeśli popatrzę tylko na zawartość obiektu odtwarzacza, wygląda to całkiem prosto. Po prostu trzeba skopiować attack, defensei toolszmienne. Proste jak ciasto! Cóż, szybko się przekonam, że użycie wskaźników sprawia, że ​​jest to trochę trudniejsze (w pewnym momencie warto spojrzeć na inteligentne wskaźniki, ale to inny temat). Łatwo to rozwiązać. Po prostu utworzę nowe kopie każdego narzędzia i umieszczę je na mojej nowej toolsliście. W końcu Tooljest to naprawdę prosta klasa z tylko jednym członkiem. Tworzę więc kilka kopii, w tym kopię Sword, ale nie wiedziałem, że to miecz, więc skopiowałem tylko name. Później attack()funkcja patrzy na nazwę, widzi, że jest to „miecz”, rzuca go i zdarzają się złe rzeczy!

Możemy porównać ten przypadek do innego przypadku w programowaniu gniazd, który używa tego samego wzorca. Mogę skonfigurować funkcję gniazda UNIX w następujący sposób:

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

Dlaczego to ten sam wzór? Ponieważ bindnie akceptuje a sockaddr_in*, akceptuje bardziej ogólne sockaddr*. Jeśli spojrzysz na definicje tych klas, zobaczymy, że sockaddrma tylko jednego członka rodziny, do której przypisaliśmy sin_family*. Rodzina mówi, do którego podtypu powinieneś użyć sockaddr. AF_INETinformuje, że strukturą adresu jest tak naprawdę sockaddr_in. Gdyby tak było AF_INET6, adresem byłby a sockaddr_in6, który ma większe pola do obsługi większych adresów IPv6.

Jest to identyczne jak w twoim Toolprzykładzie, z tym wyjątkiem, że używa liczby całkowitej, aby określić, która rodzina zamiast std::string. Jednak zamierzam twierdzić, że nie pachnie i staram się to robić z powodów innych niż „to standardowy sposób wykonywania gniazd, więc nie powinien„ wąchać ”. Oczywiście jest to ten sam wzór, który jest dlaczego twierdzę, że przechowywanie danych w obiektach ogólnych i ich rzutowanie nie powoduje automatycznie zapachu kodu, ale istnieją pewne różnice w tym, jak to robią, co czyni je bezpieczniejszym.

Podczas korzystania z tego wzorca najważniejszą informacją jest przechwytywanie przekazywania informacji o podklasie od producenta do konsumenta. To właśnie robisz z namepolem, a gniazda UNIX robią z ich sin_familypolem. To pole jest informacją, której potrzebuje konsument, aby zrozumieć, co faktycznie stworzył producent. We wszystkich przypadkach tego wzorca powinien to być wyliczenie (lub przynajmniej liczba całkowita działająca jak wyliczenie). Czemu? Zastanów się, co zrobi Twój konsument z tymi informacjami. Będą musieli napisać jakieś duże ifoświadczenie lub…switchinstrukcja, tak jak ty, gdzie określają prawidłowy podtyp, rzutuj go i używaj danych. Z definicji może istnieć tylko niewielka liczba tych typów. Możesz przechowywać go w ciągu, tak jak zrobiłeś, ale ma to wiele wad:

  • Wolno - std::stringzwykle musi wykonać pamięć dynamiczną, aby zachować ciąg. Musisz także wykonać pełne porównanie tekstu, aby dopasować nazwę za każdym razem, gdy chcesz dowiedzieć się, jaką masz podklasę.
  • Zbyt wszechstronny - można powiedzieć coś o narzucaniu sobie ograniczeń, gdy robisz coś wyjątkowo niebezpiecznego. Miałem takie systemy, które szukały podłańcucha, który powiedziałby mu, jakiego rodzaju obiekt szuka. Działa to świetnie, dopóki nazwa obiektu przypadkowo nie zawiera tego podłańcucha, i powoduje strasznie tajemniczy błąd. Ponieważ, jak powiedzieliśmy powyżej, potrzebujemy tylko niewielkiej liczby przypadków, nie ma powodu, aby używać masowo przeciążonego narzędzia, takiego jak łańcuchy. To prowadzi do...
  • Podatne na błędy - powiedzmy, że będziesz chciał dokonać morderczego szaleństwa, próbując debugować, dlaczego rzeczy nie działają, gdy jeden konsument przypadkowo ustawia nazwę magicznej tkaniny MagicC1oth. Poważnie, takie błędy mogą zająć wiele dni, zanim zdasz sobie sprawę, co się stało.

Wyliczenie działa znacznie lepiej. Jest szybki, tani i znacznie mniej podatny na błędy:

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!");
        }
   }
};

Ten przykład pokazuje także switchwyrażenie dotyczące wyliczeń, z najważniejszą częścią tego wzorca: defaultprzypadkiem, który rzuca. Nigdy nie powinieneś wchodzić w taką sytuację, jeśli robisz rzeczy doskonale. Jeśli jednak ktoś doda nowy typ narzędzia i zapomnisz zaktualizować swój kod, aby go obsługiwał, będziesz chciał, aby coś złapało błąd. W rzeczywistości polecam je tak bardzo, że powinieneś je dodać, nawet jeśli ich nie potrzebujesz.

Inną ogromną zaletą enumjest to, że daje następnemu programistowi pełną listę prawidłowych typów narzędzi, od samego początku. Nie trzeba przedzierać się przez kod, by znaleźć wyspecjalizowaną klasę fletu Boba, której używa w swojej epickiej bitwie z bossem.

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

Tak, wstawiam „pustą” domyślną instrukcję, aby wyraźnie powiedzieć następnemu deweloperowi, czego się spodziewam, jeśli pojawi się jakiś nowy nieoczekiwany typ.

Jeśli to zrobisz, wzór będzie mniej pachniał. Jednak, aby być pozbawionym zapachu, ostatnią rzeczą, którą musisz zrobić, to rozważyć inne opcje. Te obsady są jednymi z bardziej zaawansowanych i niebezpiecznych narzędzi, które masz w repertuarze C ++. Nie powinieneś ich używać, chyba że masz dobry powód.

Jedną z bardzo popularnych alternatyw jest to, co nazywam „strukturą związkową” lub „klasą związkową”. W twoim przypadku byłoby to bardzo dobre dopasowanie. Aby stworzyć jedną z nich, tworzysz Toolklasę z wyliczeniem jak poprzednio, ale zamiast podklasowania Tool, po prostu umieszczamy na niej wszystkie pola z każdego podtypu.

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

    int   attack;
    int   defense;
};

Teraz w ogóle nie potrzebujesz podklas. Wystarczy spojrzeć na typepole, aby zobaczyć, które inne pola są faktycznie prawidłowe. Jest to o wiele bezpieczniejsze i łatwiejsze do zrozumienia. Ma to jednak wady. Czasami nie chcesz tego używać:

  • Kiedy obiekty są zbyt odmienne - Możesz skończyć z listą prania pól i może nie być jasne, które z nich dotyczą każdego typu obiektu.
  • Podczas pracy w sytuacji krytycznej dla pamięci - jeśli potrzebujesz 10 narzędzi, możesz być leniwy z pamięcią. Kiedy musisz zrobić 500 milionów narzędzi, zaczniesz dbać o bity i bajty. Struktury Unii są zawsze większe, niż powinny.

To rozwiązanie nie jest używane przez gniazda UNIX z powodu problemu z podobieństwem, który jest związany z otwartością interfejsu API. Celem gniazd UNIX było stworzenie czegoś, z czym każdy smak UNIXa mógłby współpracować. Każdy smak może zdefiniować listę obsługiwanych rodzin AF_INET, a dla każdej z nich będzie krótka lista. Jednak jeśli pojawi się nowy protokół, tak jak AF_INET6zrobił, może być konieczne dodanie nowych pól. Jeśli zrobiłbyś to z strukturą związkową, ostatecznie stworzyłbyś nową wersję struktury o tej samej nazwie, powodując niekończące się problemy z niekompatybilnością. Właśnie dlatego gniazda UNIX zdecydowały się użyć wzorca rzutowania zamiast struktury związkowej. Jestem pewien, że to rozważali, a fakt, że o tym pomyśleli, jest częścią tego, dlaczego nie pachnie, gdy go używają.

Możesz też naprawdę wykorzystać związek. Związki oszczędzają pamięć, ponieważ są tak duże jak największy członek, ale mają własny zestaw problemów. Prawdopodobnie nie jest to opcja dla twojego kodu, ale zawsze jest to opcja, którą powinieneś rozważyć.

Innym interesującym rozwiązaniem jest boost::variant. Boost to świetna biblioteka pełna wieloplatformowych rozwiązań wielokrotnego użytku. Jest to prawdopodobnie najlepszy kod C ++, jaki kiedykolwiek napisano. Boost.Variant jest w zasadzie wersją związków w C ++. Jest to pojemnik, który może zawierać wiele różnych typów, ale tylko jeden na raz. Możesz zrobić swoje Sword, Shieldi MagicClothklasy, a następnie sprawić, by narzędzie było całkiem sporo!), Ale ten wzór może być niezwykle użyteczny. Wariant jest często używany, na przykład, w parsowaniu drzew, które pobierają ciąg tekstu i dzielą go za pomocą gramatyki reguł.boost::variant<Sword, Shield, MagicCloth> , co oznacza, że zawiera jedną z tych trzech typów. Nadal występuje ten sam problem z przyszłą kompatybilnością, która uniemożliwia korzystanie z gniazd UNIX (nie wspominając o gniazdach UNIX C, wcześniejszychboost

Ostatecznym rozwiązaniem, na które zaleciłbym przyjrzenie się przed zanurzeniem i zastosowaniem ogólnego sposobu rzucania obiektów, jest wzorzec projektowy Visitor . Visitor to potężny wzorzec projektowy, który wykorzystuje spostrzeżenie, że wywołanie funkcji wirtualnej skutecznie wykonuje rzutowanie, którego potrzebujesz, i robi to za Ciebie. Ponieważ kompilator to robi, nigdy nie może być źle. Dlatego zamiast przechowywać wyliczenie, Visitor używa abstrakcyjnej klasy bazowej, która ma vtable, która wie, jakiego typu jest obiekt. Następnie tworzymy zgrabne połączenie z podwójną pośrednią, które działa:

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

Więc jaki jest ten okropny wzór tego boga? Cóż, Toolma funkcję wirtualnego accept. Jeśli podasz go odwiedzającemu, oczekuje się, że się odwróci i wywoła odpowiednią visitfunkcję dla tego gościa dla danego typu. Tak visitor.visit(*this);działa każdy podtyp. Skomplikowane, ale możemy to pokazać na powyższym przykładzie:

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
}

Co się tu dzieje? Tworzymy gościa, który wykona dla nas trochę pracy, gdy tylko dowie się, jaki typ obiektu odwiedza. Następnie iterujemy listę narzędzi. Na przykład, powiedzmy, że pierwszym obiektem jest Shield, ale nasz kod jeszcze tego nie wie. Wywołuje t->accept(v)funkcję wirtualną. Ponieważ pierwszy obiekt jest tarczą, kończy się wołaniem void Shield::accept(ToolVisitor& visitor), które wywołuje visitor.visit(*this);. Teraz, gdy szukamy, do którego visitzadzwonić, wiemy już, że mamy tarczę (ponieważ ta funkcja została wywołana), więc skończymy void ToolVisitor::visit(Shield* shield)na naszym AttackVisitor. To teraz uruchamia poprawny kod, aby zaktualizować naszą obronę.

Gość jest nieporęczny. Jest tak niezgrabny, że prawie wydaje mi się, że ma swój własny zapach. Bardzo łatwo jest pisać złe wzorce odwiedzających. Ma jednak jedną ogromną przewagę , jakiej nie ma żaden inny. Jeśli dodamy nowy typ narzędzia, musimy dodać ToolVisitor::visitdla niego nową funkcję. Gdy tylko to zrobimy, każdy ToolVisitor program odmówi kompilacji, ponieważ brakuje mu funkcji wirtualnej. Dzięki temu bardzo łatwo jest złapać wszystkie przypadki, w których coś przeoczyliśmy. Znacznie trudniej jest zagwarantować, że jeśli użyjesz iflub switchoświadczeń do wykonania pracy. Te zalety są na tyle dobre, że Visitor znalazł małą niszę w generatorach scen graficznych 3D. Potrzebują dokładnie takiego zachowania, jakie oferuje Odwiedzający, więc działa świetnie!

W sumie pamiętaj, że te wzorce utrudniają następny programista. Poświęć czas, aby im to ułatwić, a kod nie będzie pachniał!

* Technicznie rzecz biorąc, jeśli spojrzysz na specyfikację, sockaddr ma jednego członka o nazwie sa_family. Na poziomie C robi się coś trudnego, co nie ma dla nas znaczenia. Zapraszamy do zapoznania się z faktyczną implementacją, ale dla tej odpowiedzi zamierzam użyć sa_family sin_familyi innych całkowicie zamiennie, używając tego, który jest najbardziej intuicyjny dla prozy, ufając, że oszustwo C zajmuje się nieistotnymi szczegółami.

Cort Ammon - Przywróć Monikę
źródło
Atakowanie kolejno sprawi, że gracz będzie nieskończenie silny w twoim przykładzie. I nie możesz rozszerzyć swojego podejścia bez modyfikacji źródła ToolVisitor. Jest to jednak świetne rozwiązanie.
Polygnome
@Polygnome Masz rację co do przykładu. Myślałem, że kod wygląda dziwnie, ale przewijając wszystkie strony tekstu, pominąłem błąd. Napraw to teraz. Jeśli chodzi o wymóg modyfikacji źródła ToolVisitor, jest to charakterystyka projektowa wzorca użytkownika. Jest to błogosławieństwo (jak napisałem) i klątwa (jak napisałeś). Zajmuje się sprawą, w której chcesz dowolnie rozszerzalny wersja jest znacznie trudniejsze i rozpoczyna kopanie w rozumieniu zmiennych, a nie tylko ich wartości, a otwiera się inne wzory, takie jak słabo wpisywanych zmiennych i słowników i JSON.
Cort Ammon - Przywróć Monikę
1
Tak, niestety nie wiemy wystarczająco dużo o preferencjach i celach PO, aby podjąć naprawdę świadomą decyzję. I tak, w pełni elastyczne rozwiązanie jest trudniejsze do wdrożenia, pracowałem nad odpowiedzią przez prawie 3 godziny, ponieważ mój C ++ jest dość zardzewiały :(
Polygnome
0

Ogólnie rzecz biorąc, unikam implementowania kilku klas / dziedziczenia, jeśli chodzi tylko o komunikację danych. Możesz trzymać się jednej klasy i stamtąd wdrażać wszystko. Dla twojego przykładu to wystarczy

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

Prawdopodobnie spodziewasz się, że twoja gra będzie implementować kilka rodzajów mieczy itp., Ale będziesz miał inne sposoby na wdrożenie tego. Eksplozja klasowa rzadko jest najlepszą architekturą. Nie komplikuj.

Arthur Havlicek
źródło
0

Jak już wspomniano, jest to poważny zapach kodu. Można jednak rozważyć źródło problemu polegającego na zastosowaniu dziedziczenia zamiast kompozycji w projekcie.

Na przykład, biorąc pod uwagę to, co nam pokazałeś, wyraźnie masz 3 koncepcje:

  • Pozycja
  • Przedmiot, który może mieć atak.
  • Przedmiot, który może mieć obronę.

Zauważ, że twoja czwarta klasa jest tylko kombinacją dwóch ostatnich koncepcji. Sugerowałbym więc użycie do tego kompozycji.

Potrzebujesz struktury danych do reprezentowania informacji potrzebnych do ataku. Potrzebujesz struktury danych reprezentującej informacje potrzebne do obrony. Na koniec potrzebujesz struktury danych do reprezentowania rzeczy, które mogą mieć lub nie mieć jedną lub obie z tych właściwości:

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_;}
};
Nicol Bolas
źródło
Ponadto: nie używaj podejścia zawsze komponuj :)! Po co używać kompozycji w tym przypadku? Zgadzam się, że jest to rozwiązanie alternatywne, ale tworzenie klasy do „enkapsulacji” pola (zwróć uwagę na „”) wydaje się w tym przypadku dziwne ...
AilurusFulgens
@AilurusFulgens: Dziś jest to „pole”. Co będzie jutro Ta konstrukcja pozwala Attacki Defensestaje się bardziej skomplikowana bez zmiany interfejsu Tool.
Nicol Bolas,
1
Nadal nie możesz za bardzo rozszerzyć Narzędzia - na pewno atak i obrona mogą stać się bardziej złożone, ale to wszystko. Jeśli użyjesz kompozycji do pełnej mocy, możesz Toolcałkowicie zamknąć dla modyfikacji, jednocześnie pozostawiając ją otwartą do rozszerzenia.
Polygnome
@Polygnome: Jeśli chcesz przejść przez trud stworzenia kompletnego systemu dowolnego elementu w tak trywialnej sprawie, jak to, zależy od Ciebie. Osobiście nie widzę powodu, dla którego chciałbym przedłużyć Toolbez modyfikacji . A jeśli mam prawo to zmodyfikować, nie widzę potrzeby stosowania dowolnych komponentów.
Nicol Bolas,
Tak długo, jak Tool jest pod Twoją kontrolą, możesz go modyfikować. Ale zasada „zamknięta na modyfikację, otwarta na rozszerzenie” istnieje z ważnego powodu (zbyt długa, by ją tu rozwinąć). Nie sądzę jednak, żeby to było takie banalne. Jeśli poświęcisz odpowiedni czas na zaplanowanie elastycznego systemu RPG, na dłuższą metę zyskasz ogromne nagrody. Nie widzę dodatkowej korzyści z tego typu kompozycji w porównaniu do zwykłych pól. Możliwość specjalizacji ataku i obrony wydaje się być scenariuszem bardzo teoretycznym. Ale jak napisałem, zależy to od dokładnych wymagań PO.
Polygnome,
0

Dlaczego nie tworzenie abstrakcyjnych metod modifyAttacki modifyDefensew Toolklasie? Następnie każde dziecko będzie miało własną implementację, a ty nazywasz to eleganckim sposobem:

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

Przekazanie wartości jako odniesienia pozwoli zaoszczędzić zasoby, jeśli możesz:

for(Tool* tool : tools){
    tool->recalculateAttack(&currentAttack);
    tool->recalculateDefense(&currentDefense);
}
// proceed with new values for currentAttack and currentDefense
Paulo Amaral
źródło
0

Jeśli używa się polimorfizmu, zawsze najlepiej jest, jeśli cały kod, który dba o to, która klasa jest używana, znajduje się w samej klasie. Oto jak bym to kodował:

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

Ma to następujące zalety:

  • łatwiej dodawać nowe klasy: wystarczy zaimplementować wszystkie metody abstrakcyjne, a reszta kodu po prostu działa
  • łatwiej usunąć klasy
  • łatwiej dodawać nowe statystyki (klasy, które nie dbają o statystyki, po prostu je zignoruj)
Siphor
źródło
Powinieneś przynajmniej dołączyć metodę unequip (), która usuwa bonus z odtwarzacza.
Polygnome,
0

Myślę, że jednym ze sposobów na rozpoznanie wad tego podejścia jest rozwinięcie swojego pomysłu do jego logicznej konkluzji.

To wygląda jak gra, więc na pewnym etapie prawdopodobnie zaczniesz martwić się wydajnością i zamienisz porównania ciągów znaków na intlub enum. Gdy lista przedmiotów się wydłuża, if-elsezaczyna się robić nieporęczna, więc możesz rozważyć przekształcenie jej w postać switch-case. W tym momencie masz też dość ścianę tekstu, więc możesz wyciszyć akcję w każdej z nich, casetworząc osobną funkcję.

Po osiągnięciu tego punktu struktura kodu zaczyna wyglądać znajomo - zaczyna wyglądać jak homebrew, ręcznie toczony vtable * - podstawowa struktura, na której zwykle wdrażane są metody wirtualne. Poza tym jest to vtable, który należy ręcznie aktualizować i utrzymywać za każdym razem, gdy dodajesz lub modyfikujesz typ elementu.

Trzymając się „rzeczywistych” funkcji wirtualnych, możesz zachować implementację zachowania każdego elementu w obrębie samego elementu. Możesz dodawać dodatkowe elementy w bardziej samodzielny i spójny sposób. A gdy to wszystko robisz, to kompilator będzie zajmował się implementacją dynamicznej wysyłki, a nie Ty.

Aby rozwiązać konkretny problem: próbujesz napisać prostą parę wirtualnych funkcji do aktualizacji ataku i obrony, ponieważ niektóre elementy wpływają tylko na atak, a niektóre tylko na obronę. Sztuczka w prostym przypadku takim jak ten, aby zaimplementować oba te zachowania, ale w niektórych przypadkach bez efektu. GetDefenseBonus()może wrócić 0lub ApplyDefenseBonus(int& defence)pozostawić defencebez zmian. To, jak sobie poradzisz, będzie zależeć od tego, jak chcesz poradzić sobie z innymi działaniami, które mają wpływ. W bardziej skomplikowanych przypadkach, w których zachowanie jest bardziej zróżnicowane, możesz po prostu połączyć działanie w jedną metodę.

* (Aczkolwiek transponowane względem typowej implementacji)

DeveloperInDevelopment
źródło
0

Posiadanie bloku kodu, który zna wszystkie możliwe „narzędzia”, nie jest świetnym projektem (zwłaszcza, że ​​skończysz z wieloma takimi blokami w kodzie); ale żadna z nich nie ma podstawowego Toolz kodami pośredniczącymi dla wszystkich możliwych właściwości narzędzia: teraz Toolklasa musi wiedzieć o wszystkich możliwych zastosowaniach.

Co każde narzędzie wie co to może przyczynić się do znaku, który go używa. Tak stanowią jedną metodę dla wszystkich narzędzi giveto(*Character owner). Dostosuje odpowiednio statystyki gracza, nie wiedząc, co mogą zrobić inne narzędzia, a co najlepsze, nie musi też wiedzieć o nieistotnych właściwościach postaci. Na przykład, tarcza nawet nie musi wiedzieć o atrybutach attack, invisibility, healthitd. Wszystko, co potrzebne, aby zastosować to narzędzie jest postać wspierać atrybuty, że obiekt wymaga. Jeśli spróbujesz dać osiołowi miecz, a osioł nie ma attackstatystyk, pojawi się błąd.

Narzędzia powinny również mieć remove()metodę, która odwraca ich wpływ na właściciela. Jest to trochę trudne (możliwe jest, aby skończyć z narzędziami, które pozostawiają niezerowy efekt, gdy zostaną podane, a następnie zabrane), ale przynajmniej są zlokalizowane dla każdego narzędzia.

Alexis
źródło
-4

Nie ma odpowiedzi, która mówi, że to nie pachnie, więc ja będę tą opinią; ten kod jest w porządku! Moja opinia opiera się na fakcie, że czasem łatwiej jest przejść dalej i pozwolić na stopniowe zwiększanie umiejętności w miarę tworzenia nowych rzeczy. Możesz utknąć na kilka dni w tworzeniu idealnej architektury, ale prawdopodobnie nikt nawet nie zobaczy go w akcji, ponieważ nigdy nie ukończyłeś projektu. Twoje zdrowie!

Ostmeistro
źródło
4
Oczywiście, poprawa umiejętności z osobistym doświadczeniem jest dobra. Ale poprawa umiejętności poprzez zadawanie pytań osobom, które mają już takie osobiste doświadczenie, abyś sam nie musiał wpaść w dziurę, jest mądrzejszy. Z pewnością dlatego ludzie zadają tu pytania, prawda?
Graham,
Nie zgadzam się Ale rozumiem, że w tej witrynie chodzi o głębokie wchodzenie. Czasami oznacza to nadmierne pedantyczne. Właśnie dlatego chciałem opublikować tę opinię, ponieważ jest zakotwiczona w rzeczywistości, a jeśli szukasz wskazówek, które pomogą ci stać się lepszym i pomóc początkującym, brakuje ci całego rozdziału na temat „wystarczająco dobrego”, który jest całkiem przydatny dla początkującego.
Ostmeistro,