Cfrgtkky

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.

Never use gotos is one of the "lies to children" concepts of computer science. It's the right advice 99% of the time, and the times it isn't are so rare and specialized that it's far better for everyone if it's just explained to new coders as "don't use them".

So when should they be used? There are a few scenarios, but the basic one you seem to be hitting is: when you're coding a state machine. If there's no better more organized and structured expression of your algorithm than a state machine, then its natural expression in code involves unstructured branches, and there isn't a lot that can be done about that which doesn't arguably make the structure of the state machine itself more obscure, rather than less.

Compiler writers know this, which is why the source code for most compilers that implement LALR parsers^* contain gotos. However, very few people will ever actually code their own lexical analyzers and parsers.

^{* - IIRC, it's possible to implement LALL grammars entirely recursive-descent without resorting to jump tables or other unstructured control statements, so if you're truly anti-goto, this is one way out.}

Now the next question is, "Is this example one of those cases?"

What I'm seeing looking it over is that you have three different possible next states depending on the processing of the current state. Since one of them ("default") is just a line of code, technically you could get rid of it by tacking that line of code on the end of the states it applies to. That would get you down to 2 possible next states.

One of the remaining ones ("Three") is only branched to from one place that I can see. So you could get rid of it the same way. You'd end up with code that looks like this:

switch (exampleValue) {

    case OneAndTwo: i += 3 break;

    case OneAndThree: i += 4 break;

    case Two: i += 2 break;

    case TwoAndThree: i += 5 break;

    case Three: i += 3 break;

    default: i++ break;

}

However, again this was a toy example you provided. In cases where "default" has a non-trivial amount of code in it, "three" is transitioned to from multiple states, or (most importantly) further maintainance is likely to add or complicate the states, you'd honestly be better off using gotos (and perhaps even getting rid of the enum-case structure that hides the state-machine nature of things, unless there's some good reason it needs to stay).

The best answer is use polymorphism.

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

if (One || OneAndTwo || OneAndThree)

  CallOne();

if (Two || OneAndTwo || TwoAndThree)

  CallTwo();

if (Three || OneAndThree || TwoAndThree)

  CallThree();

goto is probably my 58th choice here...

Why not this:

public enum ExampleEnum {

    One = 0, // Why not?

    OneAndTwo,

    OneAndThree,

    Two,

    TwoAndThree,

    Three

}

int COUNTS = { 1, 3, 4, 2, 5, 3 }; // Whatever



int ComputeExampleValue(int i, ExampleEnum exampleValue) {

    return i + COUNTS[(int)exampleValue];

}

OK, I agree, this is hackish (I'm not a C# developer btw, so excuse me for the code), but from the point of view of efficiency this is must? Using enums as array index is valid C#.

If you can't or don't want to use flags, use a tail recursive function. In 64bit release mode, the compiler will produce code which is very similar to your goto statement. You just don't have to deal with it.

int ComputeExampleValue(int i, ExampleEnum exampleValue) {

    switch (exampleValue) {

        case One: return i + 1;

        case OneAndTwo: return ComputeExampleValue(i + 2, ExampleEnum.One);

        case OneAndThree: return ComputeExampleValue(i + 3, ExampleEnum.One);

        case Two: return i + 2;

        case TwoAndThree: return ComputeExampleValue(i + 2, ExampleEnum.Three);

        case Three: return i + 3;

   }

}

If you are intent on using a switch here your code will actually be faster if you handle each case separately

switch(exampleValue)

{

    case One:

        i++;

        break;

    case Two:

        i += 2;

        break;

    case OneAndTwo:

    case Three:

        i+=3;

        break;

    case OneAndThree:

        i+=4;

        break;

    case TwoAndThree:

        i+=5;

        break;

}

only a single arithmetic operation is performed in each case

also as others have stated if you are considering using gotos you should probably rethink your algorithm (though I will concede that C#s lack of case fall though could be a reason to use a goto). See Edgar Dijkstra's famous paper "Go To Statement Considered Harmful"

For your particular example, since all you really wanted from the enumeration was a do/do-not indicator for each of the steps,
the solution that rewrites your three if statements is preferable to a switch,
and it is good that you made it the accepted answer.

But if you had some more complex logic that did not work out so cleanly,
then I still find the gotos in the switch statement confusing. I would rather see something like this:

switch (enumVal) {

    case ThreeOneTwo: DrawThree; DrawTwo; DrawOne; break;

    case ThreeTwo:    DrawThree; DrawTwo; break;

    case ThreeOne:    DrawThree; DrawOne; break;

    case TwoOne:      DrawTwo; DrawOne; break;

    case Two:         DrawTwo; break;

    default:          DrawOne; break;

}

This is not perfect but I think it's better this way than with the gotos.
If the sequences of events are so long and duplicate each other so much that it really doesn't make sense to spell out the complete sequence for each case, I'd prefer a subroutine rather than goto in order to reduce duplication of code.

I’m not sure that anyone really has a reason to hate the goto keyword these days. It’s definitely archaic and not needed in 99% of use cases, but it is a feature of the language for a reason.

The reason to hate the goto keyword is code like

if (someCondition) {

    goto label;

}



string message = "Hello World!";



label:

Console.WriteLine(message);

Oops! That's clearly not going to work. The message variable is not defined in that code path. So C# won't pass that. But it might be implicit. Consider

object.setMessage("Hello World!");



label:

object.display();

And assume that display then has the WriteLine statement in it.

This kind of bug can be hard to find because goto obscures the code path.

This is a simplified example. Assume that a real example would not be nearly as obvious. There might be fifty lines of code between label: and the use of message.

The language can help fix this by limiting how goto can be used, only descending out of blocks. But the C# goto is not limited like that. It can jump over code. Further, if you are going to limit goto, it's just as well to change the name. Other languages use break for descending out of blocks, either with a number (of blocks to exit) or a label (on one of the blocks).

The concept of goto is a low level machine language instruction. But the whole reason why we have higher level languages is so that we are limited to the higher level abstractions, e.g. variable scope.

All that said, if you use the C# goto inside a switch statement to jump from case to case, it's reasonably harmless. Each case is already an entry point. I still think that calling it goto is silly in that situation, as it conflates this harmless usage of goto with more dangerous forms. I would much rather that they use something like continue for that. But for some reason, they forgot to ask me before they wrote the language.

When you have this many choices (and even more, as you say), then maybe it's not code but data.

Create a dictionary mapping enum values to actions, expressed either as functions or as a simpler enum type representing the actions. Then your code can be boiled down to a simple dictionary lookup, followed by either calling the function value or a switch on the simplified choices.