The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

The Design of a Routine or Routine Design?

Given a routine with multiple points of failure, do you prefer that a routine exit at the point of failure (yielding multiple points of return within the routine) or a single point of return?  For example, the following routine can be written in two different ways, assuming a C-like syntax:

If(DoSomething1() == 0)
    Return;

If(DoSomething2() == 0)
    Return;

If(DoSomething3() == 0)
    Return;

// More code here

Or

If(DoSomething1() != 0)
{
    If(DoSomething2() != 0)
    {
        If(DoSomething3() != 0)
        {
            // More code here
        }
    }
}

Given these two, which do you prefer? 

Thanks!
David Brownell
David Brownell Send private email
Wednesday, October 06, 2004
 
 
I prefer the former.  I disagree that functions should have one return point always.  In the general case, yes, but there are lots of specific cases where multiple return points makes the code much cleaner (which is the point of the "rule" in the first place)

bool someFunction(TInt somevalue1, TInt somevalue2)
{
  // Validate the input, if not correct return false
  if (somevalue1 < 10 || somevalue1 > 100) return false;
  if (somevalue2 < 5 || somevalue2 > 50) return false;

  // Do the function here

  // Function processed correctly
  return true;
}

Of course, in C++ code you'd probably raise an exception.  Or return an error code (ERR_BAD_INPUT) than simply return  false.
Almost Anonymous Send private email
Wednesday, October 06, 2004
 
 
I got used to use a return value variable, for no good reason. Perhaps one too much state machine coded :-)

bool someFunction(TInt somevalue1, TInt somevalue2)
{
  // Validate the input, if not correct return false
  bool result = false;
  if (
    somevalue1 >= 10 && somevalue1 <= 100 &&
    somevalue2 >= 5 && somevalue2 <= 50 ) {
    // Do the function here

    result = true;
  }
  return result;
}
Leonardo Herrera Send private email
Wednesday, October 06, 2004
 
 
Often, I use:

int foo(...) {
  int error = 0;

  if(!error) {
  // some code that might set error != 0
  }

  if(!error) {
    // more code that might set error != 0
  }

  return error;
}

But I'm not a fanatic. Use whatever fits the appropriate situation.  Sometimes, due to a weird loop condition, return is your best friend.
anon Send private email
Wednesday, October 06, 2004
 
 
I typically return immediately when I know I can do no more useful work. That keeps the nesting level down, and I think it's much easier to read and comprehend. I disagree strongly that functions should have only one exit, because in my experience that leads to code that is way too complex and hard to read.
sid
Wednesday, October 06, 2004
 
 
I *prefer* to write and read code with one return statement, and if something fails I'd like it to throw an Exception.  Now, this isn't always the best case, so I'm not going to turn this into a stupid religious arguement, but mostly it seems clearer to me when there is only one return statement.
Vince
Wednesday, October 06, 2004
 
 
The multiple return points problem is really only a problem when the function takes up more than one screen. Nowadays if you find yoursefl with a big function, you break it up, so having a bunch of return points within a function, all of which are visible at once, is not a problem.
Dennis Atkins
Wednesday, October 06, 2004
 
 
You know, Dennis, that's an interesting point.  So I'll bring up another question.. 

How many people these days code functions that are larger than one screen?

One screen is almost always the maximum size of my functions; sometimes I have a few that are two screens.  The majority of functions are only between 5 and 20 lines long.  I don't go about that on purpose, it just seems to be the way it works out.
Almost Anonymous Send private email
Thursday, October 07, 2004
 
 
I prefer the multiple exit points approach.

The alternative is often to have multiple contrived if statements, or (dare I even mention it in this day and age!?!?) goto statements.

I like the idea of short routines, as they are easy to understand and can save a lot of typing, where they replace repetitive code. They also offer the compiler the chance to optimise by inlining.

However, I do sometimes have routines that are many screens long, do to the complexity of the situation. These cannot sensibly be broken up, except in a contrived way, which I object to.
Nemesis
Thursday, October 07, 2004
 
 
The latter, it makes it easier to read. Sadly I have to write the former due to circumstances.

Thursday, October 07, 2004
 
 
There's a problem with the multiple exit point (and I don't avoid them religiously, just try and minimise them), and that's that if there's cleanup to be done you have to remember to do that cleanup with every exit and sometimes that leads to messier and snarly problems of its own.

So on the whole I try to use a cascading structure.  If its a true/false return then intiialise the return value to false, only make it true if it is positively true and drop out the bottom with the return.

This isn't always possible but whenever it is I follow it.
Simon Lucy Send private email
Thursday, October 07, 2004
 
 
If there is cleanup to be done its best to wrap it in some kind of smart class, you can then safely forget about it. If something else outside your control throws an exception you don't need to worry about it.

I'm sure this question came up on these forums about a year ago and I seem to remember the balance was towards a single exit point then.
Tony Edgecombe Send private email
Thursday, October 07, 2004
 
 
I agree with Tony and Dennis. If there's cleanup to be done, at least in C++ you should use RAII. I very rarely do manual cleanup, which is why I can return whenever I want. I also rarely have functions that are longer than one screen.
sid
Thursday, October 07, 2004
 
 
(Brace for flame attack) not a big believer in the 'maximum one page per function' idea. I don't really see that it makes it easier to divide a five page function up into five functions, then instead of trying to keep track of five pages of code, you are having to keep track of all the jumping around. IMHO, the most important principle is the same logic never appear in more than one spot. So the division point for me is when a part of a function is or could be called from more than one place.
NetFreak Send private email
Thursday, October 07, 2004
 
 
Ummm C++ isn't the only show in town.
Simon Lucy Send private email
Thursday, October 07, 2004
 
 
goto can be your friend. it's only being used here for error handling, not arbitrary jumps. and error handling is *always* ugly.

do something
if (failed) goto exit;

do next thing
if (failed) { handle special case; goto exit }

and so on

exit:
clean up everything
return resultcode

with exceptions, you can use try/catch/finally to achieve much the same result.
mb Send private email
Friday, October 08, 2004
 
 
the 'spagetti' of multiple return points is, like hungarian notation, something superceded by tools.

So many tools these days do 'folding' and show faint vertical guides to show nesting.  The fancy ones show arrows out for return, break, goto etc too; and it is this last thing that makes good so much more navigatable, more like flowcharting.
i like i
Friday, October 08, 2004
 
 
On my first "real" project right out of college, the first "real" code I saw contained functions with if-statements nested ten levels deep, all of them closed at the end right before the only return statement.  I wanted to quit, right then and there.

Fortunately, that was inherited code, the current team didn't enforce that style as a standard, and I remain employed to this day.
Mike Bland Send private email
Friday, October 08, 2004
 
 
I think Yes, use the "return statement with an error code" multiple places in a function is a good idea.  Using 'goto' to get a single exit point is a little more clunky, but does essentially the same thing.

Throwing an exception at that point does THE SAME THING (except of course, you're now using the try-catch blocks, instead of throwing result exit values around.)

All of these approaches try to combine efficiency ("Don't process any more, if something bad happened") with readability ("You nested the IF statements HOW deep?").  As such, I believe this is a good departure from the 'single-entrance, single-exit' code approach.
AllanL5
Friday, October 08, 2004
 
 
Allan & all,

how much do you nest if statements in an if/else if/else structure?
Dino
Friday, October 08, 2004
 
 
I hate nesting.  Anything over two levels of nesting irks me.  If it's four or more levels of nesting -- that's crazy to me.
Almost Anonymous Send private email
Friday, October 08, 2004
 
 
I typically don't nest more than two levels deep. It's very rarely necessary in C++.
sid
Friday, October 08, 2004
 
 
Theoretically I allow myself multiple returns from a function, but practically I generally find I want to add some tracing (=printf) dignostic to echo the return state.

An informative, well-designed, tidy trace statement is usually a little messy to code, and getting the right balance of detail can involve repeated refinements.

Best to have just one copy of this mini-report at a single return point.
joe coder
Monday, October 11, 2004
 
 
There's a McCabe metric called "Cyclomatic Complexity" which advocates keeping the IF-THEN nesting to less than the 7 +- 2 level humans can comprehend.  The impact of going over this limit is that you get a routine which is very hard to verify that it is correct, or fix if there is a bug in it.

Personally, I feel the benefit of the multiple 'return' statement after an 'if' statement is that it can make very clear what conditions you have successfully met to reach a certain point in a routine.

So, in 'normal' routines, I tend to keep the IF nesting down to two or three.  There do exist routines referred to as 'Transaction Centers' (user input selection being a typical example) where there are a LOT of options to be selected between.  Here a 'switch' statement can make things a little clearer. 

I also use the 'if (xxx == opt) { /* call handler_xxx */ ; return(StatCode); } model in this instance, if it is clearer.
AllanL5
Monday, October 11, 2004
 
 
I'm also do not like the 'else if' construct.  Instead, if I really HAVE to nest 2 or 3 deep, I'll use:

if (a)
  {
    /* Stuff For a */
  }
else
  {
    if (b)
      {
        /* Stuff for b */
      }
    else
      {
        /* Stuff for default... */
      }
  }

The 'else-if' construct can be nice in that it prevents the 'paren-creep' toward the right-margin, but it can make the scope of the decision a little less clear.

if (a)
  {
    /* Stuff for a */
  }
else if (b)
  {
    /* Stuff for b */
  }
else  /* Default */
  {
    /* Stuff for default */
  }

So, the confusion factor -- /* Default */ looks like it happens as an 'else' for 'if (a)', when in fact it's inside the else of 'if (a)' AND the 'else' of 'if (b)'.
AllanL5
Monday, October 11, 2004
 
 
In general, whenever a series of statements could make sense as its own separate entity, I'll push it into a function, function object, or both (where the object's operator() calls the function).  This eliminates a lot of nesting, self-documents what's happening, forces me to think in terms of complete and minimal interfaces for functions and classes, and makes things more testable to boot.  Maintainability wins.

In C++, templated algorithms (from the STL or hand-rolled) plus function objects eliminate tons and tons of loops, which are a form of nesting.  Rather than code up one mega-loop by hand, I'll make a series of transform()/for_each()/etc. calls that are straightforward in self-documenting the exact intent of each set of iterations.  If profiling reveals a bottleneck, the calls can always be unrolled into a more efficient loop later, but I haven't had to do so yet and the improved maintainability more than compensates for any potential drawbacks.

I do find things getting rather Lisp-y in concept and appearance when I end up processing lots of lists in this fashion, however...  ;-)
Mike Bland Send private email
Thursday, October 14, 2004
 
 
For me, the problem of multiple exit points is bound to the concept of preconditions.

Preconditions have to be fulfilled, before the function can even start to do what it is supposed to do. So, IMHO it makes sense to exit the function right away, clearly distingushing the functional code from the precondition code rather then mixing it up.

Whilst the function had not started doing any processing at this time, the problem of cleanup code should not occur.

Within the functional code however, I usually prefer one exit point. Just because it makes it easier to modify the function later on, e.g. if you want to add - well - cleanup code or logging on failure or for just setting a breakpoint. And of course you can set a watch on the return value.
Gerd Riesselmann
Friday, October 15, 2004
 
 
For me, "single return point" refers to the accomplishment of the function and possible return of functional values. A failure point is not really a return point. In this case the less disruption to the readability of the actual function code the better. So I'll usually bail, as safely as possible.

A status code is not usually part of the function, its a test of whether the routine thinks it successfully completed its function. ("Get Printer Status" is a legitimate function and result.) The ability to throw an exception (which is a kind of controlled Go To) is often better than always remembering to test a status code and increasing the nesting of Ifs. However, you often need to pass the status code up to the exception handler.

I want code that is readable and reliable. So I'm less concerned with rules than the final result. I've seen a single well used Go To make a large sequence of code much smaller, easier to read and reliable than it would be without it.

Physicians take an oath to "do no harm" to their patients. A routine should do the same, especially when it fails. However, this can be difficult to achieve. Until the past century, surgeons were not considered to be physicians because they cut their patients, obviously violating the "do no harm" oath despite the benefits. If a routine encounters a failure point, what should it do to minimize damage? Compiler writers and users are familiar with the "message avalanche" that happens when one failure point doesn't prevent other needless failures elsewhere. If a data structure is inconsistent, should something be done to prevent further errors? If so, this should clearly be separated out from code that implements the normal function of a routine. Often error handling is the hardest part of designing a good system.
Dave Lathrop Send private email
Monday, October 18, 2004
 
 
Gerd, I'd like to ask what exactly you feel falls under preconditions.  Are you referring to things like bounds-checking that should instantly return an error of some type?  Or are you referring to things that make the function automatically false (or true!), or give it some other simple value?

In my experience, I have two real exit points in my code.  The first is typically the kind I described above where the parameters are such that the program can instantly say "This is false" or "I need to call a different function to answer this one".  An example of this can be found in Java, where the equals member in the Object class should always be of the same type as the original object-so the first line of all my equals functions are of the form

if ( !( o instanceof MyType ) ) return false;

Other examples would be of the kind

public boolean drag( Point p ) {
  //check if it is a normal drag
  if ( !specialPoint( p ) ) return super.drag( p );

  //do a special drag
  < code >
  return true;
}

After I have gotten past the initial checks of this sort, I tend to create a return variable which is returned at one spot at the very end.  So I have a group of returns at the beginning and one return at the end.  I feel that for non-error conditions, this is an appropriate, readable construct.
The Haertchen Send private email
Tuesday, October 19, 2004
 
 
Hello Haertchen,

Your experience and what I meant should be the same, I think.

I boroughed the term "preconditions" from the "Programming By Contract" paradigm. I think it is also used in aspect oriented programming, though I'm not sure with this. However, I just use the term, not the concept.

A precondition is an assumption about the data a function processes and the most common assumption is that the input params are valid. But life is cruel, so the assumptions have to be verified. You sure know code like this (C++):

bool doSomething(someClass* inst)
{
  if (inst == 0) return false;
  if (inst->isLocked() == false) return false;
 
  .. do something with inst ..
}

The precondition in this case is, that the pointer passed is valid and the instance has been locked.

I always nail down preconditions in the function documentation (and in the specification, if this falls into my responsability) by explicitly writing stuff like "The function assumes to be passed a valid and locked instance and will return false otherwise."

As far as I see, your examples check precondition, too: Instance should be of given type in the first and point should be special type in the second example.
Gerd Riesselmann
Tuesday, October 19, 2004
 
 
OK, that makes sense.  I definitely agree with the use of preconditions the way you describe it.  The alternative is having lots of if statements that surround the entire logic, and that just looks worse.

I'm not sure if you understand my second example or not, though.  In my code, I often use inheritance to modify a base behavior-but only in some circumstances.  In the example I give above with the use of point, for instance, if the object contains the point, it defaults to the super-class implementation.  Otherwise it does the sub-class implementation.  In this sense, it is not a precondition, because what the user entered was valid.  It's just a matter of deciding whether to reroute the logic or not.  Does this fit under what you mean by precondition?  And do you think it is an acceptable use of return?

I've found this useful.  Maybe it's just me, and maybe other people would condemn this usage.  I just thought I'd mention it.
The Haertchen Send private email
Tuesday, October 19, 2004
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz