8
\$\begingroup\$

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();
    }
}
\$\endgroup\$
0

2 Answers 2

11
\$\begingroup\$

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.

\$\endgroup\$
5
  • \$\begingroup\$ This is a really awesome idea! That was what I meant regarding the constructor, but your solution makes the parameters much more informative. So just to make sure I'm understanding, I would have more enums inside ProfessionType, one for each save type, and have STRONG and WEAK in each one? Is there a way of further consolidating that to just have one enum with STRONG and WEAK used in each throw type, where I would still be able to do something like Will.STRONG? \$\endgroup\$
    – BrainFRZ
    Oct 31, 2015 at 6:00
  • \$\begingroup\$ I added another alternative for consolidating the STRONG and WEAK values. I don't know a better way to avoid repeating those values in multiple enums. \$\endgroup\$
    – janos
    Oct 31, 2015 at 7:10
  • \$\begingroup\$ +1 but I disagree about moving attributes to a config file. I think it's a myth that only immature projects don't use sophisticated systems for this. There's complexity and overhead in managing, building and testing a .xml or .yaml or other format config file. Better to just make the config a .java where the compiler and existing build process handles it. \$\endgroup\$
    – djechlin
    Oct 31, 2015 at 9:57
  • \$\begingroup\$ I agree about the configure system being too much. When adding new types it is frequent that you need to change something in code anyway to handle to character classes and so forth. \$\endgroup\$ Oct 31, 2015 at 16:31
  • 1
    \$\begingroup\$ @BrainFRZ All saves are either weak or strong. That will Never, Ever change. Don't bother specifying strong and weak for each type of save. Just specify which save(s) are Strong. Everything else is weak. \$\endgroup\$
    – Eric Stein
    Oct 31, 2015 at 19:29
0
\$\begingroup\$

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
\$\endgroup\$
7
  • \$\begingroup\$ This answer doesn't provide any meaningful input in any way. \$\endgroup\$
    – lealand
    Oct 31, 2015 at 4:15
  • 1
    \$\begingroup\$ @lealand He's asking for the best way to write it--and I think the best way is not to write it at all. \$\endgroup\$ Oct 31, 2015 at 4:19
  • 1
    \$\begingroup\$ Look at what @janos has provided. It is at least comprehensible. If you think that what the OP is doing is the wrong approach -- and I agree with you on the point -- make a constructive point to that regard. \$\endgroup\$
    – lealand
    Oct 31, 2015 at 4:23
  • 1
    \$\begingroup\$ @BrainFRZ it's a bit of a myth that there's an advantage to a config file. For most typical production systems you still have to build the .jar or .war whenever you change the config file. You still should unit or integration test changes to it. In fact it's as if you may as well save the config as .java and let the compiler and build you already have worry about it. \$\endgroup\$
    – djechlin
    Oct 31, 2015 at 9:59
  • 1
    \$\begingroup\$ @BrainFRZ there are hypothetical cases where it does make sense. If you want non-programmers to be able to read or write it, would be one use case. It's also valuable to understand how to separate data from logic, but that doesn't mean data can't live in a .java file. I think functional languages are much more religious about this, and languages like java are sort of getting there, but for now an enum containing data like this is perfectly fine production code. \$\endgroup\$
    – djechlin
    Oct 31, 2015 at 10:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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