Honestly, I'm not sure I'd bother trying to merge these methods. Yes, they're essentially identical, but when you look more closely, the duplicated code is still just a few lines, most of which is unavoidable boilerplate code imposed by the language and class design you're using.
Generally, when thinking about whether to DRY up some (seemingly) repetitive code, there's a couple of questions that I'd recommend asking yourself first:
Is it really true duplication, or merely incidental similarity? That is, if you were to change one of the methods, would you expect having to change the other one too?
This seems like a particularly relevant question for your example, because most of the apparent duplication comes from the fact that the attack and defense formulas are so similar. If they're similar by design, so that changing one would always require changing the other one too, then it's true duplication and might call for refactoring. On the other hand, if you can foresee the possibility that you might want to make the formulas different in the future, then combining them now may, in fact, turn out to be detrimental later.
Will combining the methods make the code shorter and simpler? If eliminating the duplication makes the code longer or harder to understand, then you haven't really gained anything, and may have just made things worse.
The real purpose of all these "rules of thumb" like DRY is to save you time and to keep your code simple, efficient and maintainable. If you find yourself wasting significant development time or introducing lots of extra complexity just to follow a rule of thumb, you may be taking the rule too far.
Will there be more duplication like this? This is really a corollary to the previous question. If you know that there will only ever be two methods like this, and they're unlikely to become more complex than they are now, then de-duplicating them (at least for now) may be a waste of effort. However, if you think you might need a third or a fourth copy, it might be time to think about combining them into one method.
Sometimes, if you expect a piece of code to acquire more uses in the future, it may even be worthwhile to make it flexible to enough to allow such uses without duplication, even if it hasn't been duplicated yet. Don't go too far with this, though; always keep YAGNI in mind, too.
All that said, if I really believed the similarity between the two methods to be fundamental and not just incidental, and if I really wanted to eliminate the redundancy, what I'd do (at least to start with) would be to factor the shared (1 + level + sum(equipment)) * sqrt(level)
formula into a separate (static) method, leaving me with:
public class PlayerCharacter {
private int level;
public int getAttack() {
int attackBonus = 0;
for (Equipment e : equipments) attackBonus += e.getAttack();
return powerFormula(level, attackBonus);
}
public int getDefense() {
int defenseBonus = 0;
for (Equipment e : equipments) defenseBonus += e.getDefense();
return powerFormula(level, defenseBonus);
}
private static int powerFormula(int level, int bonus) {
return (1 + level + bonus) * (int) Math.sqrt(level);
}
}
(The choice of making the powerFormula()
method static is mainly a stylistic one, but I felt it appropriate to emphasize the fact that it just encapsulates a simple mathematical formula, which exists independently of any particular character object.)
Now, there's still quite a bit of apparent repetition left, but it's really all just boilerplate for summing the attack / defense bonuses of the player's equipment. In Java 8, we could tidy up the remaining duplication by using method references, e.g. like this:
public class PlayerCharacter {
private int level;
public int getAttack() {
int attackBonus = getEquipmentBonus(Equipment::getAttack);
return powerFormula(level, attackBonus);
}
public int getDefense() {
int defenseBonus = getEquipmentBonus(Equipment::getDefense);
return powerFormula(level, defenseBonus);
}
private int getEquipmentBonus(ToIntFunction<Equipment> func) {
return equipments.stream().mapToInt(func).sum();
}
private static int powerFormula(int level, int bonus) {
return (1 + level + bonus) * (int) Math.sqrt(level);
}
}
(I hope this code is correct, because I don't actually have a Java 8 compiler to test it with. It could be shortened further by combining the getEquipmentBonus()
and powerFormula()
methods into a single method, but keeping them separate feels cleaner to me — that way, each method has a single clearly defined responsibility. For the getEquipmentBonus()
method, I chose to use the Stream interface, but you could equally well just use a simple loop.)
For earlier Java versions, the duplicate loops seem pretty much unavoidable, unless you feel like playing with inner classes to simulate Java 8 lambda expressions or something similar.
In any case, both of the refactored versions above achieve the goal of ensuring that, if you ever want to change the player attack / defense power formula, you can change it in only one place and the change will affect both calculations. If this is not what you want, you may be better off keeping your original code as it is.