4
\$\begingroup\$

I am specifically looking for input on naming ModifierCalculator and Modifiers. I would also appreciate input on whether ModifierCalculator should extend EnumMap instead of having it as a field.

These classes are for modifying the status of units. The StatType enum contains different types of ..stats, such as "life", "armor", "weapon damage", etc.

StatModifier objects' purpose is to modify a unit's stat. This kind of modification can be a flat bonus, a percentage increment or a multiplication, using the formula

total = (base + flat)*(1 + percent_increments)*(1 + mult_1)*(1 + mult_2)*... *(1 + mult_n)

ModifierCalculator puts all three kinds of modifiers in a map of Modifiers data structures with StatType serving as keyword.

public enum StatType
{
    //code is trivial to the problem
}

public enum StatModifierType
{
    FLAT, PERCENTAGE, MULTIPLICATIVE;
}

public class StatModifier
{
    private StatType statType;
    private double magnitude;
    private StatModifierType modType;

    public StatModifier(StatType statType, double magnitude, StatModifierType modType)
    {
        this.statType = statType;
        this.magnitude = magnitude;
        this.modType = modType;
    }

    public StatType getStatType()
    {
        return statType;
    }

    public double getMagnitude()
    {
        return magnitude;
    }

    public StatModifierType getModifierType()
    {
        return modType;
    }
}

public class Modifiers
{
    public double flat = 0;
    public double percentage = 0;
    public final ArrayList<Double> multipliers = new ArrayList<Double>();
}

public class ModifierCalculator
{
    private EnumMap<Stat, Modifiers> statToModifiers
          = new EnumMap<Stat, Modifiers>(Stat.class);

    public ModifierCalculator()
    {
        for (StatType s : Stat.values())
            statToModifiers.put(s, new Modifiers());
    }

    public void evaluate(ArrayList<StatModifier> modifiers)
    {
        for (StatModifier modifier : modifiers)
            evaluate(modifier);
    }

    public void evaluate(StatModifier modifier)
    {
        double magnitude = modifier.getMagnitude();
        Modifiers mods = statToModifiers.get(modifier.getStat());
        switch (modifier.getModifierType())
        {
        case FLAT:
            mods.flat += magnitude;
            break;
        case PERCENTAGE:
            mods.percentage += magnitude;
            break;
        case MULTIPLICATIVE:
            mods.multipliers.add(1 + magnitude);
            break;
        }
    }

    public Modifiers getStatModifiers(StatType statType)
    {
        return statToModifiers.get(statType);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Do you have any real need to distinguish between multiplicative and percentual modifiers? They seem to serve the same purpose with just different ranges, so you maybe can merge them together? \$\endgroup\$ – Diego Martinoia Jul 28 '15 at 14:47
  • \$\begingroup\$ I don't understand what you mean with "just different ranges." They don't serve the same purpose. \$\endgroup\$ – Aeailmuawe Jul 28 '15 at 15:15
  • \$\begingroup\$ In your description you state that "(base + flat)*(1 + percent_increments)*(1 + mult_1)*(1 + mult_2)*... (1 + mult_n)" . How is this different from " (base + flat)*(1 + mult_0)*(1 + mult_1)*(1 + mult_2)... *(1 + mult_n)" ? Their behavior is the same, it seems to me that it only changes the range (percentage is 0-1, multipliers are > 1), but I may be wrong. \$\endgroup\$ – Diego Martinoia Jul 28 '15 at 15:17
  • \$\begingroup\$ Multipliers range: (-1, infinite) Percent range: (0, infinite) % modifiers increase the value of flat modifiers. Multipliers increase the value of flat modifiers, % modifiers and other multipliers. How do make new lines \$\endgroup\$ – Aeailmuawe Jul 28 '15 at 15:19
  • 1
    \$\begingroup\$ Example: you have a 50% increase and another 50% increase, total increase is 100%. Example 2: you have a 150% multipler and another 150% multipler, total is 125% increase. \$\endgroup\$ – Aeailmuawe Jul 28 '15 at 15:29
1
\$\begingroup\$

IMHO The names are fine. You could maybe want to modify Modifiers in StatModifiersSummary as it's, after all, a summary "view" of the actual modifiers.

You should make its fields final or private, though.

In that case, your calculator would become StatModifiersSummaryCalculator.

W.r.t. extending / having one map inside, composition is in general preferable to inheritance, so stick with the current design: after all you happen to use a Map, but there is no reason why you couldn't use another representation and you don't want your "users" to be able to pass your calculator around where a map is needed.

\$\endgroup\$
  • \$\begingroup\$ Making its fields final wouldn't make any sense. As for making them private, I just don't see the point. \$\endgroup\$ – Aeailmuawe Jul 28 '15 at 16:00
  • \$\begingroup\$ @Aeailmuawe Both the options are suggested for guaranteeing encapsulation: it's a public reachable class, someone may otherwise accidentally overwriting your values AFTER you evaluate them by doing getStatModifier(xxx).percentage = 100000. Granted, if you are the only user of the code, no problem, but then this argument would hold in the same way for every instance of the "encapsulation" problem \$\endgroup\$ – Diego Martinoia Jul 28 '15 at 16:06
  • \$\begingroup\$ Making them final would actually break it. As for "encapsulation", if I add setters and getters then anything that can call getStatModifier will still be able to call the setters. Following this: oracle.com/technetwork/java/javase/documentation/… 10.1 \$\endgroup\$ – Aeailmuawe Jul 28 '15 at 16:11
  • \$\begingroup\$ @Aeailmuawe Making it final (or removing the setters), would need you to refactor your code to work with immutable "Modifiers" class. This would require to change your logic to use copy-like constructors (and guaranteeing that the users accessing it don't store the old modifiers object). I am not familiar with your project, so it may be an overkill, especially if you are sure that no one ever will access the fields for writing except the calculator, but I'd argue that, in general, beans should be immutable unless needed otherwise. it's just good practice (imho) \$\endgroup\$ – Diego Martinoia Jul 28 '15 at 16:18
  • \$\begingroup\$ I prefer using private fields in objects and public ones in data structures. \$\endgroup\$ – Aeailmuawe Jul 28 '15 at 17:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.