¿Es un olor a código almacenar objetos genéricos en un contenedor y luego obtener objetos y rechazar los objetos del contenedor?

Por ejemplo, tengo un juego que tiene algunas herramientas para aumentar la capacidad del jugador:

Tool.h

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

Y algunas herramientas:

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

Y luego un jugador puede tener algunas herramientas para atacar:

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

Creo que es difícil reemplazarlo if-elsecon métodos virtuales en las herramientas, porque cada herramienta tiene propiedades diferentes, y cada herramienta afecta el ataque y la defensa del jugador, por lo que la actualización del ataque y la defensa del jugador debe hacerse dentro del objeto Jugador.

Pero no estaba satisfecho con este diseño, ya que contiene downcasting, con una larga if-elsedeclaración. ¿Este diseño necesita ser "corregido"? Si es así, ¿qué puedo hacer para corregirlo?

Sí, es un olor a código (en muchos casos).

Creo que es difícil reemplazar if-else con métodos virtuales en herramientas

En su ejemplo, es bastante simple reemplazar el if / else por métodos virtuales:

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

Ahora ya no hay necesidad de ifbloquear, la persona que llama puede usarlo como

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

Por supuesto, para situaciones más complicadas, tal solución no siempre es tan obvia (pero, sin embargo, casi siempre es posible). Pero si se encuentra con una situación en la que no sabe cómo resolver el caso con métodos virtuales, puede hacer una nueva pregunta nuevamente aquí en "Programadores" (o, si se convierte en lenguaje o implementación específica, en Stackoverflow).

El principal problema con su código es que cada vez que introduce un elemento nuevo, no solo tiene que escribir y actualizar el código del elemento, también tiene que modificar su reproductor (o donde sea que se use el elemento), lo que hace que todo sea un Mucho más complicado.

Como regla general, creo que siempre es un poco sospechoso, cuando no puede confiar en la subclasificación / herencia normal y tiene que hacer la transmisión usted mismo.

Podría pensar en dos posibles enfoques que hacen que todo sea más flexible:

• Como otros mencionaron, mueva los miembros attacky defensea la clase base y simplemente inicialícelos 0. Esto también podría funcionar como un control si realmente puedes balancear el objeto para un ataque o usarlo para bloquear ataques.

• Crear algún tipo de devolución de llamada / sistema de eventos. Hay diferentes enfoques posibles para esto.

  ¿Qué tal mantenerlo simple?

  • Puede crear una clase base de miembros como virtual void onEquip(Owner*) {}y virtual void onUnequip(Owner*) {}.
  • Sus sobrecargas se llamarían y modificarían las estadísticas al (des) equipar el elemento, por ejemplo, virtual void onEquip(Owner *o) { o->modifyStat("attack", attackValue); }y virtual void onUnequip(Owner *o) { o->modifyStat("attack", -attackValue); }.
  • Se puede acceder a las estadísticas de forma dinámica, por ejemplo, utilizando una cadena corta o una constante como clave, por lo que incluso puede introducir nuevos valores específicos de equipo o bonos que no necesariamente tiene que manejar en su jugador o "propietario" específicamente.

  • En comparación con solo solicitar los valores de ataque / defensa justo a tiempo, esto no solo hace que todo sea más dinámico, sino que también le ahorra llamadas innecesarias e incluso le permite crear elementos que afectarán a su personaje de forma permanente.

    Por ejemplo, imagina un anillo maldito que solo establecerá una estadística oculta una vez equipado, marcando a tu personaje como maldito permanentemente.

Si bien @DocBrown ha dado una buena respuesta, no va lo suficientemente lejos, en mi humilde opinión. Antes de comenzar a evaluar las respuestas, debe evaluar sus necesidades. ¿Qué es lo que realmente necesitas ?

A continuación, mostraré dos posibles soluciones, que ofrecen diferentes ventajas para diferentes necesidades.

El primero es muy simplista y se adapta específicamente a lo que ha mostrado:

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

Esto permite muy una serialización / deserialización fácil de herramientas (por ejemplo, para guardar o establecer redes), y no necesita ningún envío virtual. Si su código es todo lo que ha mostrado, y no espera que evolucione mucho más que tener más herramientas diferentes con diferentes nombres y esas estadísticas, solo en diferentes cantidades, entonces este es el camino a seguir.

@DocBrown ha ofrecido una solución que aún se basa en el envío virtual, y que puede ser una ventaja si de alguna manera especializa las herramientas para partes de su código que no se muestran. Sin embargo, si realmente necesita o desea cambiar también otro comportamiento, sugeriría la siguiente solución:

Composición sobre herencia

¿Qué pasa si luego quieres una herramienta que modifique la agilidad ? O correr la velocidad ? Para mí, parece que estás haciendo un juego de rol. Una cosa que es importante para los juegos de rol es estar abierto a la extensión . Las soluciones mostradas hasta ahora no ofrecen eso. Tendrías que alterar elTool clase y agregarle nuevos métodos virtuales cada vez que necesite un nuevo atributo.

La segunda solución que estoy mostrando es la que indiqué anteriormente en un comentario: utiliza la composición en lugar de la herencia y sigue el principio de "cerrado por modificación, abierto por extensión *. Si está familiarizado con el funcionamiento de los sistemas de entidades, algunas cosas me resultará familiar (me gusta pensar en la composición como el hermano menor de ES).

Tenga en cuenta que lo que muestro a continuación es significativamente más elegante en lenguajes que tienen información de tipo de tiempo de ejecución, como Java o C #. Por lo tanto, el código C ++ que estoy mostrando tiene que incluir alguna "contabilidad" que simplemente es necesaria para que la composición funcione aquí. Quizás alguien con más experiencia en C ++ pueda sugerir un enfoque aún mejor.

Primero, volvemos a mirar al lado de la persona que llama . En su ejemplo, a usted como la persona que llama dentro del attackmétodo no le importan las herramientas. Lo que le importa son dos propiedades: puntos de ataque y defensa. Realmente no te importa de dónde provienen, y no te importan otras propiedades (por ejemplo, velocidad de carrera, agilidad).

Primero, presentamos una nueva clase

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

Y luego, creamos nuestros dos primeros 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;
};

Luego, hacemos que una herramienta contenga un conjunto de propiedades, y hacemos que las propiedades sean consultables por otros.

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

Tenga en cuenta que en este ejemplo, solo apoyo tener un componente de cada tipo, esto facilita las cosas. En teoría, también podría permitir múltiples componentes del mismo tipo, pero eso se vuelve feo muy rápido. Un aspecto importante: Toolahora está cerrado por modificación , nunca tocaremos la fuente de Toolnuevo, pero abierto por extensión , podemos extender el comportamiento de una Herramienta modificando otras cosas y simplemente pasando otros Componentes a ella.

Ahora necesitamos una forma de recuperar herramientas por tipos de componentes. Todavía podría usar un vector para herramientas, al igual que en su ejemplo 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 << "!";
        }
};

También puedes refactorizar esto en tu propio Inventory clase, y almacenar tablas de búsqueda que simplifiquen enormemente las herramientas de recuperación por tipo de componente y evite iterar sobre toda la colección una y otra vez.

¿Qué ventajas tiene este enfoque? En attack, procesa herramientas que tienen dos componentes: no le importa nada más.

Imaginemos que tiene un walkTométodo, y ahora decide que es una buena idea si alguna herramienta podría modificar su velocidad de marcha. ¡No hay problema!

Primero, crea el nuevo Component:

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

Luego, simplemente agregue una instancia de este componente a la herramienta que desea para aumentar su velocidad de activación y cambie el WalkTométodo para procesar el componente que acaba de crear:

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

Tenga en cuenta que agregamos algún comportamiento a nuestras Herramientas sin modificar la clase Herramientas en absoluto.

Puede (y debe) mover las cadenas a una macro o variable de constante estática, por lo que no tiene que escribirla una y otra vez.

Si lleva este enfoque más allá, por ejemplo, crea componentes que se pueden agregar al jugador y crea un Combatcomponente que indique que el jugador puede participar en el combate, entonces también puede deshacerse del attackmétodo y dejar que se maneje por el Componente o ser procesado en otro lugar.

La ventaja de hacer que el jugador también pueda obtener Componentes, sería que entonces, ni siquiera necesitaría cambiar el jugador para darle un comportamiento diferente. En mi ejemplo, podrías crear un Movablecomponente, de esa manera no necesitas implementar el walkTométodo en el jugador para que se mueva. Simplemente creará el componente, lo adjuntará al reproductor y dejará que alguien más lo procese.

Puede encontrar un ejemplo completo en este resumen: https://gist.github.com/NetzwergX/3a29e1b106c6bb9c7308e89dd715ee20

Obviamente, esta solución es un poco más compleja que las otras que se han publicado. Pero dependiendo de qué tan flexible quiera ser, qué tan lejos quiera llegar, este puede ser un enfoque muy poderoso.

Editar

Algunas otras respuestas proponen herencia directa (Hacer que las espadas extiendan la Herramienta, hacer que el Escudo extienda la Herramienta). No creo que este sea un escenario en el que la herencia funcione muy bien. ¿Qué sucede si decides que bloquear con un escudo de cierta manera también puede dañar al atacante? Con mi solución, simplemente podría agregar un componente de ataque a un escudo y darse cuenta de eso sin ningún cambio en su código. Con la herencia, tendrías un problema. Los elementos / herramientas en los juegos de rol son los principales candidatos para la composición o incluso directamente utilizando sistemas de entidades desde el principio.

En términos generales, si alguna vez tiene la necesidad de usar if (en combinación con requerir el tipo de una instancia) en cualquier lenguaje OOP, eso es una señal de que algo malo está sucediendo. Al menos, deberías echar un vistazo más de cerca a tus modelos.

Modelaría su dominio de manera diferente.

Para su caso de uso, a Tooltiene un AttackBonusy a DefenseBonus, que podrían ser ambos 0en caso de que sea inútil para luchar como plumas o algo así.

Para un ataque, tienes tu baserate+ bonusdel arma utilizada. Lo mismo vale para la defensa baserate+ bonus.

En consecuencia, Tooldebe tener un virtualmétodo para calcular el boni de ataque / defensa.

tl; dr

Con un mejor diseño, podría evitar hacky ifs.

Tal como está escrito, "huele", pero eso podría ser solo los ejemplos que dio. Almacenar datos en contenedores de objetos genéricos, luego enviarlos para obtener acceso a los datos no es automáticamente código de olor. Lo verás usado en muchas situaciones. Sin embargo, cuando lo use, debe saber qué está haciendo, cómo lo está haciendo y por qué. Cuando miro el ejemplo, el uso de comparaciones basadas en cadenas para decirme qué objeto es lo que dispara mi medidor de olor personal. Sugiere que no estás completamente seguro de lo que estás haciendo aquí (lo cual está bien, ya que tuviste la sabiduría de venir aquí a los programadores. SE y decir "oye, no creo que me guste lo que estoy haciendo, ayuda ¡Sacarme!").

El problema fundamental con el patrón de enviar datos desde contenedores genéricos como este es que el productor de los datos y el consumidor de los datos deben trabajar juntos, pero puede no ser obvio que lo hagan a primera vista. En cada ejemplo de este patrón, maloliente o no, este es el problema fundamental. Es muy posible que el próximo desarrollador ignore por completo que está haciendo este patrón y lo rompa por accidente, por lo que si usa este patrón debe tener cuidado de ayudar al próximo desarrollador. Debe facilitarle que no rompa el código involuntariamente debido a algunos detalles que tal vez no sepa que existen.

Por ejemplo, ¿qué pasa si quisiera copiar un reproductor? Si solo miro el contenido del objeto reproductor, parece bastante fácil. Sólo tengo que copiar los attack, defensey toolslas variables. ¡Muy fácil! Bueno, descubriré rápidamente que su uso de punteros lo hace un poco más difícil (en algún momento, vale la pena mirar los punteros inteligentes, pero ese es otro tema). Eso se resuelve fácilmente. Simplemente crearé nuevas copias de cada herramienta y las pondré en mi nueva toolslista. Después de todo, Tooles una clase realmente simple con un solo miembro. Así que creo un montón de copias, incluida una copia de la Sword, pero no sabía que era una espada, así que solo copié la name. Más tarde, la attack()función mira el nombre, ve que es una "espada", lo lanza, ¡y suceden cosas malas!

Podemos comparar este caso con otro caso en la programación de sockets, que usa el mismo patrón. Puedo configurar una función de socket UNIX como esta:

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 qué es este el mismo patrón? Porque bindno acepta un sockaddr_in*, acepta un más genérico sockaddr*. Si observa las definiciones para esas clases, vemos que sockaddrsolo tiene un miembro de la familia que le asignamos sin_family*. La familia dice a qué subtipo se debe enviar el sockaddr. AF_INETle dice que la estructura de la dirección es en realidad un sockaddr_in. Si lo fuera AF_INET6, la dirección sería a sockaddr_in6, que tiene campos más grandes para admitir las direcciones IPv6 más grandes.

Esto es idéntico a su Toolejemplo, excepto que usa un número entero para especificar qué familia en lugar de a std::string. Sin embargo, voy a afirmar que no huele, e intentaré hacerlo por otras razones que no sean "es una forma estándar de hacer enchufes, por lo que no debe" oler ". Obviamente es el mismo patrón, que es Por eso afirmo que almacenar datos en objetos genéricos y emitirlos no es automáticamente olor a código, pero hay algunas diferencias en cómo lo hacen que lo hacen más seguro.

Cuando se usa este patrón, la información más importante es capturar la transmisión de información sobre la subclase de productor a consumidor. Esto es lo que está haciendo con el namecampo y los sockets UNIX hacen con su sin_familycampo. Ese campo es la información que el consumidor necesita para comprender lo que el productor realmente había creado. En todos los casos de este patrón, debe ser una enumeración (o al menos, un número entero que actúa como una enumeración). ¿Por qué? Piense en lo que su consumidor hará con la información. Necesitarán haber escrito algo grandeif declaración o unswitchdeclaración, como lo hizo, donde determinan el subtipo correcto, lo convierten y usan los datos. Por definición, solo puede haber un pequeño número de estos tipos. Puede almacenarlo en una cadena, como lo hizo, pero eso tiene numerosas desventajas:

• Lento: std::stringnormalmente tiene que hacer algo de memoria dinámica para mantener la cadena. También debe hacer una comparación de texto completo para que coincida con el nombre cada vez que desee averiguar qué subclase tiene.
• Demasiado versátil: hay algo que decir para imponer restricciones cuando haces algo extremadamente peligroso. He tenido sistemas como este que buscaban una subcadena para decirle qué tipo de objeto estaba mirando. Esto funcionó muy bien hasta que el nombre de un objeto accidentalmente contuvo esa subcadena y creó un error terriblemente críptico. Como, como dijimos anteriormente, solo necesitamos un pequeño número de casos, no hay razón para usar una herramienta masivamente poderosa como cadenas. Esto lleva a...
• Propenso a errores: digamos que querrá ir a un alboroto asesino tratando de depurar por qué las cosas no funcionan cuando un consumidor accidentalmente establece el nombre de una tela mágica MagicC1oth. En serio, errores como ese pueden tomar días de rascarse la cabeza antes de que te des cuenta de lo que sucedió.

Una enumeración funciona mucho mejor. Es rápido, barato y mucho menos propenso a errores:

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 ejemplo también muestra una switchdeclaración que involucra las enumeraciones, con la parte más importante de este patrón: un defaultcaso que arroja. Usted debe no estar en esa situación si haces las cosas a la perfección. Sin embargo, si alguien agrega un nuevo tipo de herramienta y olvida actualizar su código para admitirlo, querrá que algo detecte el error. De hecho, los recomiendo tanto que debería agregarlos incluso si no los necesita.

La otra gran ventaja de esto enumes que le da al siguiente desarrollador una lista completa de tipos de herramientas válidas, desde el principio. No hay necesidad de leer el código para encontrar la clase de flauta especializada de Bob que usa en su épica batalla de jefes.

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

Sí, puse una declaración predeterminada "vacía", solo para hacer explícito al próximo desarrollador lo que espero que suceda si algún nuevo tipo inesperado me llega.

Si haces esto, el patrón olerá menos. Sin embargo, para estar libre de olores, lo último que debe hacer es considerar las otras opciones. Estos lanzamientos son algunas de las herramientas más poderosas y peligrosas que tiene en el repertorio de C ++. No debe usarlos a menos que tenga una buena razón.

Una alternativa muy popular es lo que yo llamo una "estructura sindical" o "clase sindical". Para su ejemplo, esto realmente encajaría muy bien. Para crear uno de estos, crea una Toolclase, con una enumeración como antes, pero en lugar de subclasificar Tool, simplemente ponemos todos los campos de cada subtipo.

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

    int   attack;
    int   defense;
};

Ahora no necesitas subclases en absoluto. Solo tiene que mirar el typecampo para ver qué otros campos son realmente válidos. Esto es mucho más seguro y más fácil de entender. Sin embargo, tiene inconvenientes. Hay veces que no quieres usar esto:

• Cuando los objetos son muy diferentes: puede terminar con una lista exhaustiva de campos, y no está claro cuáles se aplican a cada tipo de objeto.
• Cuando se opera en una situación crítica de memoria: si necesita hacer 10 herramientas, puede ser flojo con la memoria. Cuando necesite hacer 500 millones de herramientas, comenzará a preocuparse por los bits y bytes. Las estructuras sindicales son siempre más grandes de lo que deben ser.

Los sockets de UNIX no utilizan esta solución debido al problema de disimilitud agravado por el carácter abierto de la API. La intención con los sockets de UNIX era crear algo con lo que todos los sabores de UNIX pudieran funcionar. Cada sabor podría definir la lista de familias que apoyan AF_INET, y habría una lista corta para cada uno. Sin embargo, si aparece un nuevo protocolo, como lo AF_INET6hizo, es posible que deba agregar nuevos campos. Si hiciera esto con una estructura de unión, terminaría creando efectivamente una nueva versión de la estructura con el mismo nombre, creando infinitos problemas de incompatibilidad. Esta es la razón por la cual los sockets UNIX eligieron usar el patrón de conversión en lugar de una estructura de unión. Estoy seguro de que lo consideraron, y el hecho de que lo pensaron es parte de por qué no huele cuando lo usan.

También podrías usar una unión de verdad. Los sindicatos ahorran memoria, ya que solo son tan grandes como el miembro más grande, pero vienen con su propio conjunto de problemas. Probablemente esta no sea una opción para su código, pero siempre es una opción que debe considerar.

Otra solución interesante es boost::variant. Boost es una gran biblioteca llena de soluciones reutilizables multiplataforma. Probablemente sea uno de los mejores códigos C ++ jamás escritos. Boost.Variant es básicamente la versión C ++ de las uniones. Es un contenedor que puede contener muchos tipos diferentes, pero solo uno a la vez. ¡Podrías hacer tu Sword, Shieldy MagicClothclases, y luego hacer que la herramienta sea un poco!), Pero este patrón puede ser increíblemente útil. La variante se usa a menudo, por ejemplo, en árboles de análisis, que toman una cadena de texto y la dividen usando una gramática para las reglas.boost::variant<Sword, Shield, MagicCloth> , lo que significa que contiene uno de estos tres tipos. Esto todavía sufre el mismo problema con la compatibilidad futura que impide que los sockets UNIX lo usen (sin mencionar que los sockets UNIX son C, anteriores aboost

La solución final que recomendaría mirar antes de zambullirse y usar el enfoque genérico de fundición de objetos es el patrón de diseño Visitante . Visitor es un poderoso patrón de diseño que aprovecha la observación de que llamar a una función virtual efectivamente hace el casting que necesita, y lo hace por usted. Debido a que el compilador lo hace, nunca puede estar equivocado. Por lo tanto, en lugar de almacenar una enumeración, Visitor utiliza una clase base abstracta, que tiene una vtable que sabe de qué tipo es el objeto. Luego creamos una pequeña llamada ordenada de doble indirección que hace el trabajo:

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

Entonces, ¿cuál es este patrón horrible de Dios? Bueno, Tooltiene una función virtual, accept. Si le pasa un visitante, se espera que se dé la vuelta y llame a la visitfunción correcta en ese visitante para el tipo. Esto es lo que visitor.visit(*this);hace en cada subtipo. Complicado, pero podemos mostrar esto con su ejemplo anterior:

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
}

Entonces, ¿qué pasa aquí? Creamos un visitante que hará un trabajo por nosotros, una vez que sepa qué tipo de objeto está visitando. Luego iteramos sobre la lista de herramientas. Por el bien del argumento, digamos que el primer objeto es un Shield, pero nuestro código aún no lo sabe. Llama t->accept(v), una función virtual. Debido a que el primer objeto es un escudo, termina llamando void Shield::accept(ToolVisitor& visitor), que llama visitor.visit(*this);. Ahora, cuando estamos buscando qué visitllamar, ya sabemos que tenemos un Escudo (porque se llamó a esta función), así que terminaremos llamando void ToolVisitor::visit(Shield* shield)a nuestro AttackVisitor. Esto ahora ejecuta el código correcto para actualizar nuestra defensa.

El visitante es voluminoso. Es tan torpe que casi creo que tiene un olor propio. Es muy fácil escribir malos patrones de visitante. Sin embargo, tiene una gran ventaja que ninguno de los otros tiene. Si agregamos un nuevo tipo de herramienta, tenemos que agregarle una nueva ToolVisitor::visitfunción. En el instante en que hacemos esto, todos ToolVisitor en el programa se negarán a compilar porque le falta una función virtual. Esto hace que sea muy fácil detectar todos los casos en los que nos perdimos algo. Es mucho más difícil garantizar que si usa ifo switchdeclaraciones para hacer el trabajo. Estas ventajas son tan buenas que Visitor ha encontrado un pequeño nicho en los generadores de escenas de gráficos en 3D. Por casualidad necesitan exactamente el tipo de comportamiento que ofrece Visitor, ¡así que funciona muy bien!

En general, recuerde que estos patrones dificultan el próximo desarrollador. ¡Dedique tiempo para que sea más fácil para ellos, y el código no huele!

* Técnicamente, si nos fijamos en las especificaciones, sockaddr tiene un miembro llamado sa_family. Aquí se está haciendo algo complicado en el nivel C que no nos importa. Le invitamos a ver la implementación real , pero para esta respuesta voy a usar sa_family sin_familyy otras completamente intercambiables, usando la que sea más intuitiva para la prosa, confiando en que ese truco C se ocupa de los detalles sin importancia.

En general, evito implementar varias clases / heredar si es solo para comunicar datos. Puede apegarse a una sola clase e implementar todo desde allí. Para su ejemplo, esto es suficiente.

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

Probablemente, estás anticipando que tu juego implementará varios tipos de espadas, etc., pero tendrás otras formas de implementar esto. La explosión de clase rara vez es la mejor arquitectura. Mantenlo simple.

Como se dijo anteriormente, este es un olor a código serio. Sin embargo, uno podría considerar que la fuente de su problema es usar la herencia en lugar de la composición en su diseño.

Por ejemplo, dado lo que nos ha mostrado, claramente tiene 3 conceptos:

• ít
• Artículo que puede tener ataque.
• Artículo que puede tener defensa.

Tenga en cuenta que su cuarta clase es solo una combinación de los dos últimos conceptos. Así que sugeriría usar composición para esto.

Necesita una estructura de datos para representar la información necesaria para el ataque. Y necesita una estructura de datos que represente la información que necesita para la defensa. Por último, necesita una estructura de datos para representar cosas que pueden tener o no estas o ambas propiedades:

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 qué no crear métodos abstractos modifyAttacky modifyDefenseen la Toolclase? Entonces cada niño tendría su propia implementación, y usted llama a esta forma elegante:

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

Pasar valores como referencia ahorrará recursos si puede:

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

Si uno usa polimorfismo, siempre es mejor si todo el código que se preocupa por qué clase se usa está dentro de la clase misma. Así es como lo codificaría:

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

Esto tiene las siguientes ventajas:

• Es más fácil agregar nuevas clases: solo tiene que implementar todos los métodos abstractos y el resto del código simplemente funciona
• clases más fáciles de eliminar
• es más fácil agregar nuevas estadísticas (las clases que no se preocupan por la estadística simplemente la ignoran)

Creo que una forma de reconocer los defectos de este enfoque es desarrollar su idea hasta su conclusión lógica.

Esto parece un juego, por lo que en algún momento probablemente comenzará a preocuparse por el rendimiento e intercambiará esas comparaciones de cadenas por una into enum. A medida que la lista de elementos se hace más larga, eso if-elsecomienza a ser bastante difícil de manejar, por lo que puede considerar refactorizarlo en a switch-case. También tiene una gran cantidad de texto en este punto, por lo que puede dividir la acción dentro de cada uno caseen una función separada.

Una vez que llegue a este punto, la estructura de su código comenzará a parecerle familiar (comienza a parecerse a una vtable * casera y manual), la estructura básica sobre la cual se implementan típicamente los métodos virtuales. Excepto, esta es una tabla virtual que debe actualizar y mantener manualmente, cada vez que agrega o modifica un tipo de elemento.

Al apegarse a las funciones virtuales "reales", puede mantener la implementación del comportamiento de cada elemento dentro del propio elemento. Puede agregar elementos adicionales de una manera más autónoma y coherente. Y mientras hace todo esto, es el compilador el que se encargará de la implementación de su despacho dinámico, en lugar de usted.

Para resolver su problema específico: tiene dificultades para escribir un par simple de funciones virtuales para actualizar el ataque y la defensa porque algunos elementos solo afectan el ataque y algunos elementos solo afectan la defensa. El truco en un caso simple como este es implementar ambos comportamientos de todos modos, pero sin efecto en ciertos casos. GetDefenseBonus()podría regresar 0o ApplyDefenseBonus(int& defence)simplemente podría dejar defencesin cambios. La forma en que lo haga dependerá de cómo desee manejar las otras acciones que sí tienen efecto. En casos más complejos, donde hay un comportamiento más variado, simplemente puede combinar la actividad en un solo método.

_{* (Aunque, transpuesta en relación con la implementación típica)}

Tener un bloque de código que conozca todas las "herramientas" posibles no es un gran diseño (especialmente porque terminarás con muchos de esos bloques en tu código); pero tampoco es tener un básico Toolcon apéndices para todas las propiedades posibles de la herramienta: ahora la Toolclase debe conocer todos los usos posibles.

Lo que cada herramienta sabe es lo que puede contribuir al personaje que la usa. Por lo tanto proporcionar un método para todas las herramientas, giveto(*Character owner). Ajustará las estadísticas del jugador según corresponda sin saber qué otras herramientas pueden hacer, y lo que es mejor, tampoco necesita saber acerca de las propiedades irrelevantes del personaje. Por ejemplo, un escudo ni siquiera necesita saber acerca de los atributos attack, invisibility, healthetc. Todo lo que se necesita para aplicar una herramienta es para el personaje de apoyo a los atributos que el objeto requiere. Si intentas darle una espada a un burro, y el burro no tiene attackestadísticas, obtendrás un error.

Las herramientas también deberían tener un remove()método que invierta su efecto sobre el propietario. Esto es un poco complicado (es posible terminar con herramientas que dejan un efecto distinto de cero cuando se les da y luego se las quitan), pero al menos está localizado en cada herramienta.

No hay respuesta que diga que no huele, así que seré yo quien defienda esa opinión; ¡Este código está totalmente bien! Mi opinión se basa en el hecho de que a veces es más fácil seguir adelante y permitir que tus habilidades aumenten gradualmente a medida que creas más cosas nuevas. Puedes quedarte atrapado durante días haciendo una arquitectura perfecta, pero probablemente nadie lo verá en acción, porque nunca terminaste el proyecto. ¡Aclamaciones!