Htydjtk

I have a switch structure that has several cases to handle. The switch operates over an enum which presents duplicate code issues:

public enum ExampleEnum {

    One,

    OneAndTwo,

    OneAndThree,

    Two,

    TwoAndThree,

    Three

}

When I enter the switch structure, the functionality for One, Two, and Three are the same even when coupled together. For example:

int i = 0;

if (One || OneAndTwo || OneAndThree)

    i++;

if (Two || OneAndTwo || TwoAndThree)

    i += 2;

if (Three || OneAndThree || TwoAndThree)

    i += 3;

Pretty great and simple stuff right? Well the switch structure can get kind of ugly (so can the if structure) in this scenario so I figured that this may be an acceptable use of the goto statement:

switch (exampleValue) {

    case OneAndTwo: i += 2 goto default;

    case OneAndThree: i += 3 goto default;

    case Two: i += 2 break;

    case TwoAndThree: i += 2 goto case Three;

    case Three: i += 3 break;

    default: i++ break;

}

I see no issue with the code above since it is sequential in nature and isn't misleading as to what could occur; however, this is just my opinion and I know there is a big debate over the use of goto within a professional environment. I currently have this refactored out into an if statement which definitely does what it should, but I hate the duplication factor of it. Why duplicate code when I can just tell it to execute both chunks. Even in my example above there is code duplication and I'd love to remove that too, but that would require more thought.

Important Notes

• 
  

  I have edited the post to remove the need for comments related to:

  
  
  
  
  
  • Execution path.
  
  
  • Calling methods.
  
  
  
  



• The switch structure example above is not actually implemented in my code.



• I am looking for alternatives to handling cases of fall-through.


• The execution order for each case is irrelevant in this particular code.

In this particular case the switch structure has cases which should fall-through to other cases which is not supported in C# like it is with C, C++, and Java. Would this be an acceptable use in a professional environment or should I leave it as my if structure?

Is there a more concise way of accomplishing this without effecting readability and maintainability?

I find the code hard to read with the goto statements. I would recommend structuring your enum differently. For example, if your enum was a bitfield where each bit represented one of the choices, it could look like this:

[Flags]

public enum ExampleEnum {

    One = 0b0001,

    Two = 0b0010,

    Three = 0b0100

};

The Flags attribute tells the compiler that you're setting up values which don't overlap. The code that calls this code could set the appropriate bit. You could then do something like this to make it clear what's happening:

if (myEnum.HasFlag(ExampleEnum.One))

{

    CallOne();

}

if (myEnum.HasFlag(ExampleEnum.Two))

{

    CallTwo();

}

if (myEnum.HasFlag(ExampleEnum.Three))

{

    CallThree();

}

This requires the code that sets up myEnum to set the bitfields properly and marked with the Flags attribute. But you can do that by changing the values of the enums in your example to:

[Flags]

public enum ExampleEnum {

    One = 0b0001,

    Two = 0b0010,

    Three = 0b0100,

    OneAndTwo = One | Two,

    OneAndThree = One | Three,

    TwoAndThree = Two | Three

};

When you write a number in the form 0bxxxx, you're specifying it in binary. So you can see that we set bit 1, 2, or 3 (well, technically 0, 1, or 2, but you get the idea). You can also name combinations by using a bitwise OR if the combinations might be frequently set together.

The best answer is use polymorphism.

Another answer, which, IMO, makes the if stuff clearer and arguably shorter:

if (One || OneAndTwo)

  CallOne();

if (OneAndTwo || Two)

  CallTwo();

if (OneAndThree || TwoAndThree || Three)

  CallThree();

goto is probably my 58th choice here...

IMO the root of the problem is that this piece of code shouldn't even exist.

You apparently have three independent conditions, and three independent actions to take if those conditions are true. So why is all that being funnelled into one piece of code that needs three Boolean flags to tell it what to do (whether or not you obfuscate them into an enum) and then does some combination of three independent things? The single responsibility principle seems to be having an off-day here.

Put the calls to the three functions where the belong (i.e. where you discover the need to do the actions) and consign the code in these examples to the recycle bin.

If there were ten flags and actions not three, would you extend this sort of code to handle 1024 different combinations? I hope not! If 1024 is too many, 8 is also too many, for the same reason.