Checking DnD Traits in an enum

Question

Overall, I'm working on a simulator for a modified DnD 3.5 system. Right now, I'm trying to write an enum class that handles the Classes/Professions, and am currently trying to handle the profession modifiers for saving throws.

My issue is that everything is extremely boiler-plate, and I can't figure out a better way of doing it. I was thinking of maybe having a constructor for each profession that used true or false to denote if it was a good saving throw or bad one. That's just really bad though, I think, since it doesn't give any information when reading it. I also thought of having goodSaves and badSaves variables in each and then getGoodSaves() and getBadSaves(), but I can't think of an easy way of getting it working without a literal collection like in other languages.

Does anyone have any advice on how I can make this look better?

public enum ProfessionType {
    BARBARIAN {
        @Override
        public int getWillModifier(int level) {
            return badSaveModifier(level);
        }

        @Override
        public int getFortitudeModifier(int level) {
            return goodSaveModifier(level);
        }

        @Override
        public int getReflexModifier(int level) {
            return badSaveModifier(level);
        }
    },
    BARD {
        @Override
        public int getWillModifier(int level) {
            return goodSaveModifier(level);
        }

        @Override
        public int getFortitudeModifier(int level) {
            return badSaveModifier(level);
        }

        @Override
        public int getReflexModifier(int level) {
            return goodSaveModifier(level);
        }
    },

    /* snip containing all the other professions */

    WIZARD {
        @Override
        public int getWillModifier(int level) {
            return goodSaveModifier(level);
        }

        @Override
        public int getFortitudeModifier(int level) {
            return badSaveModifier(level);
        }

        @Override
        public int getReflexModifier(int level) {
            return badSaveModifier(level);
        }
    };


    private static int goodSaveModifier(int level) {
        return ((level / 2) + 2);
    }

    private static int badSaveModifier(int level) {
        return (level / 3);
    }

    /**
     * Calculates the Profession's Fortitude modifier.
     *
     * @param level Character's current level
     * @return Profession's modifier of the Will save
     */
    abstract int getWillModifier(int level);

    /**
     * Calculates the Profession's Fortitude modifier.
     *
     * @param level Character's current level
     * @return Profession's modifier of the Fortitude save
     */
    abstract int getFortitudeModifier(int level);

    /**
     * Calculates the Profession's Reflex modifier.
     *
     * @param level Character's current level
     * @return Profession's modifier of the Reflex save
     */
    abstract int getReflexModifier(int level);


    @Override
    public String toString() {
        return name().substring(0, 1).toUpperCase() + name().substring(1).toLowerCase();
    }
}

Answer 1

I was thinking of maybe having a constructor for each profession that used true or false to denote if it was a good saving throw or bad one. That's just really bad though, I think, since it doesn't give any information when reading it.

I suppose you mean that this would not be particularly readable:

enum ProfessionType {
    BARBARIAN(false, true, false),
    BARD(true, false, true),
    WIZARD(true, false, false);
    // ...
}

Instead of booleans, you could use enums, like Will.STRONG, Fortitude.WEAK, and so on. The result will be more readable and less boilerplate.

With this refactoring, the methods like getWillModifier will be no longer abstract, and would contain the logic (probably a switch) to get the right value given the property of the enum, and the level.

In general, enums are not so well-suited to have behaviors (custom methods), they are much better as plain values, possibly with some common behavior (non-abstract methods).

Another alternative world be to use the builder pattern to build attributes, with a syntax like this:

BARBARIAN(Attributes.builder().will(WEAK).fortitude(STRONG).build()),

Where WEAK and STRONG could come from a Defense enum.

That said, the other answer makes an excellent point that perhaps the attributes should not be hardcoded, but read from a configuration file. You would gain a lot of flexibility, such as adding new types or changing the attributes without recompiling the code.

Answer 2

I would not write such code, period--this sort of information belongs in a data file, not the code. You have a bunch of possible traits, the information for them is loaded from a file, not put in code in the first place.

Thus your code looks like

[Barbarian]
WillSave=Weak