DRY attack and defense

I have these two methods on a class that differ only in one method call. Obviously, this is very un-DRY, especially as both use the same formula. Could anyone give me advice on how to tidy this up?

public class PlayerCharacter {
    int level;
    public int getAttack() {
        int attack = 1 + level;
        for(Equipment e : equipments)
           attack += e.getAttack();
        return attack * Math.sqrt(level);
    }
    public int getDefense() {
        int attack = 1 + level;
        for(Equipment e : equipments)
           attack += e.getDefense();
        return attack * Math.sqrt(level);
    }
}

I thought of refactoring the loop to an external method but the different method calls has me stumped.

A few things, bad messages first…

Double and Int

return Math.sqrt(level);

Math.sqrt returns a double. That is a floating point number, probably something like: \$2.352561\$. If you now "cast" that to an int (because your method says, "I only return integers"), you just cut everything behind the dot.

This can sometimes lead to funny behavior:


\$2.7371 \mapsto 2\$

\$2.0125 \mapsto 2\$

That's not nice…

Braces:

It's usually a very good idea to place braces, even when you could leave them out:

for (Equipment e : equipments) {
    attack += e.getAttack();
}

This is because if you want to do something else, too you need to add braces either way, and If you forget it, the results become "unpredictable"

Copy/Paste:

You should never do that, because as soon as you do, you usually can extract a method for it. But if you do it either way then at least take the bother to change the names…

public int getDefense() {
     int attack = 1 + level;

'nuff said on the bad stuff ;)

Attributes

You might want to consider changing your attribute structure to be more "generic". Consider the following code:

for (Equipment e : equipments) {
    defense += e.getAttribute(Attribute.DEFENSE);
}

or similarly for attack:

for (Equipment e : equipments) {
    attack += e.getAttribute(Attribute.ATTACK);
}

now we already can see, that if you do that twice, this wants to be a single method:

public int getPlayerAttribute(Attribute attribute) {
    int attribute = 1 + level;
    for (Equipment e : equipments) {
        attribute += e.getAttribute(attribute);
    }
    return attribute * Math.sqrt(level);
}

This would of course need a relatively large change in your code. You'd then need to change your Equipment from (wild conjecture):

public class Equipment {
    private int attack;
    private int defense;
    //getters and setters
}

to:

public class Equipment {
    private Map stats = new HashMap<>(); 
    // or a different Map implementation of your choice, like
    // EnumMap, as suggested by @ratchet freak in a comment:
    private Map stats = new EnumMap<>(Attribute.class);

    public int getAttribute(Attribute attribute) {
        return stats.get(attribute);
    }
}

And create a new enum to hold all the different kinds of attributes you have:

public enum Attribute {
    ATTACK, DEFENSE, HITPOINTS, WHATEVER
}

StackExchange Code Review Q#57177, answer score: 26