goto

[Phlip]

In 10+ years of C/C++ coding I've never seen goto used except to exit
from an inner loop, which is where it's commonly recommended.

'goto' has been frowned upon for well over 10 years. However, if you have
never encountered any related design smell, consider yourself amazingly
lucky.

 

[E. Mark Ping]

'goto' has been frowned upon for well over 10 years. However, if you have
never encountered any related design smell, consider yourself amazingly
lucky.

Yes, Dijkstra's "Go To Statement Considered Harmful" was written
before C was created, yet people still unthinkingly carried the "goto
is bad" mantra beyond it's intent. The restrictions C places on goto
and the common advice (only use for escaping inner loops) pretty much
invalidates the dogma.

 

[Andrew McDonagh]

E. Mark Ping wrote:




'goto' has been frowned upon for well over 10 years. However, if you have
never encountered any related design smell, consider yourself amazingly
lucky.

seconded!

 

[Chris Croughton]

I know some say that this is bad, I but do not agree with them. This
is an unneeded constraint, just to make our lives more difficult (or
in other words, they think that by imposing such restrictive rules
will be able to protect bad programmers from bad programming).

But the same is true about blanket forbidding of goto. Unclear
programming is always bad, no language structure is inherrently bad
(well, OK, Intercal!) it's how you use it.

I can understand the use of goto for the situations that I described.
For example, in application programming, in non-desperate for speed
conditions, why should one use goto?

For clarity when appropriate.

void func(...)
{
for (...)
{
for (...)
{
for (...)
{
/* do stuff */
if (something_wrong)
goto cleanup;
/* do more stuff */
if (something_wrong)
goto cleanup;
}
}
}
cleanup:
/* do cleanup stuff */
}

It's a lot clearer than using flags to do it, and return isn't
appropriate because the 'cleanup' code would need to be duplicated which
is a maintenabce risk.

May you say in what other conditions it is useful?

Duh, breaking out of nested loops (see above). Which is where we came
in...

Chris C

 

[Ioannis Vranos]

For clarity when appropriate.

void func(...)
{
for (...)
{
for (...)
{
for (...)
{
/* do stuff */
if (something_wrong)
goto cleanup;
/* do more stuff */
if (something_wrong)
goto cleanup;
}
}
}
cleanup:
/* do cleanup stuff */
}

It's a lot clearer than using flags to do it, and return isn't
appropriate because the 'cleanup' code would need to be duplicated which
is a maintenabce risk.

This is better done with exceptions.

Duh, breaking out of nested loops (see above). Which is where we came
in...

In the above you use it to perform error handling.


--
Ioannis Vranos

http://www23.brinkster.com/noicys

[I am using 90 characters word-wrapping - (800/640) *72= 90 or better described as:
(800/640) *80 - 10 for quotation= 90. If someone finds it inconvenient, please let me know].

 

[Andrew McDonagh]

But the same is true about blanket forbidding of goto.

I think one problem a lot of people have with it, is seeing it being
used/abused so often. Yes this should not mean we forbid it, but its use
should be very limited.

I had the great 'pleasure' to have to add a new feature to an existing
product, in which the original developer could not see an easy way out
of a 2 tier nested for loop with several If conditions inside. So they
used goto BAD_BOY:

He knew it was rubbish code, but time pressure meant he wanted something
out quickly. This code was supposed to remain in existence for a few
days until he could make it better. He even left a comment saying as much.

It lived for almost 2 years before finally being refactored away in
order to add functionality.

Unclear programming is always bad, no language structure is inherrently bad
(well, OK, Intercal!) it's how you use it.




For clarity when appropriate.

void func(...)
{
for (...)
{
for (...)
{
for (...)
{
/* do stuff */
if (something_wrong)
goto cleanup;
/* do more stuff */
if (something_wrong)
goto cleanup;
}
}
}
cleanup:
/* do cleanup stuff */
}

It's a lot clearer than using flags to do it, and return isn't
appropriate because the 'cleanup' code would need to be duplicated which
is a maintenabce risk.

The problem I have with this type of code structure though, is that
whilst I can see on the screen the 3 nested for loops, the two If
conditions, the Do stuff AND finally the cleanup stuff, I would either
have to use a very small font, a very large monitor or scroll about to
see it all.

What I'm getting at, is that whilst it may appear to be 'clear' the
sheer amount of code means we can't easily see what is happening.

I'd also bet that there is significant duplication within the class(es)
.. If we have to do 3 nested for loops here, you can 'usually' bet
another place in the code base is doing the same nested loops.

Where as, if we separated 'query', 'retrieval' and 'do stuff to',
applied the appropriate design patterns (visitor, command what ever..)
then we usually end up with much smaller methods, no or little
duplication and just as easy and clear code.

 

[Andrew McDonagh]

This is better done with exceptions.

If the 'something wrong' is an exceptional case, then I'd agree
exceptions are appropriate. However, if the something wrong is not an
exceptional case, then using exceptions is really just another
interpretation/implementation of goto.

for example, it may be appropriate to throw an exception if one of the
function params is out of bounds.

if (param > arraySize) {
throw new IndexOutOfBoundsException();
}

where as, it might not be appropriate to throw an exception just because
something does not exist.

if (something == null) {
throw new NotFoundException();
}

 

[Ioannis Vranos]

for example, it may be appropriate to throw an exception if one of the
function params is out of bounds.

if (param > arraySize) {
throw new IndexOutOfBoundsException();
}

Yes.


where as, it might not be appropriate to throw an exception just because
something does not exist.

if (something == null) {
throw new NotFoundException();
}

Yes. It will not be appropriate if for example the null is signifying the expected end of
a sequence. But then, "something wrong" is not an accurate term to describe that.


That pseudocode was displaying error handling, with the use of goto to jump to the error
handling code, clearly substituting the exception handling mechanism.



--
Ioannis Vranos

http://www23.brinkster.com/noicys

[I am using 90 characters word-wrapping - (800/640) *72= 90 or better described as:
(800/640) *80 - 10 for quotation= 90. If someone finds it inconvenient, please let me know].

 

[Andrew Koenig]

If the 'something wrong' is an exceptional case, then I'd agree exceptions
are appropriate. However, if the something wrong is not an exceptional
case, then using exceptions is really just another
interpretation/implementation of goto.

Not really.

There are (at least) three fundamental differences between exceptions and
goto:

1) The author of a throw statement does not need to know where the
exception will be caught.

2) It is possible to transfer information between the throw and the
corresponding catch.

3) It is possible to reach a catch only from code executed as part of
the corresponding try. It is possible to reach a label from anywhere in the
entire function (subject only to the restriction that you cannot jump from a
place where a variable is out of scope to a place where it is in scope).

 

[Ioannis Vranos]

where as, it might not be appropriate to throw an exception just because
something does not exist.

if (something == null) {
throw new NotFoundException();
}

About the not-exists stuff, as a return value, let me give you an example. .NET is *very*
high-level, OO and modern (meaning it uses all modern facilities). It uses exceptions
completely for error-handling, for every single error it throws an exception.

So if you want to establish a simple network connection for example and it fails, it
throws an exception. It doesn't return a null pointer. And I think this is the right approach.

Since we have exceptions, we do not need any error signalling through value return.


--
Ioannis Vranos

http://www23.brinkster.com/noicys

[I am using 90 characters word-wrapping - (800/640) *72= 90 or better described as:
(800/640) *80 - 10 for quotation= 90. If someone finds it inconvenient, please let me know].

 

[Clark S. Cox III]

For clarity when appropriate.

void func(...)
{
for (...)
{
for (...)
{
for (...)
{
/* do stuff */
if (something_wrong)
goto cleanup;
/* do more stuff */
if (something_wrong)
goto cleanup;
}
}
}
cleanup:
/* do cleanup stuff */
}

It's a lot clearer than using flags to do it, and return isn't
appropriate because the 'cleanup' code would need to be duplicated which
is a maintenabce risk.

What about this (no duplication, exception safe, no potential for goto
jumping across constructors/destructors, etc.):

void func(...)
{
struct cleanup {
~cleanup() { /* do cleanup stuff */ }
} cleaner;

for (...)
{
for (...)
{
for (...)
{
/* do stuff */
if (something_wrong)
return; /* or throw*/

/* do more stuff */
if (something_wrong)
return; /* or throw*/
}
}
}
}

 

[Phlip]

What about this (no duplication, exception safe, no potential for goto
jumping across constructors/destructors, etc.):

void func(...)
{
struct cleanup {
~cleanup() { /* do cleanup stuff */ }
} cleaner;

for (...)
{
for (...)
{
for (...)
{
/* do stuff */
if (something_wrong)
return; /* or throw*/

/* do more stuff */
if (something_wrong)
return; /* or throw*/
}
}
}
}

Try to pass local variables into ~cleanup(), to reliably clean them up. Your
little class rapidly gets very ugly - it grows a constructor, members, etc.

RAII works best when handles destruct themselves. Put another way,
destruction is an encapsulated detail of the handle, not of the function
using the handle.

 

[Ioannis Vranos]

So if you want to establish a simple network connection for example and
it fails, it throws an exception. It doesn't return a null pointer. And
I think this is the right approach.

Actually, it does not throw an exception on connection failure but a positive integer is
returned in connection success, and 0 in network failure.

However for all unexpected errors an exception is thrown, like when trying to open a file
with write access while it is being used by another application, for example.

 

[Ioannis Vranos]

Actually, it does not throw an exception on connection failure but a
positive integer is returned in connection success, and 0 in network
failure.

Actually I think it is a mistake of the book I am reading, it *does* throw an exception.


http://msdn.microsoft.com/library/e...stemnetsocketstcpclientclassconnecttopic2.asp

 

[Dave Rahardja]

Try to pass local variables into ~cleanup(), to reliably clean them up. Your
little class rapidly gets very ugly - it grows a constructor, members, etc.

RAII works best when handles destruct themselves. Put another way,
destruction is an encapsulated detail of the handle, not of the function
using the handle.

I prefer...

int cleanup_variable = 0; // Local vars needed for cleanup

class my_exception {};

try {
for (...)
{
for (...)
{
for (...)
{
// ...
if (something_wrong)
throw my_exception;

// ...
if (something_wrong)
throw my_exception;
}
}
}
} catch (my_exception&) {
// cleanup
// Optionally rethrow another exception
}

With a good compiler and optimizer, the run-time behavior of this code
should be equivalent to a goto to the outer block, with the additional
bonus that destructors for objects created in the for scope will be
properly called on the way out.

-dr

 

[Phlip]

Phlip wrote:

I prefer...

int cleanup_variable = 0; // Local vars needed for cleanup

class my_exception {};

try {
for (...)
{
for (...)
{
for (...)
{
// ...
if (something_wrong)
throw my_exception;

// ...
if (something_wrong)
throw my_exception;
}
}
}
} catch (my_exception&) {

catch(...), so called functions can throw safely.

// cleanup
// Optionally rethrow another exception
}

With a good compiler and optimizer, the run-time behavior of this code
should be equivalent to a goto to the outer block, with the additional
bonus that destructors for objects created in the for scope will be
properly called on the way out.

However, RAII works best when handles destruct themselves. Put another way,
destruction is an encapsulated detail of the handle, not of the function
using the handle.

 

[Ioannis Vranos]

I prefer...

int cleanup_variable = 0; // Local vars needed for cleanup

class my_exception {};

try {
for (...)
{
for (...)
{
for (...)
{
// ...
if (something_wrong)
throw my_exception;

// ...
if (something_wrong)
throw my_exception;
}
}
}
} catch (my_exception&) {
// cleanup
// Optionally rethrow another exception
}

With a good compiler and optimizer, the run-time behavior of this code
should be equivalent to a goto to the outer block, with the additional
bonus that destructors for objects created in the for scope will be
properly called on the way out.

A style that I saw in a .NET book recently. I am just posting it here to see comments.


for(...)
{
try
{
// Network connection attempt with object in the managed heap
}

catch(Exception *)
{
break;
}
}



I guess this makes sense for garbage collected objects in the heap.