|
|
| 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
|
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):
Code not tested but in theory it should work. There are about a million other ways of doing the same thing. [edit] __________ |
|
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
|
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++;
-- 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
|
Quote:
The docs I found say erase() returns void Your docs are probably outdated. --------------------------- |
|
Audric
Member #907
January 2001
|
|
Jakub Wasilewski
Member #3,653
June 2003
|
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. --------------------------- |
|
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. 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
|
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
|
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. -- Ryan Patterson - <http://cgamesplay.com/> |
|
TeamTerradactyl
Member #7,733
September 2006
|
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:
|
|
HoHo
Member #4,534
April 2004
|
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. __________ |
|
kosmitek
Member #8,151
December 2006
|
very interesting subject example:
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
|
for example.
|
|
Matthew Leverton
Supreme Loser
January 1999
|
|
|
TeamTerradactyl
Member #7,733
September 2006
|
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).
|
|
Goalie Ca
Member #2,579
July 2002
|
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. ------------- |
|
Richard Phipps
Member #1,632
November 2001
|
This is making my head hurt! |
|
TeamTerradactyl
Member #7,733
September 2006
|
I guess we can all just start learning low-level programming and use the goto: statement a lot more ...
|
|
Matthew Leverton
Supreme Loser
January 1999
|
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
|
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
|
Most of Allegro's goto usage is for error handling. Given C's lack of exceptions, there's not always a good alternative. |
|
|
|