Use of goto in C++?

Started by
32 comments, last by Sneftel 14 years, 1 month ago
2 ways to fix it.

1. Make functions for each of the errors
2. Place those functions inside the if statements instead of after a goto label

There are other ways which are also better, but i'm not going to go over all of them.
Advertisement
You shouldn't use goto ever. If you do, then a velociraptor will suddenly appear and attack you. True story.

Hero of Allacrost - A free, open-source 2D RPG in development.
Latest release June, 2015 - GameDev annoucement

Haha that's great. I hope a velociraptor attacks the next person to use goto. :P
Quote:Original post by Antheus
Quote:Original post by Zahlman

Seriously, though: practically speaking, you don't have a reason to use goto. If you think you have a reason to use goto, show me the code and I'll give you a better idea.


Example taken from MSDN, quickly fixed for in this editor, but hopefully it gets the picture across.
*** Source Snippet Removed ***

Obviously there are other ways, and hopefully the WinAPI will make people cringe, and one could argue that it applies to C portion of C/C++ used for Windows development. Still...



//Assume scoped handle acts kinda like shared_ptr.HBITMAP CopyBitmap( HBITMAP hbm) {    HBITMAP hbmOld2, hbmNew = 0;    BITMAP bm;        scoped_handle<HDC> hdcSrc(CreateCompatibleDC(NULL), DeleteDC);    if (!hdcSRC) { return NULL; }        scoped_handle<HDC> hdcDst(CreateCompatibleDC(NULL), DeleteDC);    if (!hdcDst) { return NULL; };    GetObject(hbm, sizeof(bm), &bm);    scoped_handle<HBITMAP> hbmOld(SelectObject(hdcSrc, hbm), bind(SelectObject, hdcSrc, _1));    if (*hbmOld == NULL || *hbmOld == GDI_ERROR) return NULL;    hbmNew = CreateBitmap( bm.bmWidth, bm.bmHeight, bm.bmPlanes, bm.bmBitsPixel, NULL);    if (hbmNew == NULL) return NULL;    hbmOld2 = SelectObject(hdcDst, hbmNew);    if (hbmOld2 == NULL || hbmOld == GDI_ERROR) return NULL;    BitBlt(hdcDst, 0, 0, bm.bmWidth, bm.bmHeight, hdcSrc, 0, 0, SRCCOPY);    return hbmNew;}

[size=1]Visit my website, rawrrawr.com

Quote:Original post by KieranW
And I agree. There is no need for goto. I don't remember the last time I used it.
There's no need for control structures beyond if statements and goto. [wink] Use whatever's appropriate.

[Website] [+++ Divide By Cucumber Error. Please Reinstall Universe And Reboot +++]

Quote://Assume scoped handle acts kinda like shared_ptr.


You just required me to download STLPort, or implement scoped_handle myself. Use of templates might also cause it to not compile under VC6.

This also require STLPort to be included into build, or my own version must be tested under VC6 SP4 which is only compiler that can build the application, yet I do not have VC6 at all.

The simple old C code (part of C++ application) is now C++ code with increased compilation times and footprint. This may require function to be moved outside of pure C extern namespace into C++ one. Rearranging the code in this way may break the includes and build process due to implicit dependency in order of includes.

It also adds bind, which means that 55496 headers will be now included instead of *one single line*.

In addition, there is now inconsistency - scoped_handle suddenly appeared in code which nobody else knows or cares about.

It may also require extra commit to repository, but I do not have permission to put it into utils or shared, so I need to stick it either in the current file, or as part of bitmap routines. Worst thing that could happen is that someone will find it useful, and start including my bitmap routines.

Also, scoped pointer requires, depending on which resource was obtained, a specialized, somethings per-call specific handle release routine (depending on whether it's shared, reference counted, in DLL), which means another functor which describes destruction process.

Keep in mind - WinAPI development is legacy stuff, applications are old and depend on million of factors, none of which are technical.
It's like anything else. Anybody speaking in absolutes is wrong, end of story. The correct refactor for a goto is a new function in nearly all cases, but sometimes the goto is just plain nicer. Modern code with goto typically enforces the "down-only" restriction, which helps a lot.
SlimDX | Ventspace Blog | Twitter | Diverse teams make better games. I am currently hiring capable C++ engine developers in Baltimore, MD.
Honestly, just DON'T use goto :)
Ok, goto is part of C/C++ you can use it but be able to doesn't mean have to....

I know some people will always find/give a special case where goto could/should be used.

But as soon you think you have a valid reason to use this keyword you'll find another one and then another, etc. In the end your code will be goto-land.

For every goto use there's always an equivalent that doesn't use it :)
So...does anyone want to build a bike shed?
Denzel Morris (@drdizzy) :: Software Engineer :: SkyTech Enterprises, Inc.
"When men are most sure and arrogant they are commonly most mistaken, giving views to passion without that proper deliberation which alone can secure them from the grossest absurdities." - David Hume
Nothing wrong with using goto. It works as advertised, is fully supported in almost every language, even new ones. Just don't abuse it. You can do a lot of stupid things with goto, and 9 times out of ten there is a better solution.

On that tenth time, when that "better solution" is to screw around with simple, easy to read code that works and make something less clear just to simulate what goto already does anyways!

I use it as a nice clean way to break, or jump to different parts of a complicated multilevel loop I use to generate some game content. I find other, over-engineered solutions around this problem laughable. Don't build a tank to solve what a water gun can. And flow control in nested loops is the only place I have ever seen them to be a good use in like 20 years.

Just make sure to set some ground rules. Like Promit said, down only! But also keep it in the same function. Make sure you understand exactly what code you are going to skip over. So you don't end up not executing code that needs to set something up or de-allocate something.

This topic is closed to new replies.

Advertisement