C++ Using an int to set an enum from a list

Question

I am using a header file for character class in a game (doing it as a side project for experience). I have it working but it just feels like I'm doing this the long way. I ask for an 'int' to set the character class then set it based on the enum position using a switch statement. Is there a clearer, shorter way to do this operation? Am I doing anything here that would be considered bad practice?

class Character_Class {
  public:
    enum classofnpc { CLERIC, FIGHTER, ROGUE, WIZARD, BARBARIAN, DRUID, PALADIN, SORCERER, BARD, MONK, RANGER, WARLOCK };

    Character_Class(const int& a, const int& b){
        switch (a) {
            case 0 :
                a_class = CLERIC;
                break;
            case 1 :
                a_class = FIGHTER;
                break;
            case 2 :
                a_class = ROGUE;
                break;
            case 3 :
                a_class = WIZARD;
                break;
            case 4 :
                a_class = BARBARIAN;
                break;
            case 5 :
                a_class = DRUID;
                break;
            case 6 :
                a_class = PALADIN;
                break;
            case 7 :
                a_class = SORCERER;
                break;
            case 8 :
                a_class = BARD;
                break;
            case 9 :
                a_class = MONK;
                break;
            case 10 :
                a_class = RANGER;
                break;
            case 11 :
                a_class = WARLOCK;
                break;
        }
        lvl = b;
    }
  private:
    classofnpc a_class;
    int lvl;
};

`a_class = a;` seems clearer. `enum`s by default start from 0 and increse by one. — zch, May 06 '15 at 17:58
@zch Yes, but only with additional check - otherwise it is not safe. `int` can yields value, that is beyond scope of this enumeration. — Mateusz Grzejek, May 06 '15 at 18:02
As an aside, I'd recommend only passing large classes by const reference, for primitives it is [not necessary](https://stackoverflow.com/questions/2108084/pass-by-reference-more-expensive-than-pass-by-value), in fact it is often slightly less efficient, you should just pass them by value. — Cory Kramer, May 06 '15 at 18:30

score 1 · Answer 1 · answered May 06 '15 at 17:56

Your constructor could just be

Character_Class(const classofnpc& a, const int& b)
: a_class { a }, lvl { b }
{ }

Since the enum is exposed publicly, you don't need the switch statement. You could instantiate an instance of this class like so

Character_Class foo{ Character_Class::ROGUE, 12 };

score 1 · Answer 2 · answered May 06 '15 at 17:57

Is there a clearer, shorter way to do this operation?

Yes.

1. Add sentinel value:

enum classofnpc
{
    CLERIC,
    //cut...
    WARLOCK,
    CLS_LIMIT = WARLOCK //set sentinel to largest value
};

2. Check and cast:

Character_Class(const int& a, const int& b)
{
    if(a > CLS_LIMIT)
        //report error, wrong value passed
    else
        a_class = static_cast<classofnpc>(a);

    lvl = b;
}

This is a perfect solution if you want to use numerical values for some reason (user input?). However, if you can, the most type-safe way would be to construct your object using classofnpc as first parameter in constructor (instead of plain int).

a_class = static_cast(a); Works great for what I'm doing. I already have a check to make sure the int 'a' is within bounds when asking for the input in another function. Thanks for the Answer. — StackAttack, May 06 '15 at 18:43

C++ Using an int to set an enum from a list

2 Answers2