TITLE: do-while as an implicit goto


Tom: Tom Keane <tkeane@cts.com>

   I too, do not like the way code with that many layers of
   indentation look.  I've been developing in C & C++ for many years,
   and that style just has never been easy to read for me.  

Robert: rmartin@oma.com (Robert C. Martin), 15 Mar 96

Too many levels of indent makes it hard for anybody to read the code.  I 
like to limit indentation to 5 or so levels.   If a function needs more than
that, I will consider splitting the function in two.

   [ SE/SE = single-entry single-exit -adc ]

Tom:

   I do
   however, believe in the merit of SE/SE, and even more than heavy
   indentation I hate code of the form where before each return you
   have to remeber what has been done and try to undo it - it a
   maintenance nightmare.  I came across a style that - to me -
   produces cleaner, easier to read code.  It doesn't have the
   advantage of all the variables being locally scoped, but most of my
   functions are short enough that this doesn't cause too much
   trouble.

Robert:

If your functions are short enough, you should be able to live with
a moderate level of indentation.

Your code:

   BOOL f()
   {
     FILE  * pFile= NULL;
     char  * b= NULL;
     BOOL    bRet= FALSE;

     do {  // Do Once

       pFile= fopen( "myfile", "r" );
       if (pFile == NULL)
	 break;

       b= new char[80];
       if (b == NULL)
	 break;
       //    Perform Necesary Functions

       if (g(f,b) == 0) {
	 // deal with g's error
	 break;
       }
       // do some real neat stuff in here
	 bRet= TRUE;
     } while (FALSE);

     delete [] b;
     if (pFile != NULL)
       fclose( pFile );

   return bRet;
   }

You are hiding something.  Lets expose it:

   BOOL f()
   {
     FILE  * pFile= NULL;
     char  * b= NULL;
     BOOL    bRet= FALSE;

     pFile= fopen( "myfile", "r" );
     if (pFile == NULL)
       goto cleanup;

     b= new char[80];
     if (b == NULL)
       goto cleanup;
     //    Perform Necesary Functions

     if (g(f,b) == 0) {
       // deal with g's error
       goto cleanup;
     }
     // do some real neat stuff in here
     bRet= TRUE;

cleanup:    
     delete [] b;
     if (pFile != NULL)
       fclose( pFile );

     return bRet;
   }

My problem with the do {...} while (0); appraoch is that folks could be
mislead into thinking you actually had some kind of loop there.  They
won't see the "while (0)" until the end, and will wonder what's going
on.  

Also, the do {...} while(0); approach is not a Single Entry/Single
Exit approach.  In SE/SE, every conditional block also has a single
entry and a single exit.  Note the 'cleanup' block has multiple
entries, and each of the conditionals have multiple exits.

Finally, the resource management depends upon state variables, not
upon code position.  While this is not bad, it does mean that the
management of the resources depends upon runtime variables...


[ I also once considered using this construct but compilers frequently 
  warn about such things as while(0). Since I want all my builds to be
  warning and error :-) free, I avoid this. -adc ]

Joe: joe (j.) halpin <jhalpin@bnr.ca>

   There's another alternative that can provide easy to read code with
   SE/SE style. To illustrate, your example might be rewritten as

   void f()
   {
     File* f;
     if ((f=fopen("MyFile", "r")) == 0)
     {
       perror("Can't open MyFile");
       goto done;
     }

     char* b = new char[80];
     if (b == 0)
     {
       printf("Out of heap space");
       goto done;
     }

     if (g(f,b) == 0)
     {
       /* deal with g's error */
       goto done;
     }

     /* do some real neat stuff */

   done:
     delete [] b;
     if(f) fclose(f);
   }

   I know that the use of goto will cause a number of knee-jerk
   reactions, but I don't think there's any question that this is easier
   to read than the indented version, doesn't duplicate code, and has a
   single entry and single exit.

Robert: (followup)

*I* don't find it easier to read, because I am used to the indented
style.  As such, I find arguments that support this style on the basis
of readability less then compelling.  

You are correct, there is no code duplication, however the management
of resources now depends upon a collection of flags.  This isn't evil,
but it is unecessary.  Finally, while the function has a single entry
and a single exit, none of its internal blocks do.  

There is nothing magical about a function with a single entry and a
single exit.  The benefit of SE/SE is only apparent when each block
has a single entry and a single exit.  It is block oriented SE/SE that
provides for resource management.

   When the logic of a function becomes more complex than this example,
   indentation can really make things hard to follow. This can be a good
   alternative in those situations.

Disagree.  The logic of the above function is more complex (in terms
of the number of executable statements, the number of decisions, and
the use and scope of variables, than the corresponding indented
function.

If indentation makes things hard to follow, adding extra decisions,
and widening the scope of variables is *not* the solution.  The
solution is to subdivide the function.