Guess what I found in MFC's source code?
-
MFC uses tons of goto in the source code. Some C/C++ book authors hold an opinion about never using goto, whereas the others think that goto has its specific position to serve. Maxwell Chen
Maxwell Chen wrote: goto has its specific position to serve. I believe goto has one valid use: when there is a common clean up at the end of a routine (closing handles) and several possible errors cause early exit and the handles must still be closed.
myroutine()
{
HFILE hFile = NULL;if((hFile = CreateFile(...)) == NULL) goto sub\_exit; ReadFile(...); if(dwRead == 0) goto sub\_exit; WriteFile(...); if(dwWritten == 0) goto sub\_exit; ... sub\_exit: CloseHandle(hFile);}
Ok, now there are try/finally blocks, but I think this is a valid use for gotos. Forget about undeclared variables and compilation errors! :) -- LuisR ___________ Luis Alonso Ramos Chihuahua, Mexico www.luisalonsoramos.com
-
I was just debugging an MFC program and surprise, surprise there is a Goto statement insinde wincore.cpp at line 2438: goto LReturnTrue; Hmmm...makes you wonder...;P Rob
Robert Buldoc wrote: Hmmm...makes you wonder... Err... Seems not surprising to us. You may try again! :-D Maxwell Chen
-
I was just debugging an MFC program and surprise, surprise there is a Goto statement insinde wincore.cpp at line 2438: goto LReturnTrue; Hmmm...makes you wonder...;P Rob
-
Maxwell Chen wrote: goto has its specific position to serve. I believe goto has one valid use: when there is a common clean up at the end of a routine (closing handles) and several possible errors cause early exit and the handles must still be closed.
myroutine()
{
HFILE hFile = NULL;if((hFile = CreateFile(...)) == NULL) goto sub\_exit; ReadFile(...); if(dwRead == 0) goto sub\_exit; WriteFile(...); if(dwWritten == 0) goto sub\_exit; ... sub\_exit: CloseHandle(hFile);}
Ok, now there are try/finally blocks, but I think this is a valid use for gotos. Forget about undeclared variables and compilation errors! :) -- LuisR ___________ Luis Alonso Ramos Chihuahua, Mexico www.luisalonsoramos.com
Actually you could call
CloseHandle(hFile);and put a return at the end of routine.myroutine()
{ HFILE hFile = NULL;
if((hFile = CreateFile(...)) == NULL)
{
CloseHandle(hFile);
return;
}
ReadFile(...);
if(dwRead == 0)
{
CloseHandle(hFile);
return;
}
WriteFile(...);
if(dwWritten == 0)
{
CloseHandle(hFile);
return;
}
...
} -
Maxwell Chen wrote: goto has its specific position to serve. I believe goto has one valid use: when there is a common clean up at the end of a routine (closing handles) and several possible errors cause early exit and the handles must still be closed.
myroutine()
{
HFILE hFile = NULL;if((hFile = CreateFile(...)) == NULL) goto sub\_exit; ReadFile(...); if(dwRead == 0) goto sub\_exit; WriteFile(...); if(dwWritten == 0) goto sub\_exit; ... sub\_exit: CloseHandle(hFile);}
Ok, now there are try/finally blocks, but I think this is a valid use for gotos. Forget about undeclared variables and compilation errors! :) -- LuisR ___________ Luis Alonso Ramos Chihuahua, Mexico www.luisalonsoramos.com
class autoclose { HFILE * handle; autoclose(HFILE & hHandle) : handle(&hHandle) {} ~autoclose() {CloseHandle(*handle);} }; myroutine() { HFILE hFile = NULL; autoclose ac(hFile); if((hFile = CreateFile(...)) == NULL) return; ReadFile(...); if(dwRead == 0) return; WriteFile(...); if(dwWritten == 0) return; ... } Drinking In The Sun Forgot Password?
-
class autoclose { HFILE * handle; autoclose(HFILE & hHandle) : handle(&hHandle) {} ~autoclose() {CloseHandle(*handle);} }; myroutine() { HFILE hFile = NULL; autoclose ac(hFile); if((hFile = CreateFile(...)) == NULL) return; ReadFile(...); if(dwRead == 0) return; WriteFile(...); if(dwWritten == 0) return; ... } Drinking In The Sun Forgot Password?
Nice one! But not applicable to pure C. Maxwell Chen
-
Maxwell Chen wrote: goto has its specific position to serve. I believe goto has one valid use: when there is a common clean up at the end of a routine (closing handles) and several possible errors cause early exit and the handles must still be closed.
myroutine()
{
HFILE hFile = NULL;if((hFile = CreateFile(...)) == NULL) goto sub\_exit; ReadFile(...); if(dwRead == 0) goto sub\_exit; WriteFile(...); if(dwWritten == 0) goto sub\_exit; ... sub\_exit: CloseHandle(hFile);}
Ok, now there are try/finally blocks, but I think this is a valid use for gotos. Forget about undeclared variables and compilation errors! :) -- LuisR ___________ Luis Alonso Ramos Chihuahua, Mexico www.luisalonsoramos.com
This gives exactly the same effect, but without explicit gotos. You'll find my code littered with this.
myroutine() { HFILE hFile = NULL; do { if((hFile = CreateFile(...)) == NULL) break; ReadFile(...); if(dwRead == 0) break; WriteFile(...); if(dwWritten == 0) break; ... }while(false); CloseHandle(hFile); } -
This gives exactly the same effect, but without explicit gotos. You'll find my code littered with this.
myroutine() { HFILE hFile = NULL; do { if((hFile = CreateFile(...)) == NULL) break; ReadFile(...); if(dwRead == 0) break; WriteFile(...); if(dwWritten == 0) break; ... }while(false); CloseHandle(hFile); }That's misleading code. When someone sees a "do" they're expecting an honest-to-god loop. I would do it this way:
error myroutine()
{
HFILE hFile = NULL;
error = success;
if ((hFile = CreateFile(...)))
{
ReadFile(...);
if (dwRead)
{
WriteFile(...);
if (!dwWritten)
{
error = write;
}
}
else
{
error = read;
}
CloseHandle(hFile);
}
else
{
error = create;
}
return error;
}No goto's, no fake loops, the calling function can always programatically respond to the "error" reported by the function, and it's more maintainable by new employees. I've never seen a valid reason to ghave goto's in a c++ function, and I've been doing C++ for 14 years. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
-
That's misleading code. When someone sees a "do" they're expecting an honest-to-god loop. I would do it this way:
error myroutine()
{
HFILE hFile = NULL;
error = success;
if ((hFile = CreateFile(...)))
{
ReadFile(...);
if (dwRead)
{
WriteFile(...);
if (!dwWritten)
{
error = write;
}
}
else
{
error = read;
}
CloseHandle(hFile);
}
else
{
error = create;
}
return error;
}No goto's, no fake loops, the calling function can always programatically respond to the "error" reported by the function, and it's more maintainable by new employees. I've never seen a valid reason to ghave goto's in a c++ function, and I've been doing C++ for 14 years. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
John Simmons / outlaw programmer wrote: No goto's, no fake loops, the calling function can always programatically respond to the "error" reported by the function, and it's more maintainable by new employees. But so many unnecessary else's certainly lower readability of code. If you use destructor(s) for releasing you would just return proper error immediately. John Simmons / outlaw programmer wrote: I've never seen a valid reason to ghave goto's in a c++ function, and I've been doing C++ for 14 years. How do you exit from multiple loops?
for(something){ for(something else){ if(something wrong) goto exitloop; // how else? do useful stuff } } exitloop: -
Maxwell Chen wrote: goto has its specific position to serve. I believe goto has one valid use: when there is a common clean up at the end of a routine (closing handles) and several possible errors cause early exit and the handles must still be closed.
myroutine()
{
HFILE hFile = NULL;if((hFile = CreateFile(...)) == NULL) goto sub\_exit; ReadFile(...); if(dwRead == 0) goto sub\_exit; WriteFile(...); if(dwWritten == 0) goto sub\_exit; ... sub\_exit: CloseHandle(hFile);}
Ok, now there are try/finally blocks, but I think this is a valid use for gotos. Forget about undeclared variables and compilation errors! :) -- LuisR ___________ Luis Alonso Ramos Chihuahua, Mexico www.luisalonsoramos.com
Then there are two good reasons:
void foo()
{
bool b = true;for (int i = 0; i < 100; i++)
{
for (int j = 0; j < 100; j++)
{
while (b)
{
// Some situation in inner loop.
if (...)
{
// We don't want to come back here, and there is more code to process.
// Obviously we can't use break, and this is actually more 'clean' than
// a bunch of test variables.
goto more_code;
}
}
}
}// More actions needs to be performed...
more_code:
...
}"After all it's just text at the end of the day. - Colin Davies "For example, when a VB programmer comes to my house, they may say 'does your pool need cleaning, sir ?' " - Christian Graus
-
John Simmons / outlaw programmer wrote: No goto's, no fake loops, the calling function can always programatically respond to the "error" reported by the function, and it's more maintainable by new employees. But so many unnecessary else's certainly lower readability of code. If you use destructor(s) for releasing you would just return proper error immediately. John Simmons / outlaw programmer wrote: I've never seen a valid reason to ghave goto's in a c++ function, and I've been doing C++ for 14 years. How do you exit from multiple loops?
for(something){ for(something else){ if(something wrong) goto exitloop; // how else? do useful stuff } } exitloop:You see "unnecessary else's", I see easily maintainable code. I guess I've been doing this too long to worry about writing something that is overly exotic, ambiguous, and universally recognized as bad form. If I were participating in a code review, and if I saw a loop that wasn't a loop, or a goto in a function this small, I'd reject the function and demand a rewrite. I would also refuse to accept macros like this (And yes, I recently worked on code that contained these macros): #define EQ == #define LOOP for( ; ; ) #define WHILE(c) if (!(c)) break #define _ 0 The LOOP...WHILE stuff was especially confusing to work with when you saw code like this:
LOOP
{
do something;
do somethin;g
WHILE (something);
do something;
WHILE (something);
do something;
do something;
}The guy that wrote these macros was famous for doing this kind of stuff because he thought it was cool, and gave absolutely no thought to the fact that he wasn't going to be the only one working on the code, and probably wasn't going to be around when someone new came onboard to maintain it. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
-
Then there are two good reasons:
void foo()
{
bool b = true;for (int i = 0; i < 100; i++)
{
for (int j = 0; j < 100; j++)
{
while (b)
{
// Some situation in inner loop.
if (...)
{
// We don't want to come back here, and there is more code to process.
// Obviously we can't use break, and this is actually more 'clean' than
// a bunch of test variables.
goto more_code;
}
}
}
}// More actions needs to be performed...
more_code:
...
}"After all it's just text at the end of the day. - Colin Davies "For example, when a VB programmer comes to my house, they may say 'does your pool need cleaning, sir ?' " - Christian Graus
You could also write fo in such a way that it returns to a calling function for the more code part instead of having "more code" itself. Voila - no goto needed. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
-
You see "unnecessary else's", I see easily maintainable code. I guess I've been doing this too long to worry about writing something that is overly exotic, ambiguous, and universally recognized as bad form. If I were participating in a code review, and if I saw a loop that wasn't a loop, or a goto in a function this small, I'd reject the function and demand a rewrite. I would also refuse to accept macros like this (And yes, I recently worked on code that contained these macros): #define EQ == #define LOOP for( ; ; ) #define WHILE(c) if (!(c)) break #define _ 0 The LOOP...WHILE stuff was especially confusing to work with when you saw code like this:
LOOP
{
do something;
do somethin;g
WHILE (something);
do something;
WHILE (something);
do something;
do something;
}The guy that wrote these macros was famous for doing this kind of stuff because he thought it was cool, and gave absolutely no thought to the fact that he wasn't going to be the only one working on the code, and probably wasn't going to be around when someone new came onboard to maintain it. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
You are not really trying to convince me that
error myroutine(){ HFILE hFile = NULL; error = success; if ((hFile = CreateFile(...))) { ReadFile(...); if (dwRead) { WriteFile(...); if (!dwWritten) { error = write; } } else { error = read; } CloseHandle(hFile); } else { error = create; } return error; }
is more readable and maintainable than:error myroutine(){ File f; if(!f.Create(...)) return create; if(!f.Read(...)) return read; if(!f.Write(...)) return write; return success; } -
John Simmons / outlaw programmer wrote: No goto's, no fake loops, the calling function can always programatically respond to the "error" reported by the function, and it's more maintainable by new employees. But so many unnecessary else's certainly lower readability of code. If you use destructor(s) for releasing you would just return proper error immediately. John Simmons / outlaw programmer wrote: I've never seen a valid reason to ghave goto's in a c++ function, and I've been doing C++ for 14 years. How do you exit from multiple loops?
for(something){ for(something else){ if(something wrong) goto exitloop; // how else? do useful stuff } } exitloop: -
toxcct wrote: have you ever heard or break keyword ? Are you sure that you know how break works? It wouldn't really do the trick in this case...
-
I was just debugging an MFC program and surprise, surprise there is a Goto statement insinde wincore.cpp at line 2438: goto LReturnTrue; Hmmm...makes you wonder...;P Rob
gotos are inherited from assembly code and that is the main reason it exist in C/C++. in all ways, i find MFC and other microsoft library codes awful to read. for example, they implement some of their functions into the header files instead of writing it into CPP files. moreover, they abuse of the imbricated operations...
TOXCCT >>> GEII power
-
I was just debugging an MFC program and surprise, surprise there is a Goto statement insinde wincore.cpp at line 2438: goto LReturnTrue; Hmmm...makes you wonder...;P Rob
Probably hangovers from C rather than C++ ;P Ant.
-
You are not really trying to convince me that
error myroutine(){ HFILE hFile = NULL; error = success; if ((hFile = CreateFile(...))) { ReadFile(...); if (dwRead) { WriteFile(...); if (!dwWritten) { error = write; } } else { error = read; } CloseHandle(hFile); } else { error = create; } return error; }
is more readable and maintainable than:error myroutine(){ File f; if(!f.Create(...)) return create; if(!f.Read(...)) return read; if(!f.Write(...)) return write; return success; }Yes, and besides that, my version has only one exit point AND it closes the handle (if the handle was created). So you see, the short way, and more importantly the *exotic* way, are almost always less desireable than the *right* way. And before anyone becomes pedant, I am aware there are several "right" ways, but my point is that some ways are more right than others, and I will always stress maintainability over short/exotic. I hope I have been sufficiently pellucid, even for those who's most prominent physical feature is their single eyebrow which is more-or-less centered on their sloping foreheads. Thanks for playing our game. You may now grunt amongst yourselves. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
-
I was just debugging an MFC program and surprise, surprise there is a Goto statement insinde wincore.cpp at line 2438: goto LReturnTrue; Hmmm...makes you wonder...;P Rob
That is why they provide goto statement in C# as well. Sonork 100.41263:Anthony_Yio Life is about experiencing ...
-
Yes, and besides that, my version has only one exit point AND it closes the handle (if the handle was created). So you see, the short way, and more importantly the *exotic* way, are almost always less desireable than the *right* way. And before anyone becomes pedant, I am aware there are several "right" ways, but my point is that some ways are more right than others, and I will always stress maintainability over short/exotic. I hope I have been sufficiently pellucid, even for those who's most prominent physical feature is their single eyebrow which is more-or-less centered on their sloping foreheads. Thanks for playing our game. You may now grunt amongst yourselves. ------- sig starts "I've heard some drivers saying, 'We're going too fast here...'. If you're not here to race, go the hell home - don't come here and grumble about going too fast. Why don't you tie a kerosene rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt "...the staggering layers of obscenity in your statement make it a work of art on so many levels." - Jason Jystad, 10/26/2001
Then I'm really sorry for everyone whose code you get to review... John Simmons / outlaw programmer wrote: it closes the handle (if the handle was created). :confused: This how we started - use destructor(s) for releasing/closing.