ES.79 is a terrible advice and should be dropped

[hasselmm]

ES.79 is a terrible advice that will cause unreliably code and serious bugs once project evolve and new enum values get added. Adding a default case to switch statements blinds compilers regarding new enum values (unless the proper compiler with the proper magic switches is used). By adding a default case to a switch statement one gives up the compiler's ability to support us to write correct code without good reason. This rule should be dropped, or even reverted into "do not ever use default: and lobby the ISO consortium to remove it form the language".

[jwakely]

ES.79 is a terrible advice that will cause unreliably code and serious bugs once project evolve and new enum values get added. Adding a default case to switch statements blinds compilers regarding new enum values (unless the proper compiler with the proper magic switches is used).

Why? The enforcement section says (emphasis mine):

Flag switch-statements over an enumeration that don’t handle all enumerators and do not have a default.

So if your local coding standards say that all switches must cover all enumerators, and your code already does that, then there's no need to add a default case. And if you add another enumerator, this guideline says that a tool enforcing the guideline should complain. Adding a new case for the new enumerator would satisfy the guideline. I assume that's exactly what you want? If not, try explaining your position properly.

"do not ever use default: and lobby the ISO consortium to remove it form the language"

lol, good one. Do you have any proper rationale for breaking millions of lines of working code?

Rationale for not doing that would be that it makes it impossible to use a switch with an enumerated type that has no enumerators, e.g. std::byte

switch(b) {
case 0xfe:
case 0xff:
  return std::make_error_code(illegal_byte_sequence);
case 0x0:
  return finish();
default:
  process(b);
}

Without default you would need to write 256 cases here. You could rewrite it as if-else but you shouldn't have to.

[hasselmm]

Your example actually demonstrates perfectly why default is not needed at all: As you return from every known case you can just handle the unexpected or default case outside of the switch statement. Actually this is generally good practise to use switch statements for delegation only and to always return (or fallthrough) from known cases: This avoids to create god-functions with way too high complexitity.

[jwakely]

Imagine there's another non-default case that doesn't return. In any case, default isn't going to be removed from the language, so that part of your issue is just silly.

Regarding the guidelines and actual constructive feedback, what part of ES.79 says you have to use default? What is the kind of code you want to write that ES.79 complains about? Why should we not just close this as lacking justification for any change?

[BenjamenMeyer]

ES.79 is a terrible advice that will cause unreliably code and serious bugs once project evolve and new enum values get added.

lol... ensuring all your cases are properly handled is actually good coding practices. default is great for helping your code catch things - like when you added another value and the switch isn't handling it correctly. Good code debugs the software for you and points to you the issues instead of hiding them.

As you return from every known case you can just handle the unexpected or default case outside of the switch statement.

This is rather absurd. using a switch provides a high speed mechanism for branching. Are there limits? Yes - there are limits to how many entries can be in a switch table (which is a good thing too since too large of a table also makes it unreadable and more prone to errors), and in those cases it's easy to move from a std::map<> or similar to keep the near constant time. Adding more branching to handle boundary cases before the switch means slower performance and less optimization.

I'll re-iterate @jwakely and quote from ES.79 itself:

Enforcement Flag switch-statements over an enumeration that don’t handle all enumerators and do not have a default.

Emphasis mine.

[hasselmm]

@BenjamenMeyer Nobody argues against handling errors. My problem really is that default: causes more errors than it prevents. It seemed like a good idea when introduced, but time has prooven this tool not being suiteable for its job. Instead of helping us to write reliable and secure code, it does quite the opposite. Simply not fit for the job. Time to retire that feature.

What really confuses me is your talk about performance. First you introduce the switch statement as performance tool, just to entirely destroy your own hypothesis when mentioning big tables and the better suited alternatives. So it seems you know, while not agreeing with your own conclusion, that switch is NOT a performance tool.

So what was the purpose of your comment, other than insulting me?

[hasselmm]

Imagine there's another non-default case that doesn't return.

@jwakely I'd call such inconsistency pretty terrible code. It wouldn't pass my code review if I see it. I'd only accept such if there are unexpected, invalid states if it makes sense to handle them consistently to out of domain values. Tools can catch such bad style.

In any case, default isn't going to be removed from the language, so that part of your issue is just silly.

Is "this is silly" a valid and rational argument these days?

Regarding the guidelines and actual constructive feedback, what part of ES.79 says you have to use default? What is the kind of code you want to write that ES.79 complains about? Why should we not just close this as lacking justification for any change?

You start with this:

enum class Result { Success, FileError, FormatError };
std::tuple<Result, Model> processData();

void something()
{
    auto [state, model] = processData();

    switch (state) {
    case Result::Success:
        useModel(std::move(model));
        break;

    default:
        showError();
        break;
    }
}

Month, years after writing the code above someone introduces the new state Retry for better UX. The ES.79 compliant snippet above will never retry. Good luck with finding and fixing all of these cases in a mature code base.

Other example:

// Identifiers of some automotive hardware's fictional UDS/TP interface
enum class DataIdentifier { 
    Gear,
    Speed,
    Handbrake,
    Brake,
    // ...
    ContainerHookLocked,
    UnderRunProtectionInstalled,
    UnderRunProtectionActive,
    UnderRunProtectionPosition,
    UnderRunProtectionFoldLimit,
    UnderRunProtectionUnfoldLimit,
    // ... some 100 other parameters
};

// Terrible, fictional example.
// Fictional story: Updating data identifiers is expensive for automotive reasons.
// Therefore we only refetch these identifiers that are relevant for the next operation.
constexpr bool affectsMoveContainerArm(DataIdentifier id)
{
    switch (id) {
    case DataIdentifier::Speed:
    case DataIdentifier::Gear:
    case DataIdentifier::Handbrake:
    case DataIdentifier::ContainerHookLocked:
    case DataIdentifier::UnderRunProtectionActive:
        return true;

    default: // I've delete the full list: was too much work to add new identifiers and ES.79 encourages this
        return false;
}

Now after years successful of operation product management introduces a new truck body that replaces the single container hook by two hooks for... "reasons". They report via the newly introduced identifiers UDS ContainerHookLeftLocked and ContainerHookRightLocked. Poor and badly crafted affectsMoveContainerArm() will never report that these two identifiers must be refetched before deciding if the container arm can be moved. Insert nasty noise of bending metal here.

Right now we have banned default from use in our code base exactly for such expensive, and probably even dangerous mistakes. I really don't like that ES.79 lobbies for ending this ban

[jwakely]

You've shown why you don't want to use default in your code. Fine. So don't use it. That's allowed by ES.79. It doesn't try to stop you banning default in your code.

I'm still not sure if you actually understand the guideline or you're just angry with default. You failed to answer my question: what part of the guideline says you have to use default? Please don't just show why you dislike it again, that's not the question and you don't need to explain that. Please explain why the guideline conflicts with your coding style.

[kcat]

You failed to answer my question: what part of the guideline says you have to use default?

In the Enforcement section: Flag switch-statements over an enumeration that don’t handle all enumerators and do not have a default. If you don't handle all enumerators and don't have a default: case, the guidelines are saying to add a default: case instead of the missing enumerators. Which is terrible for the reasons stated. The guidelines give the reason "Code clarity. Improved opportunities for error detection.", but in reality it's the opposite: using a default: case decreases code clarity and reduces the opportunities for error detection.

IMO, it is better to avoid default: cases to allow the compiler to warn you about unhandled enumerators, to ensure enumerators are handled correctly rather than being silently funneled into a default catch-all that likely isn't going to do what you want for new enumerators. But having a default silences the compiler and code checkers, hiding bugs and making them harder to track down as you have to rely on catching undesired runtime behavior, and making it less clear what the code's intention is. Taking the guideline's own example:

enum E { a, b, c, d };

void f1(E x)
{
    switch (x) {
    case a:
        do_something();
        break;
    case b:
        do_something_else();
        break;
    default:
        take_the_default_action();
        break;
    }
}

"Here it is clear that there is a default action and that cases a and b are special."

But it isn't clear that a and b are special next to c and d. Does the developer intend for c and d to be handled by the default case, or were they added later and the switch wasn't updated to account for them? Are c and d not expected values in this code path, such that there would be an error elsewhere if they manage to hit, and the default: is just to shut up the compiler? And/or did the enum value come from an outside source without verification, and the default: case is there to catch invalid enum values? The developer didn't list c and d so there's nothing saying what the intention is for them here. The guidelines even touches on this issue in its default-less hypothetical:
"Did you forget case d or deliberately leave it out? Forgetting a case typically happens when a case is added to an enumeration and the person doing so fails to add it to every switch over the enumerators."
but adding a default: case doesn't clarify if d was forgotten or deliberately left out either, and sabotages attempts to detect where someone failed to add cases for d in switches.

It would be better instead to suggest adding the missing cases explicitly:

    switch (x) {
    case a:
        do_something();
        break;
    case b:
        do_something_else();
        break;
    case c:
    case d:
        take_the_default_action();
        break;
    }

This makes it clear that c and d take a default action separate from a and b. If a new enumeration e is added, your compiler/checker will alert you that this switch doesn't know what to do with e. This improves code clarity and opportunities for error detection. I do all I can to avoid default: cases in my serious (long term maintenance, non-toy) code, so that I know I'm accounting for all enumerators and will be warned at build time if another one is added that I don't handle, and use it only to deal with user/unknown input where I have to account for unexpected values.

Ideally we'd have strict enums, that is enums that cannot hold a value other than its specified enumerators without invoking UB, and maybe even case ranges (i.e. GCC's case c ... e:) that with properly named enumerator markers would allow making a case for a range of enumerators without having to list them all individually, while still being relatively clear.

But short of that, non-swallowing default: would be the next-most ideal scenario (i.e. a default: case that doesn't stop the compiler/checker from warning about unhandled enumerators). That way you can have the safety fallback in the case of an enum holding a value not matching an enumerator, but still be warned that you haven't specified cases for known enumerators.

[jwakely]

You failed to answer my question: what part of the guideline says you have to use default?

In the Enforcement section: Flag switch-statements over an enumeration that don’t handle all enumerators and do not have a default. If you don't handle all enumerators and don't have a default: case, the guidelines are saying to add a default: case instead of the missing enumerators.

No, they're not saying that. You can add cases for the missing enumerators instead. I don't know why you think it's saying otherwise.

It says to add an empty default "if there is no default action and you mean to handle only specific cases" i.e. you do not want to handle all enumerators. That's talking about a different situation from the one you're describing. In your situation you want to handle all enumerators, so you would just add cases for all enumerators. Then you don't need a default.

Which is terrible for the reasons stated. The guidelines give the reason "Code clarity. Improved opportunities for error detection.", but in reality it's the opposite: using a default: case decreases code clarity and reduces the opportunities for error detection.

So add the missing cases for the other enumerators then.

IMO, it is better to avoid default: cases to allow the compiler to warn you about unhandled enumerators, to ensure enumerators are handled correctly rather than being silently funneled into a default catch-all that likely isn't going to do what you want for new enumerators.

So don't add a default case, and when you add new enumerators, add cases for those.

It would be better instead to suggest adding the missing cases explicitly:

So do that then. It will avoid the condition that the enforcement talks about, so would be following the guideline.

I don't understand why people keep interpreting this guideline to mean something it doesn't say.

[jwakely]

N.B. if you have 30 enumerators and you only want to handle 5 of them in the switch, you're saying it would be better to do:

switch (e) {
case e1:
  blah1();
  break;
case e2:
  blah2();
  break;
case e3:
  blah3();
  break;
case e4:
  blah4();
  break;
case e5:
  blah5();
  break;
case e6:
case e7:
case e8:
case e9:
case e10:
case e11:
case e12:
case e13:
case e14:
case e15:
case e16:
case e17:
case e18:
case e19:
case e20:
  etc etc
default:
}

Rather than just:

switch (e) {
case e1:
  blah1();
  break;
case e2:
  blah2();
  break;
case e3:
  blah3();
  break;
case e4:
  blah4();
  break;
case e5:
  blah5();
  break;
default:
  // nothing to do for all other values.
}

That seems pretty dumb to me. In a case like this, adding the default case makes a lot of sense. If you don't have so many enumerators, or you like the first example above, feel free to add them explicitly. The guidelines allow either form, so you can choose.

[jwakely]

The title of the guideline even reinforces it: "Use default to handle common cases (only)" i.e. don't use it if there isn't a common "everything else" case. So if you want to handle every enumerator explicitly and not have an "everything else" case, you should do that. Don't add a default for that situation, because there is no "common case" when you intend to handle each of them explicitly.

And if you want to be explicit about "all of these should be treated as the everything else case" then you can do that.

[kcat]

No, they're not saying that. You can add cases for the missing enumerators instead. I don't know why you think it's saying otherwise.

Because that's what it's saying. In the section titled, "Use default to handle common cases (only)", enforcement is described as "Flag switch-statements over an enumeration that don’t handle all enumerators and do not have a default". I don't know how else to read this other than "if you don't specify all enumerators and don't have a default case, add or suggest to add a default case." I'm apparently not the only one that interprets it this way.

If the purpose of the "(only)" is to basically say "(but not always)" and warn people away from doing it, after realizing it wasn't so good of an idea to do by default, that further questions its inclusion as a core guideline.

It says to add an empty default "if there is no default action and you mean to handle only specific cases" i.e. you do not want to handle all enumerators.

But a default case doesn't express that intent. It doesn't say "I mean to handle only specific cases listed above, everything else goes here" to me, it says "I didn't want to list out other possible enumerators and I wanted to shut the compiler up, hopefully nothing bad happens when a new one is added". Maybe I'm too used to lazy coders, but if the goal of the guidelines is to help adoption of modern practices for safer and more maintainable code, ES.79 is working against that.

Even if it is for a common case, seeing a default is a red flag that it may not be handling the enumerators properly, it can be silently missing one that should be handled in a non-default way or can easily and silently break when a new enumerator is added in the future. Contrary to its own reason, ES.79 reduces code clarity and inhibits error detection. This makes ES.79 bad advice, IMO.

So add the missing cases for the other enumerators then.

Which should be the suggested rule/guideline IMO, contrary to ES.79. To prefer specifying missing enumerators instead of using default to catch all unlisted ones. A default case is best used in situations where an enum can have an unknown/"invalid"/non-enumerator value, while also having non-swallowing behavior so you still get warned about unlisted enumerators. But as most compilers don't have a non-swallowing default, it's unsafe and should only be used as a last resort.

N.B. if you have 30 enumerators and you only want to handle 5 of them in the switch, you're saying it would be better to do:
[...]

That's where case ranges and enumerator markers would be helpful.

enum Enum { e0, e1, BeginGroupFoo, e2=BeginGroupFoo, e3, /*...*/, e20, EndGroupFoo=e20 };
...
switch(e)
{
case e0:
    blah0();
    break;
case e1:
    blah1();
    break;
case BeginGroupFoo ... EndGroupFoo:
    etc etc
    break;
}

This more clearly indicates e0 and e1 are handled special, and everything in "GroupFoo" is handled with the same path. If another enumerator is added inside of GroupFoo, it's handled the same as all the others in the group as one would expect, whereas if another enumerator is added outside of it, you get a warning and avoid a bug early.

In contrast, a default case swallows everything that's not listed, leaving it unclear if any of the unlisted enumerators were meant to be handled. Without case ranges to collect enumerators into explicit groups, listing the individual enumerators would be the safer and clearer option, so it's clear what each is intended to do and ensure none of the enumerators are missed. It's not pretty, but being safe isn't always pretty.

[jwakely]

"Flag switch-statements over an enumeration that don’t handle all enumerators and do not have a default".

That does not say "add a default". It says when static analysis tools should complain. If you add cases to handle all enumerators, the tools should not complain. So adding cases to handle all enumerators is a valid way to follow this guideline.

The rest of your answer is just repeating "default bad default bad" again. I get it, you've made your point. I don't agree default cases are always bad, but I get it. It's not hard to understand your position, you don't need to keep repeating it.

But the guideline does not say "always have a default case". It says "do not have unhandled cases". It's really that simple.

If you don't want to use default, don't. As long as you handle all cases in some form then analyzers should not say you're failing to follow ES.79.

Maybe the title or summary of ES.79 needs to be clarified for people misreading it but the issue says it's "terrible advice and should be dropped", based on ... something it doesn't actually say. "Do not have unhandled cases in a switch" is not terrible advice, and should not be dropped.

[kcat]

Maybe the title or summary of ES.79 needs to be clarified for people misreading it but the issue says it's "terrible advice and should be dropped", based on ... something it doesn't actually say.

At least in my case, it's both the title and provided examples, along with reasoning.

The title sets the context, so the section is interpreted in that context. Here, the title is "Use default to handle common cases (only)", so the explanation of missing cases, the reasoning of code clarity and safety, and what to do is going to be read in the light of "Use default to handle common cases" (and the added "(only)" at the end is confusing, like it's an incomplete aside statement; "only ... what?").

It harps on using default to prevent problems, how the first example is "clear that there is a default action and that cases a and b are special" (which is debatable), and for the second example where you don't need to do anything for some cases, it says "have an empty default or else it is impossible to know if you meant to handle all cases".

The section just reads as "use default, or else problems", not "handle all enumerators, either explicitly or with default". "Handle all enumerators" isn't mentioned until the end in the Enforcement paragraph, after the reader is told to use default and the problems of leaving out a default. Whether it intends to or not, ES.79 comes across as "Use default in your switches", which I would indeed call terrible general advice (I won't go as far as to say it should never be used or removed from the language, but I don't think a swallowing default should be called clear and safe, either).