Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » How bad is a goto statement?

This thread is locked; no one can reply to it. rss feed Print
 1   2   3   4 
How bad is a goto statement?
Joel Pettersson
Member #4,187
January 2004

It is seldom needed, and I don't use them particularly often, but goto statements can occasionally improve both efficiency and clarity. Once in a rare while they are the best solution; when this is the case, it is nothing short of stupid to religiously avoid them. Whatever the best tool for the job is, use it.

imaxcs
Member #4,036
November 2003

Consider this code:

handle_objectdeletion:
for(multiset<CObject*, SSortObjects>::iterator it = CObjectList::o().begin();it != CObjectList::o().end();it++) // loop through STL-multiset (could be any other container as well)
      if ((*it)->get_x() + (*it)->get_w() < CCamera::o().get_x()) // check, if some statement about the object is true and delete it
      {
        CObjectList::o().remove(it); // delete one object from the container
        goto handle_objectdeletion; // we must break from the loop as the iterators might be invalid now...
      }

Got any advice to make it better?

HoHo
Member #4,534
April 2004
avatar

use break instead of goto and put it inside two loops, outer one loops until it has reached the end of the container.

Something like this (grossly simplified to make things clearer):

1bool needsDelete=false;
2vector<Data> data;
3vector<Data>::iterator iter;
4while(1){
5 for (iter=data.begin(); iter!=data.end(); iter++){
6 if (iter.needs_to_be_deleted()){
7 needsDelete=true;
8 // found something needing removal, break from for loop.
9 break;
10 }
11 }
12 if (needsDelete){
13 iter.remove();
14 needsDelete=false;
15 } else {
16 // looped through every variable, nothing needs deletion, breaks from while(1)
17 break;
18 }
19}

Code not tested but in theory it should work. There are about a million other ways of doing the same thing.

[edit]
Of course if the container is big then some kind of system should be used that would prevent the inner for loop to start over every time something gets deleted.

__________
In theory, there is no difference between theory and practice. But, in practice, there is - Jan L.A. van de Snepscheut
MMORPG's...Many Men Online Role Playing Girls - Radagar
"Is Java REALLY slower? Does STL really bloat your exes? Find out with your friendly host, HoHo, and his benchmarking machine!" - Jakub Wasilewski

Audric
Member #907
January 2001

If you want to remove all the items that match the test:

multiset<CObject*, SSortObjects>::iterator it;
it = CObjectList::o().begin();
while(it != CObjectList::o().end()) // loop through STL-multiset (could be any other container as well)
{
      if ((*it)->get_x() + (*it)->get_w() < CCamera::o().get_x()) // check, if some statement about the object is true and delete it
      {
        CObjectList::o().remove(it); // delete one object from the container
        it = CObjectList::o().begin(); // restart the whole loop
      }
      it++;
}

In this case, it's possible because "it = CObjectList::o().begin()" is a clean "restart", and if you delete the last element it matches the end-of-loop condition.

CGamesPlay
Member #2,559
July 2002
avatar

Ugh, learn the STL ::)

for(multiset<CObject*, SSortObjects>::iterator it = CObjectList::o().begin();it != CObjectList::o().end();) // loop through STL-multiset (could be any other container as well)
      if ((*it)->get_x() + (*it)->get_w() < CCamera::o().get_x()) // check, if some statement about the object is true and delete it
      {
        it = CObjectList::o().erase(it); // delete one object from the container
      }
      else
        it++;

--
Tomasu: Every time you read this: hugging!

Ryan Patterson - <http://cgamesplay.com/>

Audric
Member #907
January 2001

The docs I found say erase() returns void :-/

My method of restarting is inefficient. Better to memorize a reference to the element to remove, then increase the iterator, then remove the to_be_removed element.

edit: anyway, multiset is probably a bad example, as it's one of the STL containers which does not invalidate iterators on remove.

Jakub Wasilewski
Member #3,653
June 2003
avatar

Quote:

The docs I found say erase() returns void :-/

Your docs are probably outdated.

---------------------------
[ ChristmasHack! | My games ] :::: One CSS to style them all, One Javascript to script them, / One HTML to bring them all and in the browser bind them / In the Land of Fantasy where Standards mean something.

Audric
Member #907
January 2001

Jakub Wasilewski
Member #3,653
June 2003
avatar

Didn't read through carefully, didn't notice that we're talking about multisets here. Sorry Audric, you're right about that.

To clarify, multisets are AssociativeContainers, and those have a void erase() (they don't need the return value anyway). The iterator erase() is part of Sequence (which is a list, vector, deque etc.). My comment about outdated docs stemmed from the fact that in the past, even Sequences had a void erase() in some compilers.

---------------------------
[ ChristmasHack! | My games ] :::: One CSS to style them all, One Javascript to script them, / One HTML to bring them all and in the browser bind them / In the Land of Fantasy where Standards mean something.

Audric
Member #907
January 2001

Quote:

With this in mind, is it wrong to use the occasional goto in code, other than for aesthetic reasons?

Once you start down the dark path, forever will it dominate your destiny, consume you it will.
-- Yoda

Personnally, I use it's much wronger to write things like:

   // (inside main, for example)
   if (some big problem in initialization)
   {
      // exception handling
      allegro_exit();
      exit(0);
   }
   else
   {
      // a few hundred lines here....
   }
   // ...continued here... for some reason. argh!
}

orz
Member #565
August 2000

Quote:

To clarify, multisets are AssociativeContainers, and those have a void erase() (they don't need the return value anyway). The iterator erase() is part of Sequence (which is a list, vector, deque etc.). My comment about outdated docs stemmed from the fact that in the past, even Sequences had a void erase() in some compilers.

IIRC, erase() returns void on associative containers UNLESS you're using microsofts implementation and/or documentation : )

TeamTerradactyl
Member #7,733
September 2006
avatar

Audric's statement gave me a question:

void some_function_other_than_main()
{
  ...
  if (error)
    exit(1); // <-- This line
}

Does the "exit(1)" do about the same thing as the following code (barring the fact that "goto" was not supposed to leave the current function, or that "exit()" passes a value to be returned):

void some_function_other_than_main()
{
  ...
  if (error)
    goto End_of_Main_Function; // <-- This line
}

int main()
{
  ...
:End_of_Main_Function
  return 0;
}

So if you don't have a chance to clean up a bunch of your dynamically-allocated arrays before your program does a nose-dive, they will NEVER reach the destructors for your classes and/or cleanup() code?

CGamesPlay
Member #2,559
July 2002
avatar

Quote:

So if you don't have a chance to clean up a bunch of your dynamically-allocated arrays before your program does a nose-dive, they will NEVER reach the destructors for your classes and/or cleanup() code?

All destructors on objects will be called with exit. They will not with abort, though.

--
Tomasu: Every time you read this: hugging!

Ryan Patterson - <http://cgamesplay.com/>

TeamTerradactyl
Member #7,733
September 2006
avatar

CGamesPlay said:

All destructors on objects will be called with exit. They will not with abort, though.

But only if the dynamic array was created inside a class (which has a proper destructor)?

int main()
{
  int *theArray;
  theArray = new int[400];

  exit(1);
};

So here, you would have a memory leak, or so I would imagine.

I would think a much better design would be to have exit() call a "cleanup" function of the programmer's choice:

1int clean_all_my_stuff(int wasErrorOnExit)
2{
3 destroy_bitmap(double_buffer);
4 delete [] globalArray;
5 
6 return wasErrorOnExit;
7}
8 
9int main()
10{
11 ...
12 if (error)
13 exit(clean_all_my_stuff(1));
14 
15 ...
16 return (clean_all_my_stuff(0)); // <-- Passes "0" to signify all went well
17}

HoHo
Member #4,534
April 2004
avatar

Quote:

So if you don't have a chance to clean up a bunch of your dynamically-allocated arrays before your program does a nose-dive, they will NEVER reach the destructors for your classes and/or cleanup() code?

I never use exit. Errors that force program shutdown usually appear while setting up the program. I just create a function for initialization that returns some value. If it is an error it returns an error value (duh!). When I see that an error occured I simply call the function that is responsible for cleaning up and return from main afterwards.

__________
In theory, there is no difference between theory and practice. But, in practice, there is - Jan L.A. van de Snepscheut
MMORPG's...Many Men Online Role Playing Girls - Radagar
"Is Java REALLY slower? Does STL really bloat your exes? Find out with your friendly host, HoHo, and his benchmarking machine!" - Jakub Wasilewski

kosmitek
Member #8,151
December 2006

very interesting subject

example:

1while(....)
2{
3 
4 
5INSTRUCTION;
6 
7here:
8if()
9{}
10else if ()
11{}
12 
13else()
14{
15goto: here;
16}
17 
18 
19}

How should I delete goto ? In above example - somebody must do something, he won't go further if he won't do something, and if I delete "goto" then instructions be become executed and I don't want it - have you solution ?

Jonatan Hedborg
Member #4,886
July 2004
avatar

1while(....)
2{
3INSTRUCTION;
4 
5 do {
6 bool repeat = false;
7 if()
8 {}
9 else if ()
10 {}
11 else()
12 {
13 repeat = true;
14 }
15 } while( repeat );
16}

for example.

Matthew Leverton
Supreme Loser
January 1999
avatar

1while(....)
2{
3 INSTRUCTION;
4 do
5 {
6 if()
7 {
8 // blah
9 }
10 else if()
11 {
12 // blah
13 }
14 else()
15 {
16 continue;
17 }
18 } while (FALSE);
19}

TeamTerradactyl
Member #7,733
September 2006
avatar

kosmitek: both Matthew and Jonatan are correct: you would use some sort of loop (while, for, etc.) until you specify that you should break out of it (or, you can specify that you will only run it once UNLESS some condition is met).

1bool breakInsideLoop = false;
2while(...)
3{
4 EXPRESSION;
5 
6 while (!breakInsideLoop)
7 {
8 ...
9 
10 if (...)
11 breakInsideLoop = true;
12 }
13 
14 ... // Now, outside of the "inside" loop
15}

Goalie Ca
Member #2,579
July 2002
avatar

Actually i've just been reading/writting some assembly and for some reason this version seems a lot nicer. I guess it depends on the audience and the style of programming and what mood your in.

here:
if()
{}
else if ()
{}

else()
{
goto: here;
}

but matts' version is quite nice as well. The adding an extra variable is kind of a nasty hack.

-------------
Bah weep granah weep nini bong!

Richard Phipps
Member #1,632
November 2001
avatar

TeamTerradactyl
Member #7,733
September 2006
avatar

I guess we can all just start learning low-level programming and use the goto: statement a lot more ... :D "Let's see... push aex onto, uh... fex... then pop foo off of bar... Whoops; that should have been fee from bak. Where's my debugger again..?"

Matthew Leverton
Supreme Loser
January 1999
avatar

Quote:

Actually i've just been reading/writting some assembly and for some reason this version seems a lot nicer.

But it really is a loop—it's not a one time jump. I prefer the do/while construct because of that. Ideally there would be a slimmer syntax like:

do {
  // ...
  if (foo) continue;
  // ...
}

Either way, the use of goto (or a similar loop construct) usually indicates that you should consider rethinking your approach altogether. I think the biggest problem with relying on goto for anything is that you might end up relying on it elsewhere out of laziness.

Arthur Kalliokoski
Second in Command
February 2005
avatar

Winduhs search just found 41 gotos in Allegro source tree on this computer...

They all watch too much MSNBC... they get ideas.

Matthew Leverton
Supreme Loser
January 1999
avatar

Most of Allegro's goto usage is for error handling. Given C's lack of exceptions, there's not always a good alternative.

 1   2   3   4 


Go to: