Switch statements can help clarity

rstinejr

I yelped just now when I ran across the following, a switch block with exactly one case.

 switch (cmd)
 {
     case Acme.CommandType.VerifyResponse:
         if ( ! this.VerifyResponse(cmd.stuff) )
         {
             throw new AcmeResponseException("Response is not valid");
         }
         break;
  }

I assume the author intended, perhaps still intends to add more cases. But somehow I'm reminded of a spoof about a prolific author, "...the only man who has written more books than he has read."

pkmnfrk

Seems fine to me. If there is only one CommandType, then there only need be one switch case.

My only criticism is that there is no default handler in case of an unforseen addition to the CommandType enum.

rstinejr

@pkmnfrk said:

Seems fine to me. If there is only one CommandType, then there only need be one switch case.

My only criticism is that there is no default handler in case of an unforseen addition to the CommandType enum.

You really think that the switch is more clear than an if statement? The switch & case baggage adds a bunch of clutter for no purpose that I can discern.

This is beyond the scope of the original post, but in the code the "cmd" enum has many values, and the overly-long function that the example comes from has several switch statements on the same enum.

zmafoox

No, if you're switching on an enum the last thing you want is a default case. That way, when you add a new CommandType the compiler directs you to exactly where other changes are needed.

Sutherlands

@zmafoox said:

No, if you're switching on an enum the *last* thing you want is a default case. That way, when you add a new CommandType the compiler directs you to exactly where other changes are needed.

Uh... no it doesn't.

rstinejr

Um, back to the switch statement with only a single "case"...

IIRC, The Art of Readable Code makes the point that if you're doing the same old thing, it helps clarity to do it the same old way. This makes it easy for a reader to pick out and recognize the various parts and pieces.

In the WTF sample, the one "case" select block boils down to an overly verbose "if" statement; it should have been implemented as a plain old "if" statement.

There would have been better ways to mark this block as a place for possible expansion (e.g., add a "TBD" coment). And my bet is that the anticipated expansion will never happen; YAGNI, and all that.

error

@Sutherlands said:

@zmafoox said:
No, if you're switching on an enum the last thing you want is a default case. That way, when you add a new CommandType the compiler directs you to exactly where other changes are needed.
Uh... no it doesn't.

Switching on an enum, the default case should throw an exception and all acceptable values should do something or just break out.

zmafoox

@Sutherlands said:

@zmafoox said:
No, if you're switching on an enum the last thing you want is a default case. That way, when you add a new CommandType the compiler directs you to exactly where other changes are needed.
Uh... no it doesn't.

Please explain as this works in both C & C++. Any language worth it's salt should complain if an enumeration is not handled in a switch statement.

morbiuswilters

@zmafoox said:

Please explain as this works in both C & C++. Any language worth it's salt should complain if an enumeration is not handled in a switch statement.

I really wouldn't consider C++ a language that's worth much of anything, but I wouldn't mind seeing this in other languages.

Sutherlands

@zmafoox said:

@Sutherlands said:
@zmafoox said:
No, if you're switching on an enum the *last* thing you want is a default case. That way, when you add a new CommandType the compiler directs you to exactly where other changes are needed.

Uh... no it doesn't.

Please explain as this works in both C & C++. Any language worth it's salt should complain if an enumeration is not handled in a switch statement.

enum MyEnum {First, Second}

void Main()

{

Print(MyEnum.First);

Print(MyEnum.Second);

}

void Print(MyEnum e)

{

switch(e)

{

case MyEnum.First:

System.Console.WriteLine("First");

break;

}

System.Console.WriteLine("End");

}

Results:

First
End
End

morbiuswilters

@Sutherlands said:

@zmafoox said:
@Sutherlands said:
@zmafoox said:
No, if you're switching on an enum the last thing you want is a default case. That way, when you add a new CommandType the compiler directs you to exactly where other changes are needed.

Uh... no it doesn't.

Please explain as this works in both C & C++. Any language worth it's salt should complain if an enumeration is not handled in a switch statement.

 
enum MyEnum {First, Second}
void Main()
{

Print(MyEnum.First);

Print(MyEnum.Second);

}
void Print(MyEnum e)
{
switch(e)
{
case MyEnum.First: 
System.Console.WriteLine("First");
break;
}
System.Console.WriteLine("End");
}

Results:

First
End
End

Right, but it's a warning in C or C++ to have a switch on an enum and not include paths for all types (apparently this guy has never used a language outside of C++ and C--so sad), so he's saying other languages should do the same. I see value in it, although it should also be able to be explicitly suppressed should you want to.

Sutherlands

Guys, zmafoox said that the compiler will point you to where you missed addressing it. It won't. That's all I'm addressing.

arotenbe

I have Eclipse set up to warn on enum switches that don't match against a constant in Java. I usually code such switches in a separate method like this:

switch (someValue) {
    case CONST1:
        ...
        return ...;
    case CONST2:
    case CONST3:
        ...
        return ...;
}
throw new AssertionError(someValue);

That way, the compiler will catch it, and it will fail at runtime if new constants have been added.

pkmnfrk

@arotenbe said:

I have Eclipse set up to warn on enum switches that don't match against a constant in Java. I usually code such switches in a separate method like this:
switch (someValue) {
    case CONST1:
        ...
        return ...;
    case CONST2:
    case CONST3:
        ...
        return ...;
}
throw new AssertionError(someValue);
That way, the compiler will catch it, and it will fail at runtime if new constants have been added.

This is basically the pattern I was referring to when I suggested having a default. That is all.

too_many_usernames

@morbiuswilters said:

Right, but it's a warning in C or C++ to have a switch on an enum and not include paths for all types

Which compilers? I don't think I've ever seen this. Perhaps because I always have a default case?

Jaime

@too_many_usernames said:

@morbiuswilters said:
Right, but it's a warning in C or C++ to have a switch on an enum and not include paths for all types

Which compilers? I don't think I've ever seen this. Perhaps because I always have a default case?

Visual Studio 2010 definitely has it.

PJH

@too_many_usernames said:

@morbiuswilters said:
Right, but it's a warning in C or C++ to have a switch on an enum and not include paths for all types
Which compilers? I don't think I've ever seen this.

GCC for one can be made to emit a warning on a missing case, but (without scouring The Standards) I'm fairly certain it's not a required diagnostic.

[pjh@sofa tmp]$ cat switch.c
#include <stdio.h>
#include <stdlib.h>

enum foo{
        bar = 2,
        baz = 3,
        qux = 4
};

int main(void){
        enum foo quux = 2 + (rand()%2);
        switch (quux){
                case bar: printf("bar\n"); break;
                case baz: printf("baz\n"); break;
        }
        return 0;
}
[pjh@sofa tmp]$ gcc switch.c -o switch
[pjh@sofa tmp]$ gcc switch.c -o switch -Wswitch
switch.c: In function ‘main’:
switch.c:12:2: warning: enumeration value ‘qux’ not handled in switch
[pjh@sofa tmp]$

Perhaps because I always have a default case?

-Wswitch-enum will emit a warning even if you have a default in gcc