Remove duplication of enumerated elements

73 views Asked by At

I have the following enumerator and it's likely to be expanded over the course of program development:

enum myEnum {
    Element1,
    Element2,
    Element3
    ...
    ElementX
    Last
};

I have a function that uses the enumerator in the following way:

bool CheckEnumValidity(myEnum a)
{
    bool valid = false;
    switch (a) {
    case Element1:
    case Element2:
    case Element3:
    case ...
    case ElementX:
        valid true;
        break;
    case Last:
        valid false;
        break;
    };
    return valid;
}

QUESTIONS:

1) I duplicate Element1, Element2 etc. in two places in my program. How to get rid of the duplication in the safest way?

2) Should I have default behavior that throws an exception (or return false) in the aforementioned switch statement given that CheckEnumValidity() has an argument of myEnum type?

NOTES:

C++ 11 is unavailable for my application.

3

There are 3 answers

2
dlask On BEST ANSWER

Provided that your enum really doesn't contain any explicit value assignment then you can write:

if (a <= Last) {
    return (a < Last);
} else {
    throw AnyExceptionYouWant();
}
0
Peter On

It would probably be easier, through coding guidelines, peer pressure, policy enforcement (sack any programer who does not comply with the coding guideline) or other means to ensure that calling code which uses your enum only ever supplies named values.

In other words, disallow conversion of an integral value to an enumerated type. After all, doing such things negates most of the reason for using an enumerated type in the first place.

If, despite this suggestion, you want to test, I'd write a little program that parses your header files, finds all the enum types, and automatically generates your SomeFunction(). With makefiles, it is easy to ensure that program is run whenever relevant header files change, which means the function would be updated, recompiled, and linked into your program to keep the check consistent with the type definition.

As to whether your check function should throw an exception, that comes down to what the consequences of a value failing the test are. If your program should really not continue, then throw an exception. If the error is benign and your program can continue somehow, simply log an error message (e.g. to std::cerr) and continue.

0
antron On

To answer your first question, there is no very straightforward way to do this in C++, though I will leave a comment by your question pointing to some approaches.

For your second question, I recommend you use a default case. Here is why. The first reason is weaker, but the last two are stronger.

  1. Someone may convert an integer explicitly to an enumerated value without checking that it is valid. This should be forbidden, but it still sometimes happens, and you should catch this programming error at run time if it was missed in code review.
  2. You may read a struct or other data from an untrusted external source, where that struct contains an enum field, and forget to properly validate it. The untrusted external source could even be a file saved with an older version of your program, where the enum had a different set of valid values.
  3. You may have an uninitialized enum somewhere.

Even something as simple as this:

enum A {X = 1, Y, Z};

int main()
{
    A foo;

    switch (foo) {
        case X: return 0;
        case Y: return 1;
        case Z: return 2;
    }
}

As to what you should do in the default case, it depends on your project and the specific enum. For example, if enums should always be validated before entering the bulk of your program, thus preventing invalid values, and it's okay to fail if this is violated, then you should probably throw an exception or even call exit, after printing a suitable error message – this is a programming failure caught at run time.

If failing like this is not an option, you should probably at least still try to log it, at least in a debug build, so you can detect the problem.

If invalid values make sense for a particular enum, then handle it as you see fit for that enum according to why it makes sense.