Guess what I found in MFC's source code?
-
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.
-
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.
And I feel sorry for anyone that has to suffer through your version of "coding" when you've been fired and are no longer around to explain how your stuff works (if it indeed does work), or why it's there. I bet you're one of those people who think that comments are unnecessary because "the code is the documentation". This IS NOT how we started. The original statement concerned the use of goto's in MFC source code. ------- 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
-
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
Do you mean "overloaded", and not "imbricated" (overlapping)? 10 points for using a word that a lot of people would have to look up in the dictionary. :) ------- 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
-
Do you mean "overloaded", and not "imbricated" (overlapping)? 10 points for using a word that a lot of people would have to look up in the dictionary. :) ------- 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
sorry for my poor english... i mean that some function calls are made into other calls, and that can really become very hard to understand the code written. another thing, you must be sure of the operators priorities to understand the behavior of such code.
TOXCCT >>> GEII power
-
sorry for my poor english... i mean that some function calls are made into other calls, and that can really become very hard to understand the code written. another thing, you must be sure of the operators priorities to understand the behavior of such code.
TOXCCT >>> GEII power
Hmmmm, I think overlapping is still the wrong term. Maybe "nested function calls", where a function calls a function that calls a function, etc., etc.? I think that's the nature of C++, and in most real-world apps that perform a useful set of actions, you simply won't be able to avoid it in your own code. ------- 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
-
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;
}
...
}That's because there's only 1 handle to close, but if you have many of them, and many points of failure, all you are doing is a huge Copy-Paste, which can even be worst than goto. Teachers and books encouraged NOT to use GOTO, to force programmers to abandon bad programming techniques, inherited from basic and asm. But It is also said you should first learn and respect all programming rules, and when you are an advanced programmer, be able to decide when to brake them. Fortunately this can be solved now by try-catch blocks.
-
Hmmmm, I think overlapping is still the wrong term. Maybe "nested function calls", where a function calls a function that calls a function, etc., etc.? I think that's the nature of C++, and in most real-world apps that perform a useful set of actions, you simply won't be able to avoid it in your own code. ------- 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: I think that's the nature of C++, and in most real-world apps that perform a useful set of actions, you simply won't be able to avoid it in your own code. what do you mean ??? i was thinking of code that microsoft library programmers that is awful, and that could be other way. look at this :
<vector> line 234:
//...
{for (; _F != _L; ++_P, ++_F)
allocator.construct(_P, *_F);
return (_P); }
//...if i had to write such code myself, i would give explicit names to the variables, unpack the commands, and of course, i would have written such code in a cpp (not a header)....
TOXCCT >>> GEII power
-
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, but there are many different handles which must be closed in different ways. Ej: File Handles (HFILE), C-Files (FILE*), mallocs and news, etc. I've one encountered this problem with a cryptographyc library, where each single object had to be closed with a different function. Developing an autoclose for each object can result even more confusing than using goto. What's more, if you are going to use these handles in other places/programms, I simply prefer to envelope it in a class, where you can add other functionality.
-
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;
}
...
}Now rewrite it so you're acquiring more, and more, and more resources, of different types. Before each
returnstatement, you have to clean them all up all the resources you acquired, so your return blocks grow, and grow, and grow. It's a lot easier to assign a return value to a variable and jump to a cleanup block with agoto. As for using the Resource Acquisition Is Initialisation pattern: MFC 6 still supports the oldsetjmp/longjmp-based TRY/CATCH macros, which in a traditional environment (when you haven't included setjmpex.h) doesn't run your destructors if youlongjmpout of a block. The MFC 6 code-base is therefore defensively coded in case this occurs - explicitly freeing resources. I'm very familiar with this because I mainly code for Pocket PC, where there is no C++ exception handling, and MFC uses the old technique (warning! it still throwsCMemoryExceptionusinglongjmpifnewfails). MFC 7 doesn't support thesetjmp/longjmpscheme (if you look inafx.h, you'll see that defining_AFX_OLD_EXCEPTIONSwill cause an error). However, I'd still expect to see a lot of the old codebase present - if it ain't broke, etc - so you'll see a number of gotos in MFC 7, too. Stability. What an interesting concept. -- Chris Maunder -
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
Man and hear I am thinking you found something coll like Bill Gates Plans for world Domination hidden within comment lines ;P Discovery consist of seeing what everybody has seen and thinking what nobody has thought -- Albert Szent-Györgyi Name the greatest of all the inventors: accident --Mark Twain
-
John Simmons / outlaw programmer wrote: I think that's the nature of C++, and in most real-world apps that perform a useful set of actions, you simply won't be able to avoid it in your own code. what do you mean ??? i was thinking of code that microsoft library programmers that is awful, and that could be other way. look at this :
<vector> line 234:
//...
{for (; _F != _L; ++_P, ++_F)
allocator.construct(_P, *_F);
return (_P); }
//...if i had to write such code myself, i would give explicit names to the variables, unpack the commands, and of course, i would have written such code in a cpp (not a header)....
TOXCCT >>> GEII power
toxcct wrote: i would have written such code in a cpp (not a header).... I'm not sure about Microsoft's compiler but as a general rule template classes must be implemented inside header files so the compiler can generate the code for each of the types they're declared with. Regards, Alvaro
Give a man a fish, he owes you one fish. Teach a man to fish, you give up your monopoly on fisheries.
-
toxcct wrote: i would have written such code in a cpp (not a header).... I'm not sure about Microsoft's compiler but as a general rule template classes must be implemented inside header files so the compiler can generate the code for each of the types they're declared with. Regards, Alvaro
Give a man a fish, he owes you one fish. Teach a man to fish, you give up your monopoly on fisheries.
heinnnnn ??? where have you read that ? can you prove this ? header are not for implementation code. we usually put inside them the declarations, the constants sometimes, the prototypes, but no executable code. i'm waiting for your response with interest !
TOXCCT >>> GEII power
-
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 is the approached supported by the book Code Complete, Funny enough writen by Microsoft!