TITLE: breaks in loops and structured programming

KM  = emzee@telerama.lm.com (Kenneth L Moore)
RCM = rmartin@rcmcon.com (Robert Martin), 12 Oct 95


KM:

Please explain what is wrong with returning from the middle of
a loop.  I quess you are advocating a break then a return?

Suppose that I am looking through a linked list in a while loop:

while(ptr)
   {
   if (ptr->a == target) return ptr->b;
   ptr->next;
   }

return NOT_FOUND;

I've actually done things like this, so I would appreciate
knowing what is wrong with the code!  To me it looks very
clean and takes care of the case where the target was not
found.  To implement it with a break out of the loop:

notFound = TRUE;
while(ptr)
   {
   if(ptr->a == target) 
     {
     notFound = FALSE; 
     break;
     }
   }
if (notFound)
   return NOT_FOUND;
else
   return ptr->b;

which seems more cumbersome, less elegant.


RCM:

Look at the words that you are using to describe the software.
"clean", "cumbersome", "elegant".  Although these words are commonly
used as software descriptors, they carry little actual objective
meaning.  The cleanliness, elegance or cumbersome-ness of a software
module cannot be quantified.   Moreover, (as I am sure this thread
will bear out) two people can look at a stretch of code and come to
completely different conclusions.  One will say it is clean and
elegant, the other will say it is cumbersome.

However, there is something objective about both the solutions above.
They both violate one of the principles of Structured Programming.
That principle is that code should be subdivided into blocks which
have a single entry and a single exit.

When there are return statements in the midst of a function, then that
function has multiple exits.  When there are break statements in the
midst of a loop, then that loop has multiple exits.  Regardless of how
clean or elegant one person might consider the code, the fact that
there are multiple exits violates SP.

So what?  What is so great about SP?  Consider that code is often
added to a block such that it depends upon the exit conditions.  If
there are multiple exits from the block, then that code must be placed
at each exit point, and must deal with the differing conditions at
each.  As more exit points are added, more variants of this code must
be created.

Consider how the code you wrote above would have to be changed by a
maintainer who has to deal with allocating a block of memory, or
managing a semaphore.

char* buf = new char[80];
while(ptr)
{
   if (ptr->a == target) 
   {
      delete [] buf;
      return ptr->b;
   }
   ptr->next;
}
delete [] buf;
return NOT_FOUND;

-------------------
char* buf = new char[80]
notFound = TRUE;
Semaphore s;
while(ptr)
{
   s.Sieze();
   if(ptr->a == target) 
   {
     notFound = FALSE; 
     s.Release();
     break;
   }
   s.Release();
}
if (notFound)
{
   delete [] buf;
   return NOT_FOUND;
}
else
{
   delete [] buf;
   return ptr->b;
}

Now consider how this program could be modified:

X* retval = 0;
while(ptr && !retVal)
{
    if (ptr->a == target)
        retval = ptr;
}
return retval;

In this case both the loop and the function have a single exit.  Thus
we could add the buffer and semaphore code in one place.

char* buf = new char[80];
Semaphore s;
X* retval = 0;
while(ptr && !retVal)
{
    s.Sieze();
    if (ptr->a == target)
        retval = ptr;
    s.Release();
}
delete [] buf;
return retval;

KM:

I'm sure you had a good reason for saying that you shouldn't
return from inside a loop and I would like to know what that
is as I would like my code to be readable and CORRECT!

RCM:

I hope the above has demonstrated those good reasons.  It is never
enough to look at a piece of code and think "gee that's clean".  You
must always think of it in terms of the kinds of changes that are
likely to be made to it.  If those changes are not going to be simple,
or if they are going to create situations that are error prone, then
it does not matter how "clean" the code looks...


[ I tend to program with breaks and returns for only the very simplest
  cases. If the flow of control gets more complex than this simple
  level, I switch over to structured programming style. (I know, I
  know. This is not consistent style.)

  Here is the way I would program this loop.

  class SCharArray
  {
    public:
  	SCharArray (int nChars); // allocate from heap
        ~SCharArray ();          // free memory
        operator char* ();       // convenience
    private:
	char* cp;
  };

  class SSemaphore
  {
    public:
 	SSemaphore  () { fSemaphore.Seize   (); }
 	~SSemaphore () { fSemaphore.Release (); }
    private:
	Semaphore fSemaphore;
  };

  --- body of function ---

  SCharArray buf (80);

  X* retval = 0;

  while(ptr)
  {
    SSemaphore s;

    if (ptr->a == target)
        return ptr;
  }

  return retval;


  By making a generic class for handling semaphores and
  character buffers, you can express your intent precisely
  and make maintaining it nearly bullet proof against future
  modifications. 

  These particular techniques also are required for exeception
  safe code.

  -adc ]