Using default in a switch statement when switching over an enum

What is your procedure when switching over an enum where every enumeration is covered by a case? Ideally you'd like the code to be future proof, how do you do that?

Also, what if some idiot casts an arbitrary int to the enum type? Should this possibility even be considered? Or should we assume that such an egregious error would be caught in code review?

enum Enum
{
 开发者_如何学Python   Enum_One,
    Enum_Two
};

Special make_special( Enum e )
{
    switch( e )
    {
        case Enum_One:
            return Special( /*stuff one*/ );

        case Enum_Two:
            return Special( /*stuff two*/ );
    }
}

void do_enum( Enum e )
{
    switch( e )
    {
        case Enum_One:
            do_one();
            break;

        case Enum_Two:
            do_two();
            break;
    }
}

• leave off the default case, gcc will warn you (will visual studio?)
• add a default case with a assert(false);
• add a default case that throws a catchable exception
• add a default case that throws a non-catchable exception (it may just be policy to never catch it or always rethrow).
• something better I haven't considered

I'm especially interested in why you choose to do it they way you do.

I throw an exception. As sure as eggs are eggs, someone will pass an integer with a bad value rather than an enum value into your switch, and it's best to fail noisily but give the program the possibility of fielding the error, which assert() does not.

I would put an assert.

Special make_special( Enum e )
{
    switch( e )
    {
        case Enum_One:
            return Special( /*stuff one*/ );

        case Enum_Two:
            return Special( /*stuff two*/ );

        default:
            assert(0 && "Unhandled special enum constant!");
    }
}

Not handling an enumeration value, while the intention is to cover all cases, is an error in the code that needs to be fixed. The error can't be resolved from nor handled "gracefully", and should be fixed right away (so i would not throw). For having the compiler be quiet about "returning no value" warnings, call abort, like so

#ifndef NDEBUG
#define unreachable(MSG) \
  (assert(0 && MSG), abort())
#else
#define unreachable(MSG) \
  (std::fprintf(stderr, "UNREACHABLE executed at %s:%d\n", \
                __FILE__, __LINE__), abort())
#endif 

Special make_special( Enum e )
{
    switch( e )
    {
        case Enum_One:
            return Special( /*stuff one*/ );

        case Enum_Two:
            return Special( /*stuff two*/ );

        default:
            unreachable("Unhandled special enum constant!");
    }
}

No warning by the compiler anymore about a return without value, because it knows abort never returns. We immediately terminate the failing program, which is the only reasonable reaction, in my opinion (there is no sense in trying to continue to run a program that caused undefined behavior).

First of all, I would always have a default in a switch statement. Even if there are no idiots around to cast integers to enums, there's always the possibility of memory corruption that the default can help to catch. For what it's worth, the MISRA rules make the presence of a default a requirement.

Regarding what you do, that depends on the situation. If an exception can be handled in a good way, handle it. If it's a state variable in a non-critical part of code, consider silently resetting the state variable to the initial state and carrying on (possibly logging the error for future reference). If it is going to cause the entire program to crash in a really messy way, try to fall over gracefully or something. In short, it all depends on what you're switching on and how bad an incorrect value would be.

As an additional remark (in addition to other responses) I'd like to note that even in C++ language with its relatively strict type-safety restrictions (at least compared to C), it is possible to generate a value of enum type that in general case might not match any of the enumerators, without using any "hacks".

If you have a enum type E, you can legally do this

E e = E();

which will initialize e with zero value. This is perfectly legal in C++, even if the declaration of E does not include a enumeration constant that stands for 0.

In other words, for any enum type E, the expression E() is well-formed and generates zero value of type E, regardless of how E is defined.

Note, that this loophole allows one to create a potentially "unexpected" enum value without using any "hacks", like a cast of an int value to enum type you mentioned in your question.

LLVM Coding Standards' opinion is: Don’t use default labels in fully covered switches over enumerations

Their rationale is:

-Wswitch warns if a switch, without a default label, over an enumeration does not cover every enumeration value. If you write a default label on a fully covered switch over an enumeration then the -Wswitch warning won’t fire when new elements are added to that enumeration. To help avoid adding these kinds of defaults, Clang has the warning -Wcovered-switch-default which is off by default but turned on when building LLVM with a version of Clang that supports the warning.

Based on this , I personally like doing:

enum MyEnum { A, B, C };

int func( MyEnum val )
{
    boost::optional<int> result;  // Or `std::optional` from Library Fundamental TS

    switch( val )
    {
    case A: result = 10; break;
    case B: result = 20; break;
    case C: result = 30; break;
    case D: result = 40; break;
    }

    assert( result );  // May be a `throw` or any other error handling of choice

    ...  // Use `result` as seen fit

    return result;
}

If you choose to return on every case of the switch, you don't need boost::optional: simply call std::abort() or throw unconditionally just after the switch block.

It's nonetheless important to remember that maybe switch is not the best design tool to begin with (as already stated in @UncleBens answer): Polymorphism or some type of lookup-table could place a better solution, especially if your enum has a lot of elements.

PS: As a curiosity, the Google C++ Style Guide makes an exception to switchs-over-enums not to have default cases:

If not conditional on an enumerated value, switch statements should always have a default case

Your items are good. But I'd remove 'throw catchable exception'.

Additional:

• Make warnings to be treated as errors.
• Add logging for default cases.

As a further option: Avoid switching over enums.

I tend to do option 2:

add a default case that throws a catchable exception

That should highlight the problem if it ever occurs and it only costs you a couple of lines to imlpement.

assert and then maybe throw.

For in-house code thats in the same project as this (you didnt say what the function boundary is - internal lib, external lib, inside module,...) it will assert during dev. THis is what you want.

If the code is for public consumption (other teams, for sale etc) then the assert will disappear and you are left with throw. THis is more polite for external consumers

If the code is always internal then just assert

My $0.02:

If this method is externally visible (called by code that you don't control), then you probably need to handle the possibility of someone sending you an invalid enum value. In that case, throw an exception.

If this method is internal to your code (only you call it), then the assertion should be all that is necessary. This will catch the case of someday adding a new enum value and forgetting to update your switch statement.

Always provide a default case in every switch, and at the very least, assert if it gets hit. This habit will pay off by saving you hours or even days of head scratching every once in a while.

In addition to the advise to throw an exception, if you use gcc, you can use the -Wswitch-enum (and eventually -Werror=switch-enum) which will add a warning (or an error) if a member of you enum doesn't appear in any case. There may be an equivalent for other compilers but I only use it on gcc.

In cases similar to the example provided you can actually combine option 1 with one of the other options: Omit the default and enable the appropriate compiler warning if available. This way you find out immediately if a new enumerated value is added and you can conveniently add the case without having to guarantee that a specific code path executes to find it at runtime.

Then between the end of the switch and the end of the function add code to assert or throw (I prefer assert as this really is an invalid situation). This way if someone casts an int to your enum type you still get the runtime checking as well.

I am not a C++ guy. But, in C#, I whould have written it like

enum Enum
{
    Enum_One,
    Enum_Two
};


Special make_special( Enum e )
{

    if(Enums.IsDefined(typeof(Enum),(int)e))
    {
       switch( e )
       {
          case Enum_One:
              return Special( /*stuff one*/ );
          case Enum_Two:
              return Special( /*stuff two*/ );
       }
    }
    // your own exception can come here.
    throw new ArgumentOutOfRangeException("Emum Value out of range");

}

I personally recommend any of your solutions, except the first. Leave in the default case, and assert, throw (whatever exception type you like). In C#, Visual Studio doesn't warn you if you leave out the default case.

The reason I suggest you add the case and fail on it is that this is an important point of code maintenance. As soon as someone adds to the enumerated list, the switch statement has to grow to match.

Also (as UncleBen said), see if you can design against this whole scenario by using polymorphism. That way, when you add a new value to the enumeration, you only have to add it in one place. Any time you see a switch on an enum, you should consider using polymorphism instead.

Apart from exceptions which would be the preferred solution at runtime (and will handle crazy casting), I tend to use static compile time assertions as well.

You can do the following:

//this fails at compile time when the parameter to the template is false at compile time, Boost should provide a similar facility
template <COLboolean IsFalse> struct STATIC_ASSERTION_FAILURE;
template <> struct STATIC_ASSERTION_FAILURE<true>{};
#define STATIC_ASSERT( CONDITION_ ) sizeof( STATIC_ASSERTION_FAILURE< (COLboolean)(CONDITION_) > );

//
// this define will break at compile time at locations where a switch is being
// made for the enum. This helps when adding enums
//
// the max number here should be updated when a new enum is added.
//
// When a new enum is added, at the point of the switch (where this
// define is being used), update the switch statement, then update
// the value being passed into the define.
//
#define ENUM_SWITCH_ASSERT( _MAX_VALUE )\
   STATIC_ASSERT( _MAX_VALUE  ==  Enum_Two)

enum Enum
{
    Enum_One = 0,
    Enum_Two = 1
};

Then in your code, whenever you use the enum set:

ENUM_SWITCH_ASSERT( Enum_Two )
switch( e )
{
    case Enum_One:
        do_one();
        break;
    case Enum_Two:
        do_two();
        break;
}

Now whenever you change the macro ENUM_SWITCH_ASSERT to handle a new enum value, it will break at compile time near locations that use the enum set. Helps lots when adding new cases.

Because it may help to know what the unexpected value of the enum was, write your own BADENUM(Enum) macro, along the lines of:

#define _STRIZE(x) _VAL(x)
#define _VAL(x) #x
extern void  __attribute__ ((noreturn)) AssertFail(const char *Message);
#define ASSERT(Test) ((Test) ? (void)0 : AssertFail(__FILE__ ":" _STRIZE(__LINE__) " " #Test))

extern void  __attribute__ ((noreturn)) BadEnum(const char *Message, const long unsigned Enum);
#define BADENUM(Enum)  BadEnum(__FILE__ ":" _STRIZE(__LINE__), (u32)Enum))