Search code examples
c#goto

What is the better pratice: duplicate code or use goto statement?


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;
        }

Solution

  • 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