¿Cómo hacer que este código C++ sea más SECO?

Question

Tengo estos dos métodos en una clase que difieren solo en una llamada a un método. Obviamente, esto es muy poco seco, especialmente porque ambos usan la misma fórmula.

int PlayerCharacter::getAttack() { 
    int attack; 
    attack = 1 + this->level; 
    for(int i = 0; i < this->current_equipment; i++) { 
     attack += this->equipment[i].getAttack(); 
    } 
    attack *= sqrt(this->level); 
    return attack; 
} 
int PlayerCharacter::getDefense() { 
    int defense; 
    defense = 1 + this->level; 
    for(int i = 0; i < this->current_equipment; i++) { 
     defense += this->equipment[i].getDefense(); 
    } 
    defense *= sqrt(this->level); 
    return defense; 
} 

¿Cómo puedo arreglar esto en C++?

Answer 1

En mi opinión, lo que tienes está bien, ya que te permitirá ajustar ataque/defensa más que si representaras a ambos con una función. Una vez que comiences a probar tu juego, comenzarás a equilibrar las fórmulas de ataque/defensa, por lo que tener funciones separadas para ellos está bien.

Todo el concepto de DRY [no se repita] es [afortunadamente] para evitar que su código se convierta en una gran copia & paste fest. En su situación, las fórmulas de defensa/ataque cambiarán con el tiempo [por ejemplo, ¿qué pasa si los personajes tienen buffs/status-medical? Una dolencia de estado específica podría reducir la defensa a la mitad, mientras aumenta el ataque por 2 (Berserk, FF reference, heh)]

Answer 2

Una manera fácil es representar todos los atributos de un equipo en una matriz, indexada por una enumeración.

enum Attributes { 
    Attack, 
    Defense, 
    AttributeMAX 
}; 

class Equipment { 
    std::vector<int> attributes; 

    Equipment(int attack, int defense): attributes(AttributeMAX) 
    { 
    attributes[ATTACK] = attack; 
    attributes[DEFENSE] = defense; 
    } 

}; 

A continuación, cambiar su función a

int PlayerCharacter::getAttribute(int& value, Attribute attribute) { 
    value = 1 + this->level; 
    for(int i = 0; i <= current_equipment; i++) { 
     value += this->equipment[i].attributes[attribute]; 
    } 
    value *= sqrt(this->level); 
    return value; 
} 

Y se le puede llamar como tal

player.getAttribute(player.attack, Attack); 

Answer 3

bien, yo por lo menos considerar la extracción de sqrt(this.level); como una función separada llamada getLevelModifier()

y

defense = 1 + this.level; 

attack = 1 + this.level; 

podría ser

defense = getBaseDefense(); 

attack= getBaseAttack(); 

Esto no sólo añadir flexibilidad, también auto-documentos de su función.

Answer 4

Desde un punto de vista estrictamente refactorización, usted puede hacer esto:

int PlayerCharacter::getDefense() { 
    return getAttribute(&EquipmentClass::getDefense); 
} 

int PlayerCharacter::getOffense() { 
    return getAttribute(&EquipmentClass::getOffense); 
} 

int PlayerCharacter::getAttribute(int (EquipmentClass::*attributeFun)()) { 
    int attribute = 0; 
    attribute= 1 + this->level; 
    for(int i = 0; i <= current_equipment; i++) { 
     attribute += this->equipment[i].*attributeFun(); 
    } 
    attribute *= sqrt(this->level); 
    return attribute; 
} 

Answer 5

OMI, ItzWarty hace un punto razonable - es posible que desee dejar el código solo. Suponiendo que decida que el cambio es una buena cosa, sin embargo, se podría hacer algo como esto:

class equipment { 
public: 
    int getAttack(); 
    int getDefense(); 
}; 

int PlayerCharacter::getBattleFactor(int (equipment::*get)()) { 
    int factor = level + 1; 
    for (int i=0; i<current_equipment; ++i) 
     factor += equipment[i].*get(); 
    return factor * sqrt(level + 1); 
} 

que se podría llamar esto como:

int attack = my_player.getBattleFactor(&equipment::getAttack); 

o:

int defense = my_player.GetBattleFactor(&equipment::getDefense); 

Editar :

Otra posibilidad obvia sería decretar que cualquier pieza de un equipo puede solo sea defensivo u ofensivo.En este caso, las cosas se vuelven más simples aún, hasta el punto que incluso podría ser cuestionable si realmente necesita una función en absoluto:

class PlayerCharacter { 
    std::vector<equipment> d_equip; 
    std::vector<equipment> o_equip; 

// ... 

int d=level+1+std::accumulate(d_equip.begin(), d_equip.end(), 0)*sqrt(level+1); 

int o=level+1+std::accumulate(o_equip.begin(), o_equip.end(), 0)*sqrt(level+1); 

Answer 6

Aparte de respuesta ltzWarty 's recomendaría refactorización su bucle en una función de mejor legibilidad:

int PlayerCharacter::getEquipmentAttack() { 
    int attack = 0; 
    for(int i = 0; i <= current_equipment; i++) { 
     attack += this.equipment[i].getAttack(); 
    } 
    return attack; 
} 
int PlayerCharacter::getAttack() { 
    int attack = 1 + this->level; 
    attack += getEquipmentAttack(); 
    attack *= sqrt(this->level); 
    return attack; 
} 

Además, cuando usted declara su variable local attack que debiera initialize it immediately.

Answer 7

Dependiendo de otros códigos en la aplicación, puede o no valer la pena, pero un enfoque OOP haría valores de defensa y ataque de objetos de una clase en lugar de un simple int. Luego, puede derivarlos de una clase base común que tiene un método get() que llama a un método virtual getEquipmentRate() definido por cada una de las subclases según sea necesario.