Skip to main content
gamedev.net
🔒 Locked

Avoiding using a "goto"

Started by bakery2k1 Jun 3, 2006 at 6:43 PM 67 replies 5k views
Original Post
bakery2k1
bakery2k1
A question for those who say "never use goto": What would you do in this situation? I have 2 boolean conditions a and b, and three functions A, B, C. If a is true, I want to execute A. If b is true, execute B. If neither, execute C. C will do some work, then set either a or b to true, in which case I again want to execute A or B respectively. My current solution is: 

int foo(void)
{
retry:

    if(a)
        return A();

    if(b)
        return B();

    C();
    goto retry;
}
If I wanted to avoid "goto" here, what should I do? I guess the only construct which would allow me to jump backwards like this would be a loop, but semantically this function is not a loop - in the (highly likely) case that a or b is true, it will exit very quickly. Note also that C is a heavy function and "foo" is called very often - so placing the call to C at the top of the function, in order to be sure that a or b is true before doing any tests, kills my performance.
Daedalus AI
Daedalus AI
...It'll all get compiled down into assembler where there's tons of goto's, so doesn't really matter.

They should only be avoided for good coding practice (less confusing for others to read).

To answer your Q though; for this problem, I would make the function call itself.

return foo();

instead of

goto retry;
janta
janta 
int foo(void){    while(true)    {        if(a)            return A();         if(b)            return B();        C();    }}


How about that ? (I assume that somehow C() is able to modify either conditions a and/or b.

Nonetheless I would neither agree with people saying "never use goto" nor with people saying "never use xxxxx" or "always use xxxxx". because one size doesn't fit all...

Edit:

Quote:
Original post by Daedalus AI

To answer your Q though; for this problem, I would make the function call itself.

return foo();

instead of

goto retry;


Noooo ! :)
If you have the function calling itself you may get some overflow very quickly depending on the situation. (What if for example foo is a render loop, "a" is "the user wants to quit" and "b" is "an error occured", and C() is "render" ?
Red Ant
Red Ant 
int foo(void){    while ( true )    {        if(a)            return A();        if(b)            return B();        C();    } }



P.S. Grrrrrr .....
Zahlman
Zahlman
You're all making it much too complicated. Rearrange: Do the work that makes sure either a or b is set first, and then handle the one that is set.

int foo(void) {  while (!(a || b)) { C(); } // could use 'if', if your description is accurate  return a ? A() : B();}
Sneftel
Sneftel
Quote:
Original post by Zahlman
You're all making it much too complicated. Rearrange: Do the work that makes sure either a or b is set first, and then handle the one that is set.

int foo(void) {  while (!(a || b)) { C(); } // could use 'if', if your description is accurate  return a ? A() : B();}

Personally, I don't like that rewrite. It suggests that the loop-until is separate functionality from the decision of what to return, and also implies that a has something to do with B(). Of course, those things might be true, but given how the original code snippet looked it seems unlikely.
janta
janta
Quote:
How to write unmaintainable code - Misc techniques #2:

2 - Foiling Debuggers

A very simple way to confound people trying to understand your code by tracing it with a line debugger, is to make the lines long. In particular, put the then clause on the same line as the if. They can't place breakpoints. They can't tell which branch of an if was taken.


:)

Plua, "a == !b" is not necessatily always true, and if youd want to add a third condition, then you'd have to rewrite it all.
bakery2k1
bakery2k1
Quote:
Original post by janta and Red Ant
while(true)...


The thing I don't like about these is that on first glance, they appear to be a loop, which could last for several iterations. This is not the general behaviour of this function - most of the time, it exits quickly.

Quote:
Original post by Zahlman
Rearrange: Do the work that makes sure either a or b is set first, and then handle the one that is set.


As I said, that is not an option - function C is too heavy. It must only be called when necessary. Just to test, however, I tried the rearrange - my entire application performance dropped by over 25%.

I think the thing to do though, is to notice that C will always set one of a or b to true, so this is not really a loop at all. Thus my dislike of the "while(1)" solution, but on a closer look, the goto probably looks that way as well. I guess I'd be best off with:

int foo(void){    if(a)        return A();    if(b)        return B();    C();    if(a)        return A();    if(b)        return B();    return something_has_gone_horribly_wrong;}


possibly putting the repeated part into a separate function/macro.
Enigma
Enigma
Quote:
Original post by bakery2k1
As I said, that is not an option - function C is too heavy. It must only be called when necessary. Just to test, however, I tried the rearrange - my entire application performance dropped by over 25%.

Huh? Zahlman's function has the exact same characteristics as your original code. It only calls C if neither a nor b are true, same as your original code. Visual C++ 8.0 even produces exactly the same assembly code for the two snippets (with full optimizations).

Σnigma
yapposai
yapposai
I hope your just removed the parameters while posting the code. It seems that C() is modifying a and b because they are global vars? I suggest passing the variables as pointers to foo(), C() and removing their global scope. 
int foo(bool *a, bool *b, bool *c){   int retVal=0;   if(a==true || b==true){      retVal = ProcessAandB(a,b);    }else{          C(a,b);     if(a==false && b==false){         //Error, a and b are still both false     }         retVal = ProcessAandB(a,b);   }   return retVal;}int ProcessAandB(bool a, bool b){   if(a==true){     return A();   }    if (b==true){     return B();   }   return -1; //this should never execute}
---------------Magic is real, unless declared integer.- the collected sayings of Wiz Zumwalt
Sneftel
Sneftel
Quote:
Original post by _winterdyne_
goto's compile to jmp (jump). while's (and other evaluation criteria) are usually ble (branch if less than or equal). With many calls, the performance gain of a few clock cycles per iterations can add up.

I'm pretty sure that any compiler written in the last ten years will optimize while(true) to JMP and if(...)goto... to JE/JNE/whatever.
janta
janta
Quote:
Original post by _winterdyne_
This is one of those situations where the goto is the most efficient method -

goto's compile to jmp (jump). while's (and other evaluation criteria) are usually ble (branch if less than or equal). With many calls, the performance gain of a few clock cycles per iterations can add up.


Isn't such a way of optimizing a bit old fashioned ? I mean that a good compiler who deserves it's name will take care of that tricky stuff for you (even if there's no harm in being aware of what happens behind the scenes)

_winterdyne_
_winterdyne_
Yes. That's why I just deleted my post. I almost miss the really old compilers I learned C on.
Winterdyne Solutions Ltd is recruiting - this thread for details!
DvDmanDT
DvDmanDT
Quote:
Original post by yapposai
I hope your just removed the parameters while posting the code. It seems that C() is modifying a and b because they are global vars? I suggest passing the variables as pointers to foo(), C() and removing their global scope.
*** Source Snippet Removed ***


He never said they were variables, did he? He might very well call some function or do alot more than what the sample code he posted does (that's the impression I got anyway)..

Also, you forget to dereference your pointers, you compare a pointer with true/false, and you pass a pointer as argument to a function requireing a bool? And where did c come from? Finally, passing smaller types (like bools) as pointers are considered bad practise unless you intend to modify them (at least that's what I'm taugth).
JohnBolton
JohnBolton
For those that object to the rule "never use goto", I would like to point out that I have been programming in C for 20 years, and in those 20 years, I have never found the need to use goto. I don't see anything wrong with that rule (especially since you always have the option of breaking it).
John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!
mikeman
mikeman 
do{  if (a) return A();  if (b) return B();  C();} while (a||b)


It doesn't have the "endless loop" look you don't like, plus it makes sure it exits soon just in case C() doesn't change either a or b.
janta
janta
I think that your sample code is very far from behaving the same as the OP's.
Nathan Baum
Nathan Baum
Quote:
Original post by janta
I think that your sample code is very far from behaving the same as the OP's.

Not exactly very far. You only need to insert three extra characters.
do{  if (a) return A();  if (b) return B();  C();} while (!(a||b))

The condition is always true, of course: if either a or b are true, we never got there, so:
do{  if (a) return A();  if (b) return B();  C();} while (true)

I don't really like do ... while. Since it doesn't matter when the condition is tested, I'd use:
for (;;){  if (a) return A();  if (b) return B();  C();}

You may complain that it looks like a loop, but I don't see that as a problem since it is a loop, so pretending otherwise seems pointless.

Update: Given that you've said it isn't 'really' a loop, you're doing things entirely the wrong way...
int foo (void){  if (!(a || b))    C();  if(a)    return A();  return B();}

janta
janta
Relatively to your example:
do{  if (a) return A();  if (b) return B();  C();} while (!(a||b))


Basically, we have sth like:

void C() { a = true; }

So, the loop would end and then, what ?
Or do I need a break on programming (I'tired so I might have missed something obvious)

(but your second example using while(true) is fine)
abso
abso
Just use recursion:

int foo(void){    if(a)        return A();    else if(b)        return B();    else        C();    return foo()}

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.