1

When writing a switch statement that only ever has to deal with a known set of values (imagine an implicit enumeration), I find myself wondering what should be the last entry in the construct. I'm always considering the same 3 approaches, but I'd really like to just stick with the one that makes the most sense and never think about this again. The alternatives are described below. For the sake of discussion, let's assume there are exactly 4 options for the "switching variable".

  • Approach #1: 4 cases, no default case.
  • Approach #2: 3 cases, default acts as the last case (+ comment explaining this).
  • Approach #3: 4 cases, default case with a "ShouldNeverHappenException".

My thoughts on this are as follows:

  • One the one hand, since default is effectively a case that cannot be reached, it seems pointless to clutter the code with it.
  • On the other hand, it's bad not to handle a default case out of of future-proofing considerations, that is, if someday another option becomes available (due to e.g. API changes outside of my control), the code might react to it incorrectly if the default case is used for one of the expected options.
  • If the last case handles an error scenario, it might not be so bad if it's the default behavior (assuming I have no fine-grained error handling).

Based on the above reasoning I tend to prefer the 3rd approach, but I am no software engineer, and perhaps I'm missing something.

What are the best practices in this case?

Dev-iL
  • 233
  • 1
  • 10
  • 4
    Sorry, but being a software developer requires you to think. The correct answer is "use a default case when you need one." – Robert Harvey Jul 19 '18 at 15:42
  • @RobertHarvey So what you're saying, in other words, is that I should consider these options each time and decide on a per-case basis? Would you say that all approaches are equally valid? – Dev-iL Jul 19 '18 at 15:52
  • I would avoid approach 3. – Robert Harvey Jul 19 '18 at 15:55
  • @RobertHarvey Would you be able to elaborate please? If it's just the "comical" aspect of the exception, it can obviously be turned into a more meaningful "UnhandledScenarioException" etc. – Dev-iL Jul 19 '18 at 15:59
  • Approach 3 is a "you don't need one" case. – Robert Harvey Jul 19 '18 at 16:14
  • Perhaps I misunderstand - but why don't I need it? If I get an unexpected value, I'd like to know that my code didn't handle/recognize it at all, instead of silently skipping the entire `switch` block (which is what happens when none of the cases were met and there's no `default`). As you said in your now-deleted comment, this can be solved with an `assert(isValueInExpectedRange(val))`, but then it might be difficult to validate the value... – Dev-iL Jul 19 '18 at 16:19
  • 1
    Then you need it, so put it in. If your peace of mind requires the default case, by all means, put it in. – Robert Harvey Jul 19 '18 at 16:21
  • @RobertHarvey Approach 3 is the only sensible choice given that enums change over time and can be derived from poorly controlled sources like deserialization. – Eric Jul 19 '18 at 17:12
  • @Eric: Then you need it, so put it in. If your code base has enums that are going to change frequently without your developers going back and checking all of the places where they are used, then by all means, put it in. Wouldn't it be better if the developers did what they're supposed to do? – Robert Harvey Jul 19 '18 at 17:20
  • @RobertHarvey, yes but _Defensive programming_ :) – Dev-iL Jul 19 '18 at 17:20
  • @Dev-iL: I'm not going to tell anyone that they *have* to do anything. We already have too many people in our industry that can't think for themselves. – Robert Harvey Jul 19 '18 at 17:22
  • @RobertHarvey Yes, imagine all the money we could save by firing all the QA staff and how much more productive we could be without writing all those pesky unit tests. – Eric Jul 19 '18 at 17:23
  • @Eric: Um, what? You're not actually proposing that always putting a default case in a switch statement would allow us to do that, are you? – Robert Harvey Jul 19 '18 at 17:24
  • @RobertHarvey No, but if programmers just did what they were supposed to do, we could. A programmer's job is not to create bugs after all. – Eric Jul 19 '18 at 17:25
  • @Eric: Yeah, I never made that assertion, nor would I. People make mistakes. The one where someone adds an item to an enum but forgets to add a case to a switch statement will become abundantly obvious when the unit test for the new case fails. – Robert Harvey Jul 19 '18 at 17:27
  • Switch statement might be generating an abstract factory and require a default implementation if none provided. You wouldn’t want your animal factory to default to no animals would you? Car factory? Burger factory? You are thinking of case statement if a user would provide input but that is not always the case: default thinking. – StackOverFowl Jul 19 '18 at 19:25
  • Handling a case that should never happen is called defense in depth. I consider it good practice, but I don't use an exception I just add an ’assert(!"explain why this should not happen")’. – kamikaze Jul 19 '18 at 20:29
  • Switches and enums differ radically between languages so I don't think there can be a language-agnostic answer. E.g. some languages can guarantee that no switch is non-exhaustive or has unreachable branches, in which case the future-proofing argument doesn't apply. – amon Jul 20 '18 at 08:50

4 Answers4

7

imagine an implicit enumeration

I think this is the key point. Implicit means not an actual enum type, but, say, numbers with special meaning.

const int A = 1;
const int B = 2;
const int C = 3;

And a method that is using the switch statement:

public void DoSomething(int type)
{
    switch (type)
    {
        case A:
            // Stuff
            break;
        case B:
            // Stuff
            break;
        case C:
            // Stuff
            break;
        default:
            // Do I even needs this?
            break;
    }
}

The "implicit" enumeration in this example is a 32 bit integer. The entire range of values for the 32 bit integer are your possible cases. So what if I call:

DoSomething(355);

Or:

DoSomething(-2);

You might say "I'm always using the constants A, B and C in my program when calling this method, so it's impossible to pass -1"

But the compiler allows it, therefore a code change in the future in how DoSomething is called can pass an illegal value.

If you pass an illegal value, is there an intelligent default you can fall back on? If so, that's your default. If not, throw an exception.

I would argue for a default even for an explicit enumeration:

enum Foo
{
    A = 1,
    B = 2,
    C = 3
}

public void DoSomething(Foo type)
{
    switch (type)
    {
        case A:
            // Stuff
            break;
        case B:
            // Stuff
            break;
        case C:
            // Stuff
            break;
        default:
            throw new NotImplementedException("Type " + type + " is not yet implemented");
            break;
    }
}

The default clause throwing an exception to the fact a value is not implemented is a good debugging tool for the future where you add an item to the enum, and then you forget to change the application behavior based on that.

Will the exception happen in production? Most likely not. It will probably happen during development or testing and be fixed.

But that's the benefit of having a default clause and either doing something by default or blowing up.

There are some cases where doing nothing is OK. I honestly would still put in a default clause with a comment:

public void DoSomething(int type)
{
    switch (type)
    {
        case A:
            ...
            break;
        case B:
            ...
            break;
        case C:
            ...
            break;
        default:
            // Do nothing intentionally
            break;
    }
}

To me this falls under the same guidelines as exception handling. If you catch the exception, do something with it. If you don't want to do anything with it, catch the exception and explain why nothing is done:

try
{
    ...
}
catch(Exception)
{
    // Do nothing, because we are trying to do something and we
    // really don't care if it happens.
}
Greg Burghardt
  • 34,276
  • 8
  • 63
  • 114
  • Thank you for your input! I think I'll adopt this reasoning. – Dev-iL Jul 19 '18 at 16:58
  • I would say that whether or not to use the default that throws an exception vs not having one also depends on the language. Java for example will throw a compiler warning about missing a case. If the decision is between a runtime exception and a compiletime warning/error I would go for the latter. – Hangman4358 Jul 19 '18 at 17:48
  • @Hangman4358: Good point. Me too. – Greg Burghardt Jul 19 '18 at 17:51
  • In your last code block, you demonstrate what I know to be a very dangerous practice (the empty `catch`). AFAIK, one should either handle the exception or log the error (or maybe both). Not only do try/catch have an impact on performance, a construct like that can be interpreted as code smell. OTOH, blindly sticking to old convictions is not always good, and after all, I'm here to learn... Do you have an example for when an empty `catch` block is the way to go? I can only think of the case of early bailout from a nested loop. – Dev-iL Jul 19 '18 at 18:10
  • 1
    @Dev-iL: An empty catch **without a comment** is a code smell. Sometimes (like when all the planets align) you want to catch an exception and literally do nothing - but if you do nothing *explain why*. It might be a good reason. – Greg Burghardt Jul 19 '18 at 18:22
  • @Dev-iL: I like to approach switch statements without the need for a default clause in the same way. If I'm doing something based on a value, and there is no intelligent default, and it doesn't matter that nothing is done, explain why nothing is done. I'm always worried about unhandled values in switch statements without a default clause. – Greg Burghardt Jul 19 '18 at 18:24
2

I remember early in my career writing a series of if else if statements which I thought covered all cases.

At the end I added:

else 
{
    Throw new Exception("THE IMPOSSIBLE HAPPENED!!");
}

Of course some months later my boss approached me with a worried look on his face, "Ewan, what does 'The impossible happened' mean?!".

It means you have forgotten about nulls, that's what it means

Ewan
  • 70,664
  • 5
  • 76
  • 161
  • I think my situation is more similar to [this](https://stackoverflow.com/a/1125805/) :) – Dev-iL Jul 19 '18 at 15:56
  • @Ewen, in my case it was someone added a new enumeration value without updating the switch statement. But that was a project in a language that restricted enums to non-nullable values. – Berin Loritsch Jul 19 '18 at 16:46
0

If you're really dealing with an enumerated type, for which there are a fixed and relatively small set of values, I see no real reason to have a default case:

  • Using it as a form of "pre-debugging" to catch your potential errors for you seems to have little value. In fact, it's totally useless if you've written your switch right the first time. You could just take the extra time you would have spent adding the default case to give your switch another once-over for errors.

  • Using it as a form of future-proofing to catch potential errors from later code modifications seems to have little value as well. Say someone adds more enumerations later. If they are diligent, they will correctly add their new case to the switch and the default would again have served no purpose. And if they are not diligent in paying attention to the existing switch code, your future-proofing could just as easily cause a problem instead of solving it. Maybe they wanted the switch to do nothing at all with their new enumeration (i.e. no default case), but they failed to see you have put a default in. Your future-proofing would have then made a little extra work for them.

Ultimately, it will depend on your situation and how well "known" your set of values is. If the set of values is large, or you don't trust in your capabilities to completely "know" what they are or could be, maybe a catch-all default is worth the added peace of mind.

gnovice
  • 109
  • 1
  • 1
  • 6
  • When you say "cause a problem", I understand this as _correctly_ throwing an exception that means "hey, you need to modify this `switch` to suit your new behavior", after which, said not-diligent person will now notice the missed problem => purpose served. – Dev-iL Jul 19 '18 at 16:38
  • @Dev-iL: But if their assumption is "I'm adding this new enumeration, and the existing code should ignore it and do nothing special when encountered (for now)", then an unexpected/unnecessary default *is* the problem. – gnovice Jul 19 '18 at 16:43
  • 2
    @gnovice, the exception would alert them to the fact that their assumption is in fact incorrect. If they want to actively ignore the enum value, they can easily add the case statement and `break` right after. When your enums are part of a Finite State Machine, then that approach is absolutely correct. It's better not to assume anyway. – Berin Loritsch Jul 19 '18 at 16:50
  • @BerinLoritsch: You make a good point. I guess my concern is putting too much time/effort into future-proofing without knowing how or if it will even be necessary. I guess I'm of the mindset that it's better to not make assumptions on what someone will want or need in the future and let them handle things themselves. – gnovice Jul 19 '18 at 17:11
0
switch country
US: minimum_drinking_age = 21
Spain: minimum_drinking_age = 16
default: minimum_drinking_age = 18

I just pulled these numbers out of my nose. In there, I assume in US the legal age of drinking is 21+, in spain its 16+ and in every other country in the world its 18+.

This is just a random scenario. There are just 2 cases where a value must be set specifically, in every other case, it will default to one value. Why would you throw an exception? Makes no sense. The default is used for defaults. Misusing it for other purposes is besides the point!

According to this it seems like I was not quite off (18 is default)!

Aphton
  • 98
  • 1
  • 4
  • This is a good example of why the "right" solution is problem-dependent. In your example, it might be a question of _resolution_, as there are about 200 countries in the world, but you only want to be precise with respect to a small subset. In other scenarios you might be very particular about handling each and every case correctly (let's say you can a `case` for every country, and all of a sudden a new country forms, so now you want to force the programmer to adjust the code such that it handles it correctly. {P.S. not that a `switch` with 200 cases is a good idea}). – Dev-iL Jul 19 '18 at 17:18
  • Well, I dont use switch. I use polymorphism! – Aphton Jul 19 '18 at 18:58