代码之家 › 专栏 › 技术社区 › Macha

如何使这个C++代码更干燥?

code-cleanup dry c++

Macha · 技术社区 · 15 年前

我在一个类上有这两个方法,它们只在一个方法调用中有所不同。显然,这是非常不干燥,特别是因为两者都使用相同的公式。

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++中整理这个?

7 回复 | 直到 15 年前

Warty 15 年前

在我看来,你所拥有的是很好的,因为它将允许你调整攻击/防御比如果你用一个函数来代表他们两个。一旦你开始测试你的游戏,你将开始平衡攻击/防御公式,所以为他们有单独的功能是好的。

Dry的整个概念[不要重复您自己]是[希望]防止您的代码成为一个巨大的复制粘贴活动。在你的情况下,防御/攻击公式会随着时间而改变[例如,如果角色有buff/status-ailment怎么办?特定状态的疾病可能会使防御减半,同时使攻击增加2(狂暴,FF参考,HEH)]

Bill Prin 15 年前

一种简单的方法是在一个数组中表示设备的所有属性,并通过枚举进行索引。

enum Attributes {
  Attack, 
  Defense,
  AttributeMAX
};

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

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

};

然后你把你的功能改为

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

你可以这样称呼它

  player.getAttribute(player.attack, Attack);

Salman A 15 年前

从严格的重构角度来看,您可以这样做:

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

Bill 15 年前

嗯,我至少会考虑提取 sqrt(this.level); 作为单独的函数调用 getLevelModifier()

和

defense = 1 + this.level;

attack = 1 + this.level;

可以是

defense = getBaseDefense();

attack= getBaseAttack();

这不仅增加了灵活性,还自动记录了您的功能。

Clifford 15 年前

取决于应用程序中的其他代码,它可能值得,也可能不值得,但是OOP方法将使防御和攻击值对象为类,而不是普通的 int . 然后,您可以从一个公共的基类派生它们,该类有一个get()方法,该方法根据需要调用由每个子类定义的虚拟getEquipmentRate()方法。

Community CDub 8 年前

除了 ltzWarty 我建议您将循环重构为函数,以提高可读性:

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

另外,当您声明局部变量时 attack 你应该 initialize it immediately .

Jerry Coffin 15 年前

在我看来,itzwarty提出了一个合理的观点——你可能只想把代码放在一边。假设你认为改变它是件好事,你可以这样做:

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

你可以这样称呼它:

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

或:

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

编辑:

另一个明显的可能性是命令任何一件设备可以只有防守或进攻。在这种情况下,事情会变得更简单,以至于你是否真的需要一个函数甚至是有疑问的:

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