switch case programming practice

Question

enum SQLErrorCode{
      OK = 0,
      PARTIAL_OK = 1,
      SOMEWHAT_OK = 2,
      NOT_OK = 3,
};

Code 1:

int error = getErrorCode();
if((error == SQLErrorCode.PARTIAL_OK) ||
  (error == SQLErrorCode.SOMEWHAT_OK) ||
  (error == SQLErrorCode.NOT_OK) ||
  (error < 0))
   callFunction1();
else
    callFunction2();

Code 2:

switch(error){
       case SQLErrorCode.PARTIAL_OK: 
                                    callFunction1();
                                    break;
        case SQLErrorCode.SOMEWHAT_OK:
                                    callFunction1();
                                    break;
        case SQLErrorCode.NOT_OK: 
                                    callFunction1();
                                    break;
        default:
                                    callFunction2();
                                    break;
}

Which method should I prefer. As far as performance is considered there should not be much difference. How to handle error < 0 condition in switch case.

EDIT: Joel's solution:

switch(error) {
     case SQLErrorCode.PARTIAL_OK: 
     case SQLErrorCode.SOMEWHAT_OK:
     case SQLErrorCode.NOT_OK: 
         callFunction1();
         break;
     case SQLErrorCode.OK:
         callFunction2();
         break;
     default:     // error < 0 is handled
         callFunction1();
         break;
}

Q. error < 0 is handled. If I have to handle other numbers in error which do not belong to any of the cases here including default.

You are talking about C here? Not C#? The `SQLErrorCode.` prefix isn't legal C. — quark, Aug 06 '09 at 20:54
This is tagged C, but C doesn't allow enumtype.enumval. Did you mean to tag it C#? — Rob K, Aug 06 '09 at 21:04
Most of the switch examples are changing the meaning of the original code. Their meaning is more like: "if( .... || (error < 0) || (error>NOT_OK) )". And if that is really what you want, then just have: if (error != 0). — Richard Corden, Aug 07 '09 at 08:00
I've modified my answer for your edit. The new one should handle all cases. Basic concept: handle known cases with specific case statements and everything else in the default. — Joel, Aug 07 '09 at 14:37

score 12 · Answer 1 · answered Aug 06 '09 at 20:35

12

Without expressing preference for which is best, here is another possibility:

switch(error){
    case SQLErrorCode.PARTIAL_OK: 
    case SQLErrorCode.SOMEWHAT_OK:
    case SQLErrorCode.NOT_OK: 
                                callFunction1();
                                break;
    default:
                                callFunction2();
                                break;
}

answered Aug 06 '09 at 20:35

Adam Batkin

51,711
9
123
115

Much better than the original switch function. – David Thornley Aug 06 '09 at 20:36
Oops!! I missed that :| My bad! – pankajt Aug 06 '09 at 20:36
Hey, we're all allowed to screw up once... but next time... you're done for :) – Matthew Whited Aug 06 '09 at 20:43
This would be my preference. Related question: http://stackoverflow.com/questions/188461/switch-statement-fallthrough-should-it-be-allowed – Fred Larson Aug 06 '09 at 20:43
Ok, this one is "elegant" but I wouldn't use this method. Why, you may ask? This one can be very dangerous when you suddenly wants to handle SOMEWHAT_OK with another function. It is easy to break the whole logic here. I didn't down this answer, but David's comment is aligned with what I'm thinking. – wtaniguchi Aug 06 '09 at 20:49
-1: Despite all the happy up votes, this doesn't answer the OP's question about error valuess outside the known range. – Joel Aug 07 '09 at 14:38
Sure it does. That's what the `default` is there for (well, the code may not work exactly the way the poster wants it to, but the example is - by design - vague). This was meant to just illustrate another way of laying out the code since when I wrote it, everything from "EDIT: Joel's solution" to the bottom, wasn't there. – Adam Batkin Aug 07 '09 at 15:05

score 6 · Accepted Answer · answered Aug 06 '09 at 21:12

Not that it matters much for such a small number of cases but switch is in fact faster for integers: it can be, and often is, implemented as a jump table instead of a series of conditional checks.

As a comparison up the number of different cases to 10:

enum SQLErrorCode{
    CODE0 = 0,
    CODE1 = 1,
    CODE2 = 2,
    CODE3 = 3,
    CODE4 = 4,
    CODE5 = 5,
    CODE6 = 6,
    CODE7 = 7,
    CODE8 = 8,
    CODE9 = 9
};

enum SQLErrorCode getErrorCode();

void run()
{
    int error = getErrorCode();
#ifdef CASE1
    if((error == CODE0) ||       
       (error == CODE1) ||
       (error == CODE2) ||
       (error == CODE3) ||
       (error == CODE4) ||
       (error == CODE5) ||
       (error == CODE6) ||
       (error == CODE7) ||
       (error == CODE8) ||
       (error == CODE9) ||
       (error < 0))
        callFunction1();
    else
        callFunction2();
#endif
#ifdef CASE2
    switch(error)
    {
        case CODE0:
            callFunction1();
            break;
    case CODE1:
        callFunction1();
        break;
    case CODE2:
        callFunction1();
        break;
    case CODE3:
        callFunction1();
        break;
    case CODE4:
        callFunction1();
        break;
    case CODE5:
        callFunction1();
        break;
    case CODE6:
        callFunction1();
        break;
    case CODE7:
        callFunction1();
        break;
    case CODE8:
        callFunction1();
        break;
    case CODE9:
        callFunction1();
        break;
    default:
        callFunction2();
        break;
}
#endif

}

Now look at the assembly generated by doing the first case vs. the second, when built on Linux using GCC.

If you look at the assembly you ll see a significant difference (for larger statements): the series of ||s (or if/else if you do it that way) is a series of branches taken one at a time. The switch turns into a big table: it takes up more code but can mean that it can be handled in one jump.

(Incidentally, we are talking about C here right? Not C#? The code you have won't compile: in C enumerators don't use the enumeration name as a prefix. So it's PARTIAL_OK without the SQLErrorCode.)

Code 1: cc -DCASE1 -s switch.s switch.c

        .file   "1241256.c"
        .text
.globl run
        .type   run, @function
run:
        pushl   %ebp
        movl    %esp, %ebp
        subl    $24, %esp
        call    getErrorCode
        movl    %eax, -4(%ebp)
        cmpl    $0, -4(%ebp)
        je      .L2
        cmpl    $1, -4(%ebp)
        je      .L2
        cmpl    $2, -4(%ebp)
        je      .L2
        cmpl    $3, -4(%ebp)
        je      .L2
        cmpl    $4, -4(%ebp)
        je      .L2
        cmpl    $5, -4(%ebp)
        je      .L2
        cmpl    $6, -4(%ebp)
        je      .L2
        cmpl    $7, -4(%ebp)
        je      .L2
        cmpl    $8, -4(%ebp)
        je      .L2
        cmpl    $9, -4(%ebp)
        je      .L2
        cmpl    $0, -4(%ebp)
        jns     .L13
.L2:
        call    callFunction1
        jmp     .L15
.L13:
        call    callFunction2
.L15:
        leave
        ret
        .size   run, .-run
        .ident  "GCC: (GNU) 4.2.4 (Ubuntu 4.2.4-1ubuntu4)"
        .section        .note.GNU-stack,"",@progbits

Code 2: cc -DCASE2 -s switch.s switch.c

        .text
.globl run
        .type   run, @function
run:
        pushl   %ebp
        movl    %esp, %ebp
        subl    $24, %esp
        call    getErrorCode
        movl    %eax, -4(%ebp)
        cmpl    $9, -4(%ebp)
        ja      .L2
        movl    -4(%ebp), %eax
        sall    $2, %eax
        movl    .L13(%eax), %eax
        jmp     *%eax
        .section        .rodata
        .align 4
        .align 4
.L13:
        .long   .L3
        .long   .L4
        .long   .L5
        .long   .L6
        .long   .L7
        .long   .L8
        .long   .L9
        .long   .L10
        .long   .L11
        .long   .L12
        .text
.L3:
        call    callFunction1
        jmp     .L15
.L4:
        call    callFunction1
        jmp     .L15
.L5:
        call    callFunction1
        jmp     .L15
.L6:
        call    callFunction1
        jmp     .L15
.L7:
        call    callFunction1
        jmp     .L15
.L8:
        call    callFunction1
        jmp     .L15
.L9:
        call    callFunction1
        jmp     .L15
.L10:
        call    callFunction1
        jmp     .L15
.L11:
        call    callFunction1
        jmp     .L15
.L12:
        call    callFunction1
        jmp     .L15
.L2:
        call    callFunction2
.L15:
        leave
        ret
        .size   run, .-run
        .ident  "GCC: (GNU) 4.2.4 (Ubuntu 4.2.4-1ubuntu4)"
        .section        .note.GNU-stack,"",@progbits

Yes, that's with the default (the command lines shown are the one I used). — quark, Dec 16 '09 at 19:52

Joel · Answer 3 · 2009-08-07T14:34:58.347

4

Why not...

switch(error) {
    case SQLErrorCode.PARTIAL_OK: 
    case SQLErrorCode.SOMEWHAT_OK:
    case SQLErrorCode.NOT_OK: 
         callFunction1();
         break;
    case SQLErrorCode.OK:
         callFunction2();
         break;
    default:
         if (error < 0)
              callFunction1();
         else
              callFunction2();
         break;
}

Easier to write than your switch, and easier to read than your if. Yet it still handles error < 0.

EDIT:

Richard brings up a good point. I've edited to handle positive and negative errors outside the known range.

edited Aug 07 '09 at 14:34

answered Aug 06 '09 at 20:36

Joel

19,175
2
63
83

1

I voted for this one although I prefer to have break aligned with the word case myself. – KPexEA Aug 06 '09 at 23:17
What happens when 'error' is a positive value other than one of those listed in the enumeration? for example 10? In the original code, "callFunction2" would be called, however, in your code "callFunction1" is called. This behaviour may not be "wrong", however, it is different to the original code. – Richard Corden Aug 07 '09 at 07:55

score 2 · Answer 4 · answered Aug 06 '09 at 20:55

2

Assuming getErrorCode() returns one of your enumerated values or something less than 0, how about

int error = getErrorCode();
if (error == SQLErrorCode.OK)
  callFunction2(); // Good path
else
  callFunction1(); // Error / not good enough path

Obviously, if your code needs to callFunction2() on error > 3, then this doesn't work.

answered Aug 06 '09 at 20:55

Phil Miller

36,389
13
67
90

I was about to suggest the same thing. – Rob K Aug 06 '09 at 21:01

JRL · Answer 5 · 2009-08-06T21:13:59.827

2

You could also write a function that will determine what an OK error or not OK error code is:

bool isOK(int code)
{
  return code == SQLErrorCode.OK;
}

and your code could become:

if (isOk(getErrorCode()))
{
  callFunction2;
}
else
{
  callFunction1;
}

edited Aug 06 '09 at 21:13

answered Aug 06 '09 at 21:08

JRL

76,767
18
98
146

+1: This is the only answer that seems to have grasped that the switch statements are overkill for saying " != OK". – Richard Corden Aug 07 '09 at 08:02

score 1 · Answer 6 · answered Aug 06 '09 at 20:36

I haven't touched C in awhile, but doesn't it have fall through? So you could write the second chuck as such...

switch(error){
   case SQLErrorCode.PARTIAL_OK: 
    case SQLErrorCode.SOMEWHAT_OK:
    case SQLErrorCode.NOT_OK: 
                                callFunction1();
                                break;
    default:
                                callFunction2();
                                break;

}

score 1 · Answer 7 · answered Aug 06 '09 at 20:39

Whenever you have multiple ways to get the same effect, you're causing confusion. Since, in the switch version, you've got different cases for SqlErrorCode.PARTIAL_OK and SqlErrorCode.SOMEWHAT_OK, the implication is that they have different processing. It takes some study to see what is happening (and it isn't completely compatible with your if-statement handling, which probably means it confused you).

In this case, I'd use an if statement, since the idea is to use one function or another.

You are very true here. I am confused here about which one should I prefer. — pankajt, Aug 06 '09 at 20:45

switch case programming practice

7 Answers7

Linked