- Home /
Unclear about an Ancient "evil" keyword (goto)
Hey!
This can be taken as a more-basic or more-advanced question depending, but I have a method that contains a switch statement:
case CurrentInputEvent.InputHeld:
if (_inputHeld_inh.ignoreState)
{
if (_hover_inh.ignoreState)
{
if (_normal_inh.ignoreState)
{
if (!throw_Error002)
{
throw_Error002 = true;
Debugging.Error002_IgnoringTooManyEventStates("Normal");
return _normal_inh;
}
else return _normal_inh;
}
else return _normal_inh;
}
else return _hover_inh;
}
else return _inputHeld_inh;
Not so pretty, right? I mean it does everything that I want it to do. Nothing more, and nothing less. I may have something wrong with me, but I don't see a way I can optimize this any further and maintain the same functionality.
WHAT I WANT TO ACCOMPLISH:
If one type is ignored, i want it to get the next one and the next one and the next one UNTIL there comes a point when it gets the type that isn't ignored.
I have about 15 states to switch through in the "CurrentInputEvent" enum, and each will look something like this. This right here is 20 lines (if I counted to 20 right). 20 x 15 is a lot, especially if there are other cases that will be longer than this.
I never used goto before because I heard it was evil, but does anyone have any concrete evidence that something that looks like:
case 1:
{
if (!lol)
goto case 2;
break;
}
case 2:
{
if (!lol)
goto case 3;
break;
}
case 3:
{
if (!lol)
goto case 4;
break;
}
case 4:
{
if (!lol)
goto case 5;
break;
}
case 5:
if (!lol)
goto default;
break;
is CRIMINAL compared to what I posted originally? Personally I find it to me more concise, line-saving, and more readable.
As I'm working on increasing performance at the moment, if any red flags fly up that "goto is super duper performance heavy" then I'll have to make the 400 lines of code. With goto I might have 50-60 lines.
Apologies that this is only partially Unity-related, but I'm tired of reading all of the same hate on the goto word with the same single reason: IT'S NOT GOOD PROGRAMMING PRACTICE!
Answer by undead-steve · Nov 08, 2013 at 10:35 PM
You could avoid the goto's by just updating a state value in the first pass, and then acting on it in the second. In the example you posted you could just stick the output case into an int, and then process that int in a second switch-case. It's a little nicer than gotos because you have an intermediate step between the which-case-do-i-use logic and the in-this-case-i-do-x logic: jumbling those together is what makes gotos unpopular.
Another method is to create list of handler classes, and loop through them until one of them says "I can use this input" and handles it . This is how, for example, lots of script parsers work: first lets see if this is a noun, then lets's see if its a verb, and so on. If any handler class says "i got it" then the others don't get called. This known as the Chain of responsibility pattern. It's great for choosing one item out of a long list of possibilities ("is this word english? spanish? latin? chinese?")
However, if you've really got 15 states to manage, neither switch/case nor goto is the right language construct. You want a state machine: at any given moment the choices in front of you are mostly determined by what state you're in; as new events happen, you either change states or handled the event in the current state. Each individual state can be super simple because it doesn't know or care about things happening in other states. It's the best way to handle lots of contextual choice making. Generically the code would be:
1) does this input change my state? If so, go to the new state.
2) do what this state does
3) I'm done
States could be functions (this example shows a simple function based state machine) or classes, depending on how much you need to do.
Thanks for posting about the State $$anonymous$$achine. I've heard it a lot before but never actually attempted anything with it. Also thanks for the clarification.
Answer by numberkruncher · Nov 08, 2013 at 09:57 PM
The following should be equivalent to your if statement which imo looks much tidier:
if (!_inputHeld_inh.ignoreState) {
return _inputHeld_inh;
}
else if (!_hover_inh.ignoreState) {
return _hover_inh;
}
else if (!_normal_inh.ignoreState) {
return _normal_inh;
}
else if (!throw_Error002) {
throw_Error002 = true;
Debugging.Error002_IgnoringTooManyEventStates("Normal");
return _normal_inh;
}
else {
return _normal_inh;
}
Thanks for posting that, but the length is still there which is what I was really what I wanted to reduce.
Answer by cmpgk1024 · Nov 08, 2013 at 09:59 PM
The problem with goto and the reason why it is bad programming practice is because it creates so-called spaghetti logic, making your code harder to debug and harder to read. Without goto, you will be able to look at the brackets and follow the logic through. However, there is an even easier way to solve your problem.
case 1:
if(lol){
break;
}
case 2:
//etc
and so on and so forth. It will simply fall through to the next statement, so you will avoid using goto and have clean and readable code.
Thanks for posting that, and I understand your explanation about goto. Though falling through cases like that isn't possible :(
Your answer
Follow this Question
Related Questions
Multiple Cars not working 1 Answer
Distribute terrain in zones 3 Answers
Problems with enums and switches (C#) 2 Answers
How to check if an enum’s case has changed 3 Answers
Using a switch statement to set tile type based on texture. 1 Answer