2

I'm studing C# now, and came across the following situation, what's the better pratice, duplicate the code like "EX 1" or use goto statement like "EX 2"? I don't want a personal opnion.

        // EX 1: 

        switch (a)
        {
            case 3:
                b = 7;
                c = 3; // duplicate code <-|
                break; //                    |
            case 4:    //                    |
                c = 3; // duplicate code --|
                break;
            default:
                b = 2;
                c = 4;
                break;
        }


        // EX 2: 

        switch (a)
        {
            case 3:
                b = 7;
                goto case 4; // not duplicate code and use goto statement
            case 4:
                c = 3;
                break;
            default:
                b = 2;
                c = 4;
                break;
        }
Guilherme Oliveira
  • 2,008
  • 3
  • 27
  • 44
Only a Curious Mind
  • 2,807
  • 23
  • 39
  • 1
    To quote the [C# language specifications](http://msdn.microsoft.com/en-us/library/aa664749.aspx), "When execution of a switch section is to be followed by execution of another switch section, an explicit `goto case` or `goto default` statement must be used". – GSerg Mar 27 '14 at 13:46
  • In this particular example (3 cases total), I would more likely use an if/else construct. – plinth Mar 27 '14 at 13:47
  • 2
    You asked a question that *requires* a personal opinion ("which is better"), and yet state you "don't want a personal opinion". Make up your mind - you can't have it both ways. – Ken White Mar 27 '14 at 13:50
  • Well ideally I'd be saying a,b & c were related and so should be in a a class or a method of the class, and then factor the entire thing out. Unless it's lot of effort for very little, I like my cases to be one liners, but that's a personal preference. – Tony Hopkinson Mar 27 '14 at 13:52
  • possible duplicate of [Switch statement fallthrough in C#?](http://stackoverflow.com/questions/174155/switch-statement-fallthrough-in-c) – GSerg Mar 27 '14 at 13:58
  • Why put on-Hold my question? I've written "I don't want a personal opnion", So, this isn't a "based on opinions" question. – Only a Curious Mind Mar 27 '14 at 14:35

4 Answers4

5

It really depends.

Is case 3 a special case of case 4?

In that situation then a goto may be in order, because if we at a later point in time add some new behaviour to case 4 then we will get that automatically for case 3 as well.

If case 3 & 4 are unrelated, then duplicating code is better.

If your real case is this small, with so few lines, I would prefer duplicating code, because of simplicity and readability.

Cyberwiz
  • 11,027
  • 3
  • 20
  • 40
1

I personally dislike goto since it makes your code less easy to understand and reproduce.

I see no major issues with your first code sample as it is. If you need to, you could also split processing of b and c if that makes sense.

You should consider what is more important:

  • Code readability;
  • Minimal number of lines of code;
  • How often does this code change? When it is changing a lot and the dependency might get lost, you probably don't want to use goto.
Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325
1

Example 1 pros and cons

+ Common structure
+ Simple to understand logic
- More lines of code
- Code repetition

Example 2 pros and cons

+ Fewer lines of code
+ No code repetition
- It complicates logic
- It is not commonly used in production code

Bottom line

I would prefer example 1, because, in this particular instance, the savings are minimal, but the logic gets more complicated. Goto, arguably, increases the chances of a bug if more people starts working on the same code, as difficulty of code increases. Let's have a look at this embarrassing bug. Had developers not used a goto, there wouldn't be such a problem!

Bonus points

  • You can use enums for case selection, so case 3: => case CarPart.SteeringWheel
  • make sure each case has a break;
  • make sure there is a default case
  • consider using polymorphism and inheritance instead of a switch case

    ICarPart part1 = new SteeringWheel();
    ICarPart part2= new Mirror();
    var parts = new List<ICarPart>() {part1, part2};
    
    // now call your original method on the parts
    // no more need for a switch case 
    
oleksii
  • 35,458
  • 16
  • 93
  • 163
  • The bug you linked has nothing to do with `goto`, but with using `if` clauses without curly brackets. Also, using `goto` for clean-up is a common and accepted idiom in C. – sloth Mar 27 '14 at 14:12
  • It is indeed used in C a lot. But not in C#. And it is a combination of `if` and `goto`, so I wouldn't say it *has nothing to do with goto`* – oleksii Mar 27 '14 at 14:20
1

In general, using goto is considered to be bad practice (and rightfully so), but using goto solely for a forward jump out of structured control statements is usually considered to be OK, especially if the alternative is to have more complicated code.

Here's an example:

for (...) {
    for (...) {
        ...
        if (something)
            goto end_of_loop;
    }
}

end_of_loop:

Here you check some other acceptable usages of goto.

So, goto would be considered a bad practice. But, as I said, it still can be used.

Guilherme Oliveira
  • 2,008
  • 3
  • 27
  • 44