É um cheiro de código armazenar objetos genéricos em um contêiner e, em seguida, obter um objeto e fazer o downcast dos objetos do contêiner?

Por exemplo, eu tenho um jogo, que possui algumas ferramentas para aumentar a capacidade do Player:

Tool.h

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

E algumas ferramentas:

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

E então um jogador pode segurar algumas ferramentas para ataque:

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

Eu acho que é difícil substituir if-elsepor métodos virtuais nas ferramentas, porque cada ferramenta possui propriedades diferentes e afeta o ataque e a defesa do jogador, para os quais a atualização do ataque e da defesa do jogador precisa ser feita dentro do objeto Player.

Mas não fiquei satisfeito com esse design, pois ele contém downcasting, com uma if-elsedeclaração longa . Esse design precisa ser "corrigido"? Se sim, o que posso fazer para corrigi-lo?

Sim, é um cheiro de código (em muitos casos).

Eu acho que é difícil substituir if-else por métodos virtuais em ferramentas

No seu exemplo, é bastante simples substituir o if / else por métodos virtuais:

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

Agora não há mais necessidade para o seu ifbloqueio, o chamador pode apenas usá-lo como

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

Obviamente, para situações mais complicadas, essa solução nem sempre é tão óbvia (mas, no entanto, quase sempre que possível). Mas se você chegar a uma situação em que não sabe resolver o caso com métodos virtuais, poderá fazer uma nova pergunta novamente aqui em "Programadores" (ou, se se tornar linguagem ou implementação específica, no Stackoverflow).

O principal problema com o seu código é que, sempre que você introduz um novo item, você não apenas precisa escrever e atualizar o código do item, mas também modificar o seu player (ou onde quer que o item seja usado), o que torna a coisa toda um muito mais complicado.

Como regra geral, eu acho que é sempre meio suspeito, quando você não pode confiar na subclasse / herança normal e precisa fazer o upcasting por conta própria.

Eu poderia pensar em duas abordagens possíveis para tornar tudo mais flexível:

• Como outros mencionaram, mova os membros attacke defensepara a classe base e simplesmente inicialize-os para 0. Isso também pode servir para verificar se você realmente consegue balançar o item para um ataque ou usá-lo para bloquear ataques.

• Crie algum tipo de sistema de retorno de chamada / evento. Existem diferentes abordagens possíveis para isso.

  Que tal mantê-lo simples?

  • Você pode criar membros da classe base como virtual void onEquip(Owner*) {}e virtual void onUnequip(Owner*) {}.
  • Suas sobrecargas seriam chamadas e modificariam as estatísticas ao (des) equipar o item, por exemplo, virtual void onEquip(Owner *o) { o->modifyStat("attack", attackValue); }e virtual void onUnequip(Owner *o) { o->modifyStat("attack", -attackValue); }.
  • As estatísticas podem ser acessadas de alguma maneira dinâmica, por exemplo, usando uma string curta ou uma constante como chave, para que você possa introduzir novos valores ou bônus específicos de equipamento que você não precisa necessariamente lidar com seu jogador ou "proprietário" especificamente.

  • Comparado a apenas solicitar os valores de ataque / defesa na hora certa, isso não apenas torna tudo mais dinâmico, mas também economiza chamadas desnecessárias e até permite criar itens que afetarão seu personagem permanentemente.

    Por exemplo, imagine um anel amaldiçoado que definirá algumas estatísticas ocultas uma vez equipadas, marcando seu personagem como amaldiçoado permanentemente.

Embora a @DocBrown tenha dado uma boa resposta, ela não vai longe o suficiente, imho. Antes de começar a avaliar as respostas, você deve avaliar suas necessidades. Do que você realmente precisa ?

Abaixo, mostrarei duas soluções possíveis, que oferecem vantagens diferentes para diferentes necessidades.

O primeiro é muito simplista e adaptado especificamente ao que você mostrou:

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

Isso permite serialização / desserialização muito fáceis de ferramentas (por exemplo, para salvar ou conectar em rede) e não precisa de despacho virtual. Se o seu código é tudo o que você mostrou e não espera que ele evolua muito mais, então, com mais ferramentas diferentes com nomes diferentes e essas estatísticas, apenas em quantidades diferentes, então este é o caminho a seguir.

A @DocBrown ofereceu uma solução que ainda depende do envio virtual e que pode ser uma vantagem se você, de alguma forma, especializar as ferramentas para partes do seu código que não foram mostradas. No entanto, se você realmente precisa ou deseja também alterar outro comportamento, sugiro a seguinte solução:

Composição sobre herança

E se você mais tarde quiser uma ferramenta que modifique a agilidade ? Ou correr velocidade ? Para mim, parece que você está fazendo um RPG. Uma coisa importante para os RPGs é estar aberta para extensão . As soluções mostradas até agora não oferecem isso. Você precisaria alterar a Toolclasse e adicionar novos métodos virtuais sempre que precisar de um novo atributo.

A segunda solução que estou mostrando é a que sugeri anteriormente em um comentário - ela usa composição em vez de herança e segue o princípio "fechado para modificação, aberto para extensão *. Se você estiver familiarizado com o funcionamento de sistemas de entidades, algumas coisas parecerá familiar (eu gosto de pensar em composição como o irmão menor do ES).

Observe que o que estou mostrando abaixo é significativamente mais elegante em linguagens que possuem informações sobre o tipo de tempo de execução, como Java ou C #. Portanto, o código C ++ que estou mostrando deve incluir algumas "escrituração contábil" que são simplesmente necessárias para fazer a composição funcionar bem aqui. Talvez alguém com mais experiência em C ++ seja capaz de sugerir uma abordagem ainda melhor.

Primeiro, olhamos novamente para o lado do chamador . No seu exemplo, você como o chamador dentro do attackmétodo não se importa com ferramentas. Você se preocupa com duas propriedades - pontos de ataque e defesa. Você realmente não se importa de onde eles vêm e não se importa com outras propriedades (por exemplo, velocidade de execução, agilidade).

Então, primeiro, apresentamos uma nova classe

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

E então, criamos nossos dois primeiros componentes

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

Posteriormente, tornamos uma ferramenta um conjunto de propriedades e tornamos as propriedades passíveis de consulta por outras pessoas.

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

Observe que, neste exemplo, eu só apoio ter um componente de cada tipo - isso facilita as coisas. Em teoria, você também pode permitir vários componentes do mesmo tipo, mas isso fica feio muito rápido. Um aspecto importante: Toolagora está fechado para modificação - nunca mais tocaremos na fonte Tool- mas aberto para extensão - podemos estender o comportamento de uma ferramenta modificando outras coisas e apenas passando outros componentes para ela.

Agora precisamos de uma maneira de recuperar ferramentas por tipos de componentes. Você ainda pode usar um vetor para ferramentas, assim como no seu exemplo de código:

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

Você também pode refatorar isso em sua própria Inventoryclasse e armazenar tabelas de pesquisa que simplificam bastante as ferramentas de recuperação por tipo de componente e evitam a repetição de toda a coleção.

Que vantagens tem essa abordagem? Em attack, você processa ferramentas que possuem dois componentes - você não se importa com mais nada.

Vamos imaginar que você tenha um walkTométodo, e agora você decide que é uma boa ideia se alguma ferramenta ganhar a capacidade de modificar sua velocidade de caminhada. Sem problemas!

Primeiro, crie o novo Component:

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

Em seguida, você simplesmente adiciona uma instância desse componente à ferramenta que deseja aumentar sua velocidade de ativação e altera o WalkTométodo para processar o componente que você acabou de criar:

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

Observe que adicionamos algum comportamento às nossas ferramentas sem modificar a classe Tools.

Você pode (e deve) mover as strings para uma macro ou variável estática const, para não precisar digitá-lo novamente.

Se você levar essa abordagem adiante - por exemplo, faça componentes que possam ser adicionados ao jogador e faça um Combatcomponente que sinalize o jogador como capaz de participar de combate, você também poderá se livrar do attackmétodo e deixar que ele seja manipulado. pelo componente ou ser processado em outro local.

A vantagem de tornar o jogador capaz de obter componentes também seria que, então, você nem precisaria mudar o jogador para ter um comportamento diferente. No meu exemplo, você pode criar um Movablecomponente para que não precise implementar o walkTométodo no player para fazê-lo se mover. Você apenas criaria o componente, anexá-lo ao player e permitir que outra pessoa o processasse.

Você pode encontrar um exemplo completo nesta lista: https://gist.github.com/NetzwergX/3a29e1b106c6bb9c7308e89dd715ee20

Esta solução é obviamente um pouco mais complexa que as outras postadas. Mas, dependendo de quão flexível você queira ser, até onde você quer levá-lo, essa pode ser uma abordagem muito poderosa.

Editar

Algumas outras respostas propõem herança direta (Fazendo as espadas estenderem a Ferramenta, fazendo o Shield estender a Ferramenta). Não acho que esse seja um cenário em que a herança funcione muito bem. E se você decidir que bloquear com um escudo de uma certa maneira também pode danificar o atacante? Com a minha solução, você pode simplesmente adicionar um componente de ataque a um escudo e perceber isso sem nenhuma alteração no seu código. Com herança, você teria um problema. itens / ferramentas em RPGs são os principais candidatos à composição ou até mesmo diretamente usando sistemas de entidades desde o início.

De um modo geral, se você precisar usar if(em combinação com a exigência do tipo de instância) em qualquer idioma OOP, isso é um sinal de que algo está acontecendo. Pelo menos, você deve dar uma olhada em seus modelos.

Eu modelaria seu domínio de maneira diferente.

Para o seu caso, a Toolpossui um AttackBonuse um DefenseBonus- o que poderia ser um 0caso inútil para lutar como penas ou algo assim.

Para um ataque, você tem o seu baserate+ bonusda arma usada. O mesmo vale para defesa baserate+ bonus.

Em conseqüência, você Toolprecisa ter um virtualmétodo para calcular os bônus de ataque / defesa.

tl; dr

Com um design melhor, você pode evitar hacky ifs.

Como está escrito, "cheira", mas esses podem ser apenas os exemplos que você deu. Armazenar dados em contêineres de objetos genéricos e depois convertê-los para obter acesso aos dados não é automaticamente um cheiro de código. Você o verá usado em muitas situações. No entanto, quando você o usa, deve estar ciente do que está fazendo, como está fazendo e por quê. Quando olho para o exemplo, o uso de comparações baseadas em seqüências de caracteres para me dizer qual objeto é o que aciona meu medidor de cheiro pessoal. Isso sugere que você não tem muita certeza do que está fazendo aqui (o que é bom, pois você teve a sabedoria de vir aqui para os programadores. SE e diga "ei, eu não acho que gosto do que estou fazendo, ajude estou fora!").

A questão fundamental com o padrão de transmissão de dados de contêineres genéricos como esse é que o produtor dos dados e o consumidor dos dados devem trabalhar juntos, mas pode não ser óbvio que eles o façam à primeira vista. Em todos os exemplos desse padrão, fedorento ou não fedido, esta é a questão fundamental. É muito possível que o próximo desenvolvedor não saiba que você está executando esse padrão e quebre-o acidentalmente; portanto, se você usar esse padrão, tenha cuidado para ajudar o próximo desenvolvedor. Você precisa facilitar a quebra do código acidentalmente devido a alguns detalhes que ele talvez não saiba que existem.

Por exemplo, e se eu quisesse copiar um jogador? Se eu apenas olhar o conteúdo do objeto player, parece bem fácil. Eu só tenho que copiar os attack, defensee toolsvariáveis. Fácil como torta! Bem, descobrirei rapidamente que o uso de ponteiros torna um pouco mais difícil (em algum momento, vale a pena olhar para ponteiros inteligentes, mas esse é outro tópico). Isso é facilmente resolvido. Vou apenas criar novas cópias de cada ferramenta e colocá-las na minha nova toolslista. Afinal, Toolé uma aula realmente simples, com apenas um membro. Então, eu crio um monte de cópias, incluindo uma cópia do Sword, mas eu não sabia que era uma espada, então apenas copiei o name. Mais tarde, a attack()função olha para o nome, vê que é uma "espada", lança-a e coisas ruins acontecem!

Podemos comparar esse caso com outro caso na programação de soquetes, que usa o mesmo padrão. Eu posso configurar uma função de soquete UNIX assim:

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

Por que esse é o mesmo padrão? Como bindnão aceita um sockaddr_in*, ele aceita um mais genérico sockaddr*. Se você observar as definições dessas classes, veremos que sockaddrhá apenas um membro da família à qual designamos sin_family*. A família diz em qual subtipo você deve usar o sockaddr. AF_INETinforma que a estrutura de endereço é realmente a sockaddr_in. Se fosse AF_INET6, o endereço seria a sockaddr_in6, que possui campos maiores para suportar os endereços IPv6 maiores.

Isso é idêntico ao seu Toolexemplo, exceto que ele usa um número inteiro para especificar qual família, em vez de a std::string. No entanto, vou afirmar que não tem cheiro, e tentarei fazê-lo por outras razões além de "é uma maneira padrão de fazer soquetes, para que não cheire". Obviamente é o mesmo padrão, que é por que afirmo que armazenar dados em objetos genéricos e convertê-los não é automaticamente um cheiro de código, mas há algumas diferenças em como eles fazem isso, o que o torna mais seguro.

Ao usar esse padrão, as informações mais importantes são capturar o transporte de informações sobre a subclasse do produtor para o consumidor. É isso que você está fazendo com o namecampo e os soquetes UNIX fazem com o sin_familycampo deles . Esse campo é a informação que o consumidor precisa para entender o que o produtor realmente criou. Em todos os casos desse padrão, deve ser uma enumeração (ou pelo menos um número inteiro que atua como uma enumeração). Por quê? Pense no que seu consumidor fará com as informações. Eles precisam ter escrito uma grande ifdeclaração ou umswitch, como você determinou, onde eles determinam o subtipo correto, o convertem e usam os dados. Por definição, pode haver apenas um pequeno número desses tipos. Você pode armazená-lo em uma string, como você fez, mas isso tem inúmeras desvantagens:

• Lento - std::stringnormalmente precisa fazer alguma memória dinâmica para manter a string. Você também precisa fazer uma comparação de texto completo para corresponder ao nome toda vez que quiser descobrir qual subclasse você possui.
• Muito versátil - há algo a ser dito para colocar restrições em si mesmo quando você está fazendo algo extremamente perigoso. Eu tive sistemas como este que procuravam uma substring para dizer a que tipo de objeto ele estava olhando. Isso funcionou muito bem até que o nome de um objeto conteve acidentalmente essa substring e criou um erro terrivelmente enigmático. Como, como declaramos acima, precisamos apenas de um pequeno número de casos, não há razão para usar uma ferramenta massivamente sobrecarregada, como strings. Isto leva a...
• Propenso a erros - digamos apenas que você desejará entrar em um tumulto assassino tentando depurar por que as coisas não estão funcionando quando um consumidor acidentalmente define o nome de um pano mágico MagicC1oth. Sério, erros como esse podem levar dias para coçar a cabeça antes que você percebesse o que aconteceu.

Uma enumeração funciona muito melhor. É rápido, barato e muito menos propenso a erros:

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

Este exemplo também mostra uma switchdeclaração envolvendo as enumerações, com a parte mais importante desse padrão: um defaultcaso que é lançado. Você nunca deve entrar nessa situação se fizer as coisas perfeitamente. No entanto, se alguém adicionar um novo tipo de ferramenta e você esquecer de atualizar seu código para suportá-lo, precisará de algo para detectar o erro. Na verdade, eu recomendo tanto que você os adicione, mesmo que não precise deles.

A outra grande vantagem enumdisso é que ele fornece ao próximo desenvolvedor uma lista completa de tipos de ferramentas válidos, desde o início. Não há necessidade de vasculhar o código para encontrar a classe especializada de flauta de Bob que ele usa em sua batalha épica com chefes.

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

Sim, coloquei uma declaração padrão "vazia", ​​apenas para explicitar ao próximo desenvolvedor o que eu espero que aconteça se algum novo tipo inesperado aparecer no meu caminho.

Se você fizer isso, o padrão cheira menos. No entanto, para não sentir cheiro, a última coisa que você precisa fazer é considerar as outras opções. Esses lançamentos são algumas das ferramentas mais poderosas e perigosas que você tem no repertório C ++. Você não deve usá-los, a menos que tenha um bom motivo.

Uma alternativa muito popular é o que chamo de "estrutura de união" ou "classe de união". Para o seu exemplo, isso seria realmente um ajuste muito bom. Para criar um desses, você cria uma Toolclasse, com uma enumeração como antes, mas, em vez de subclassificar Tool, colocamos todos os campos de todos os subtipos nela.

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

    int   attack;
    int   defense;
};

Agora você não precisa de subclasses. Você só precisa olhar para o typecampo para ver quais outros campos são realmente válidos. Isso é muito mais seguro e fácil de entender. No entanto, tem desvantagens. Há momentos em que você não deseja usar isso:

• Quando os objetos são muito diferentes - Você pode acabar com uma lista de campos completa e não fica claro quais se aplicam a cada tipo de objeto.
• Ao operar em uma situação crítica de memória - Se você precisar criar 10 ferramentas, poderá ficar preguiçoso com a memória. Quando você precisar criar 500 milhões de ferramentas, começará a se preocupar com bits e bytes. As estruturas de união são sempre maiores do que precisam.

Essa solução não é usada pelos soquetes UNIX devido ao problema de dissimulação composto pelo limite aberto da API. A intenção com soquetes UNIX era criar algo com o qual todos os sabores do UNIX pudessem trabalhar. Cada sabor poderia definir a lista de famílias que eles sustentam AF_INET, e haveria uma pequena lista para cada um. No entanto, se um novo protocolo aparecer, como AF_INET6aconteceu, talvez você precise adicionar novos campos. Se você fizesse isso com uma estrutura de união, acabaria criando efetivamente uma nova versão da estrutura com o mesmo nome, criando infinitos problemas de incompatibilidade. É por isso que os soquetes do UNIX optaram por usar o padrão de conversão em vez de uma estrutura de união. Tenho certeza de que eles consideraram e o fato de pensarem sobre isso faz parte do motivo pelo qual não cheira quando o usam.

Você também pode usar uma união de verdade. Os sindicatos economizam memória, sendo tão maiores quanto o maior membro, mas eles vêm com seu próprio conjunto de problemas. Provavelmente, essa não é uma opção para o seu código, mas sempre é uma opção que você deve considerar.

Outra solução interessante é boost::variant. O Boost é uma ótima biblioteca cheia de soluções reutilizáveis ​​entre plataformas. Provavelmente é um dos melhores códigos C ++ já escritos. Boost.Variant é basicamente a versão C ++ dos sindicatos. É um contêiner que pode conter muitos tipos diferentes, mas apenas um de cada vez. Você poderia fazer o seu Sword, Shielde por um pouco!), Mas este padrão pode ser extremamente útil. A variante é frequentemente usada, por exemplo, em árvores de análise, que pegam uma sequência de texto e a dividem usando uma gramática para regras.MagicCloth as classes, em seguida, fazer ferramenta ser um boost::variant<Sword, Shield, MagicCloth>, o que significa que contém um desses três tipos. Isso ainda sofre com o mesmo problema com compatibilidade futura que impede os soquetes UNIX de usá-lo (para não mencionar os soquetes UNIX são C, anterioresboost

A solução final que eu recomendaria analisar antes de mergulhar e usar a abordagem genérica de conversão de objetos é o padrão de design do Visitor . Visitor é um poderoso padrão de design que tira proveito da observação de que chamar uma função virtual efetivamente faz a conversão necessária e faz isso para você. Como o compilador faz isso, nunca pode estar errado. Assim, em vez de armazenar uma enumeração, o Visitor usa uma classe base abstrata, que possui uma tabela que sabe que tipo de objeto é. Em seguida, criamos uma chamada simples e indireta, que faz o trabalho:

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

Então, qual é esse padrão horrível de Deus? Bem, Tooltem uma função virtual accept,. Se você passar um visitante, espera-se que ele retorne e chame a visitfunção correta nesse visitante para o tipo. Isso é o que visitor.visit(*this);faz em cada subtipo. Complicado, mas podemos mostrar isso com o seu exemplo acima:

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
}

Então o que acontece aqui? Criamos um visitante que fará algum trabalho para nós, uma vez que ele sabe que tipo de objeto está visitando. Em seguida, iteramos sobre a lista de ferramentas. Por uma questão de argumento, digamos que o primeiro objeto seja a Shield, mas nosso código ainda não sabe disso. Chama t->accept(v), uma função virtual. Como o primeiro objeto é um escudo, ele acaba chamando void Shield::accept(ToolVisitor& visitor), que chama visitor.visit(*this);. Agora, quando procuramos por quem visitchamar, já sabemos que temos um Shield (porque essa função foi chamada), por isso, acabamos chamando void ToolVisitor::visit(Shield* shield)nosso AttackVisitor. Agora, o código correto é executado para atualizar nossa defesa.

O visitante é volumoso. É tão desajeitado que quase acho que tem um cheiro próprio. É muito fácil escrever padrões de visitantes ruins. No entanto, ele tem uma enorme vantagem que nenhum dos outros possui. Se adicionarmos um novo tipo de ferramenta, precisamos adicionar uma nova ToolVisitor::visitfunção para ele. No instante em que fazemos isso, todos ToolVisitor os programas se recusam a compilar porque está faltando uma função virtual. Isso facilita muito a captura de todos os casos em que perdemos alguma coisa. É muito mais difícil garantir isso se você usar ifou switchinstruções para fazer o trabalho. Essas vantagens são boas o suficiente para que o Visitor tenha encontrado um pequeno nicho nos geradores de cenas gráficas em 3D. Por acaso, eles precisam exatamente do tipo de comportamento que o Visitor oferece, por isso funciona muito bem!

Ao todo, lembre-se de que esses padrões tornam difícil o próximo desenvolvedor. Gaste tempo facilitando para eles, e o código não cheira!

* Tecnicamente, se você olhar as especificações, sockaddr tem um membro chamado sa_family. Há algumas coisas complicadas sendo feitas aqui no nível C que não importam para nós. Você pode analisar a implementação real , mas para esta resposta eu vou usar sa_family sin_familye outras de forma totalmente intercambiável, usando a que for mais intuitiva para a prosa, confiando que essa manobra em C cuide dos detalhes sem importância.

Em geral, evito implementar várias classes / herança, se for apenas para comunicar dados. Você pode manter uma única classe e implementar tudo a partir daí. Para o seu exemplo, isso é suficiente

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

Provavelmente, você está antecipando que seu jogo implementará vários tipos de espadas, etc., mas você terá outras maneiras de implementá-lo. Explosão de classe raramente é a melhor arquitetura. Mantenha simples.

Como afirmado anteriormente, esse é um cheiro sério de código. No entanto, pode-se considerar a fonte do seu problema usando herança em vez de composição em seu design.

Por exemplo, dado o que você nos mostrou, você claramente tem 3 conceitos:

• Item
• Item que pode ter ataque.
• Item que pode ter defesa.

Observe que sua quarta classe é apenas uma combinação dos dois últimos conceitos. Então, eu sugiro usar a composição para isso.

Você precisa de uma estrutura de dados para representar as informações necessárias para o ataque. E você precisa de uma estrutura de dados representando as informações necessárias para defesa. Por fim, você precisa de uma estrutura de dados para representar itens que podem ou não ter uma ou ambas as propriedades:

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

Por que não criar métodos abstratos modifyAttacke modifyDefensena Toolclasse? Então cada criança teria sua própria implementação, e você chama isso de maneira elegante:

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

Passar valores como referência economizará recursos se você puder:

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

Se alguém usa polimorfismo, é sempre melhor que todo o código que se preocupa com a classe usada seja dentro da própria classe. É assim que eu codificaria:

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

Isso tem as seguintes vantagens:

• mais fácil adicionar novas classes: você só precisa implementar todos os métodos abstratos e o restante do código funciona
• mais fácil remover classes
• mais fácil adicionar novas estatísticas (as classes que não se importam com as estatísticas simplesmente ignoram)

Eu acho que uma maneira de reconhecer as falhas nessa abordagem é desenvolver sua idéia até a conclusão lógica.

Parece um jogo, então, em algum momento, você provavelmente começará a se preocupar com o desempenho e trocará essas comparações de strings por um intou enum. À medida que a lista de itens fica mais longa, isso if-elsecomeça a ficar bastante pesado, então você pode refatorá-la para a switch-case. Você também tem uma parede de texto neste momento, para poder dividir a ação em cada uma delas caseem uma função separada.

Quando você chega nesse ponto, a estrutura do seu código começa a parecer familiar - começa a parecer uma tabela de mesa homebrew * - a estrutura básica na qual os métodos virtuais são tipicamente implementados. Exceto que esta é uma tabela que você deve atualizar e manter manualmente, sempre que adicionar ou modificar um tipo de item.

Ao aderir a funções virtuais "reais", você é capaz de manter a implementação do comportamento de cada item dentro do próprio item. Você pode adicionar itens adicionais de uma maneira mais independente e consistente. E enquanto você faz tudo isso, é o compilador que cuidará da implementação do seu envio dinâmico, e não você.

Para resolver seu problema específico: você está tentando escrever um par simples de funções virtuais para atualizar o ataque e a defesa, porque alguns itens afetam apenas o ataque e outros afetam a defesa. O truque em um caso simples como esse para implementar ambos os comportamentos de qualquer maneira, mas sem efeito em determinados casos. GetDefenseBonus()pode retornar 0ou ApplyDefenseBonus(int& defence)apenas deixar defenceinalterado. A maneira como você vai fazer isso dependerá de como você deseja lidar com as outras ações que têm efeito. Em casos mais complexos, onde há comportamento mais variado, você pode simplesmente combinar a atividade em um único método.

_{* (Embora, transposto em relação à implementação típica)}

Ter um bloco de código que conheça todas as "ferramentas" possíveis não é um ótimo design (principalmente porque você terá muitos desses blocos no código); mas também não há um básico Toolcom stubs para todas as propriedades possíveis da ferramenta: agora a Toolclasse deve conhecer todos os usos possíveis.

O que cada ferramenta sabe é o que pode contribuir para o personagem que a usa. Portanto, forneça um método para todas as ferramentas giveto(*Character owner),. Ele ajustará as estatísticas do jogador conforme apropriado, sem saber o que outras ferramentas podem fazer e, o que é melhor, também não precisa saber sobre propriedades irrelevantes do personagem. Por exemplo, um escudo nem precisa de saber sobre os atributos attack, invisibility, healthetc. Tudo o que é necessário para aplicar uma ferramenta é para o personagem para apoiar os atributos que o objeto requer. Se você tentar dar uma espada a um burro, e o burro não tiver attackestatísticas, você receberá um erro.

As ferramentas também devem ter um remove()método que inverta seu efeito no proprietário. Isso é um pouco complicado (é possível acabar com ferramentas que deixam um efeito diferente de zero quando são dadas e tiradas), mas pelo menos é localizado em cada ferramenta.

Não há resposta que diga que não cheira, então eu serei a pessoa que defende essa opinião; esse código é totalmente bom! Minha opinião é baseada no fato de que, às vezes, é mais fácil seguir em frente e permitir que suas habilidades aumentem gradualmente à medida que você cria mais coisas novas. Você pode ficar parado por dias criando uma arquitetura perfeita, mas provavelmente ninguém nunca a verá em ação, porque você nunca terminou o projeto. Felicidades!