Only one point of return

[cppquester]

A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

Regards,
Marc

Example:

bool f()
{
if( !pointer1) return false;
pointer1->doSomething();

if( !pointer2) return false;
pointer2->doSomething1();

return true;
}

vs.

bool f()
{
bool retVal=true;
if( pointer1)
{
pointer1->doSomething();
}
else
retVal=false;

if( pointer2)
{
pointer2->doSomething();
}
else
retVal=false;

return retVal;
}

[Ian Collins]

cp********@googlemail.com wrote:

A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

It is a fairly common coding standard.

In my opinion it makes more sense for C than C++, provided your code is
exception safe an early return shouldn't do any harm. Some may argue
that a single point of return makes debugging easier.

A common problem early returns with either C or C++ that isn't exception
safe is resource leaks where the resource in question is only released
at the end of the function or before the last return.

--
Ian Collins.

[duane hebert]

<cp********@googlemail.comwrote in message
news:11**********************@o61g2000hsh.googlegr oups.com...

>A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

Regards,
Marc

Example:

bool f()
{
if( !pointer1) return false;
pointer1->doSomething();

if( !pointer2) return false;
pointer2->doSomething1();

return true;
}

vs.

bool f()
{
bool retVal=true;
if( pointer1)
{
pointer1->doSomething();
}
else
retVal=false;

if( pointer2)
{
pointer2->doSomething();
}
else
retVal=false;

return retVal;
}

This second example is different than the first since
it goes to the pointer2 part even if pointer1 doesn't
exist.
Everyone has an opinion but IMO in C++, maintaining
a single entry point leads to more obfuscated code
rather than less (which is, I believe, the argument in
favor of it.)

[duane hebert]

"duane hebert" <sp**@flarn.comwrote in message
news:9p********************@weber.videotron.net...

This second example is different than the first since
it goes to the pointer2 part even if pointer1 doesn't
exist.
Everyone has an opinion but IMO in C++, maintaining
a single entry point leads to more obfuscated code

a single exit point (duh)

[werasm]

Alf P. Steinbach wrote:
but if so, try

>
bool f()
{
if( pointer1 != 0 )
{
pointer1->doSomething();
if( pointer1 != 0 )
{
pointer2->doSomething1();
return true;
}
}
return false;
}

Hmmm, or:

bool f()
{
if( pointer1 == 0 ){ return false; }
pointer1->doSomething();
if( pointer2 == 0 ){ return false; }
pointer2->doSomething();
return true;
}

I like this because it seems less complex to read and understand, but
the rationale is of course subjective (as is SESE and SEME in C++,
depending to who you speak). I like getting the pre-conditions out of
the way before handling the meat. SEME for me seems to do this better
than SESE.

Another alternative...

template <class T>
class ExistentPtr
{
public:
ExistentPtr
( T*& p );
T& operator*() const; //throws if pointer_ NULL.
T* operator->() const; //throws if pointer_ NULL.

private:
T*& pointer_;
};

bool f()
{
ExistentPtr<Typep1( pointer1 );
ExistentPtr<Typep2( pointer2 );

try
{
p1->doSomething();
p2->doSomething();
return true;
}
catch( const null_ptr_error& )
{
return false;//
}
}

Perhaps, if a function is written properly, it may never require
visible ifs
for the sake of handling errors, but this is very subjective.

Regards,

Werner

[James Kanze]

On Aug 1, 11:09 am, Ian Collins <[e-mail]:

[e-mail] wrote:

A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

It is a fairly common coding standard.

In my opinion it makes more sense for C than C++, provided your code is
exception safe an early return shouldn't do any harm. Some may argue
that a single point of return makes debugging easier.

The real argument is that it makes understanding the code and
reasoning about the correction of the code a lot easier, so you
don't have to debug. It's more than just common; I'd say that
it's present almost always where code review is taken seriously
(which in turn means everywhere where the quality of code is
taken seriously).

About the only exceptions I've seen is clearing out
pre-conditions at the top of the function, and in a few cases,
if the function consists only of a single switch or a single
list of chained if/else if/else, with all branches ending in a
return. (The latter case is rarely allowed, probably because it
is difficult to formulate precisely.)

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Aug 1, 11:11 am, "Alf P. Steinbach" <[e-mail]:

* [e-mail]:

A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

It's called SESE (Single Entry Single Exit), as opposed to SEME (Single
Entry Multiple Exit).

SESE is a good idea in languages like C, which have no provision for
cleaning up automatically on function return. There, when maintaining
code, you don't want cleanup code to be jumped over by an early return.

The issue has more to do with understanding and reasoning about
code than with the any cleanup code.

C++ code, on the other hand, must be prepared to handle exceptions at
any point, i.e. it's multiple exit at the outset.

I've noticed that in well written C++, any exceptions tend to
cluster at the top of the function, in a manner coherent with
the one frequently accepted exception: checking pre-conditions.
You don't want to exit code at just any old point.

Or at least, if it's written by competent people, it will be
designed for that, using RAII. So there's much less that can
go wrong by using early normal returns, and the only factor
should be what's more /clear/, in the context of a given
function.

Example:

bool f()
{
if( !pointer1) return false;
pointer1->doSomething();

if( !pointer2) return false;
pointer2->doSomething1();

return true;
}

Bad, because it uses global variables, and is unclear.

Are you really want it to do one of the things without doing both?

But if so, try

bool f()
{
if( pointer1 != 0 )
{
pointer1->doSomething();
if( pointer1 != 0 )
{
pointer2->doSomething1();
return true;
}

Just curious, but what invariants hold here?

}
return false;
}

Which is easily (and more clearly) rewritten as:

bool
f()
{
bool succeeded = false ;
if ( pointer1 != NULL ) {
pointer1->doSomething() ;
if ( pointer2 != NULL ) {
pointer2->doSomething() ;
succeeded = true ;
}
}
return succeeded ;
}

By naming the condition, you've adding important semantic
content for the reader. Even better would be something like:

bool
maybe1()
{
if ( pointer1 != NULL ) {
pointer1->doSomething() ;
}
return pointer1 != NULL ;
}

bool
maybe2()
{
// Alternate way of writing it...
// (only for those who swear by functional
// programming:-)
return pointer2 != NULL
&& (pointer2->doSomething(), true) ;
}

bool
f()
{
return maybe1() && maybe2() ;
}

(But like you, I have a great deal of difficulty imagining a
case where you'd want to do half the job, and then report that
you'd not done any of it.)

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[Jorgen Grahn]

On Wed, 1 Aug 2007 08:55:31 -0400, duane hebert <sp**@flarn.comwrote:

>
<cp********@googlemail.comwrote in message
news:11**********************@o61g2000hsh.googlegr oups.com...

>>A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.

....

Everyone has an opinion but IMO in C++, maintaining
a single entry [he means exit] point leads to more obfuscated code
rather than less (which is, I believe, the argument in
favor of it.)

My experience, too, kind of.

I haven't been in a project where we had to follow the rule, but I
have worked with people who have adopted it, to some extent.

I can well believe that otherwise well-written code (where functions
are the right size and have the right amount of responsibility) are
often better and more elegant if they are rewritten with a single exit
point.

But if the code, the programmers and the time schedule are only
half-decent and you enforce the single-exit-point rule (it's easy to
spot and comment on in a code review) I believe you tend to end up
with the ugliness and bugs that come with:

- deeply nested if/else logic
- numerous local boolean flags controlling the flow
- the return value being set in half a dozen places, on different code
paths

For a concrete example: I hate reading a two-page function which ends
with "return result;" and searching back for all places where 'result'
is (or isn't) initialized with a relevant value.

/Jorgen

--
// Jorgen Grahn <grahn@ Ph'nglui mglw'nafh Cthulhu
\X/ snipabacken.dyndns.org R'lyeh wgah'nagl fhtagn!

[James Kanze]

On Aug 2, 12:18 pm, "Alf P. Steinbach" <[e-mail]:

* James Kanze:

On Aug 1, 11:11 am, "Alf P. Steinbach" <[e-mail]:

* [e-mail]:

>A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

It's called SESE (Single Entry Single Exit), as opposed to SEME (Single
Entry Multiple Exit).

SESE is a good idea in languages like C, which have no provision for
cleaning up automatically on function return. There, when maintaining
code, you don't want cleanup code to be jumped over by an early return.

The issue has more to do with understanding and reasoning about
code than with the any cleanup code.

Well, that's a generalization:

It's a relativisation. The original arguments in favor of SESE
(e.g. by Dijkstra et al.) did not involve cleanup code. That
doesn't mean that cleanup code isn't important in practice.
Just that it wasn't the original motivation (and that even when
cleanup code isn't an issue, e.g. when you are using RAII, the
original arguments hold).

cleanup code is just the most important
in practice, because maintainance is more important than original
development. You may be one of the lucky people who has not had to help
maintain C systems with 600 to 1000 line functions, evolved haphazardly.

I have. In such cases, there is only one solution: rewrite the
code with smaller functions.

I can assure that you such systems are very common, for you see, most
programmers are average. And the average programmer who maintains such
code has a tendency to introduce early returns that skip cleanup code
(often this is later corrected by duplicating the cleanup code in
question, which then gets out of synch, and so on and on).

The problem isn't just the programmers. "Average" programmers
can write very good code, if they are managed correctly. Good
code review, and coding guidelines, for example.

C++ code, on the other hand, must be prepared to handle exceptions at
any point, i.e. it's multiple exit at the outset.

I've noticed that in well written C++, any exceptions tend to
cluster at the top of the function, in a manner coherent with
the one frequently accepted exception: checking pre-conditions.
You don't want to exit code at just any old point.

You're IMO right that it's best to not hide exits deeply nested in
masses of other code, but C++ code must be /prepared/ to exit any point.

It depends. I tend to prefer a transactional style, which
verifies first that nothing can fail, before modifying any
actual data. (But of course, it isn't always possible.) In
practice, no throw guarantees are important for many idioms.

[...]

Are you really want it to do one of the things without doing both?

But if so, try

bool f()
{
if( pointer1 != 0 )
{
pointer1->doSomething();
if( pointer1 != 0 )
{
pointer2->doSomething1();
return true;
}

Just curious, but what invariants hold here?

}
return false;
}

"Invariants" is not really meaningful here.

Invariants are always important.

The problem here, of course, is that we have an abstract
function, without any idea as to what it really means. So we
don't know what invariants are relevant. The important point is
that the structure of the code (indenting, etc.) very strongly
suggests that the results of pointer2->doSomething1() may in
fact hold, where as it is actually guaranteed that they don't.
The indenting is a lie.

But at the point you
indicate, you can assert(pointer1!=0), so that's an invariant of sorts.
Also, at that point pointer1-doSometing() has been executed.

It's the first I was thinking of. The indentation and program
structure says one thing. (We're after the if, so the condition
of the if is no longer guaranteed.) The actual situation is
another. The indentation is lying.

Of course, the original author, when writing the code, is very
much aware of the preceding return, and the additional
guarantees it introduces. So he writes code which takes
advantage of them. The later maintenance programmer then has to
try to figure out how it can work, when the invariant on which
it depends apparently isn't guaranteed.

Which is easily (and more clearly) rewritten as:

bool
f()
{
bool succeeded = false ;
if ( pointer1 != NULL ) {
pointer1->doSomething() ;
if ( pointer2 != NULL ) {
pointer2->doSomething() ;
succeeded = true ;
}
}
return succeeded ;
}

I wouldn't call that clear, rather completely /misleading/: judging from
the original code, this function succeeds even when it does nothing.

Who knows? I just guessed at a semantic signification for the
bool. The whole function is, as you originally pointed out,
problematic. It's very hard to discuss what is reasonable in a
solution without knowing what the problem is.

But using a result variable has its charms.

Yes. It allows naming the condition. This is very important
when dealing with bool, since the name is all you really have to
go on. (It's a totally different issue, but in my experience,
it is very, very rare that a function should return bool. But
there are obvious exceptions: just about any function whose name
begins with "is", for example, or something like "contains(key)"
on a map.)

It allows allows the visual structure to correspond to the
actual control flow; code which is outside of a block controled
by an if doesn't depend on that if.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[werasm]

Alf P. Steinbach wrote:

That's a design error. Don't do it. Keep functions reasonably small.

In all honesty, I'm an advocate of short functions - "One function
one responsibility" as well. Originally I considered a similar
solution to Kanze where two seperate functions existed, each
returning boolean, but this function seemed to short to bother.
Boolean also seems the wrong return value as this function has
more than one reason for failure.

I don't have that big a problem with what you've done really,
in the context is was done. This is an unusual function as one
does not know whether pointer2 becomes valid due to action on
pointer1. Perhaps a better solution would have been to verify both
pointers prior to performing the operations on them, but one does
not know in this case whether its an error or intentional. IMO if
intentional, this should have actually been two functions as
performing these actions in one function allows state to be modified
partially, so to speak (the function does not have commit/rollback
semantics (fail or succeed)).

Stating now that I always get pre-conditions out of the way early,
would not be true. I just did not like your nesting, and in
retrospect none of the proposed solutions - my response on
yours included were sufficient. Of all I liked "return( doOne()
&& doTwo() );" the best, but a boolean return value is deceiving
in this case. This should have been two functions.

BTW. I have done maintenance for 3 years on a large, ugly 'oll
C program. I know what it feels like to work through 300 liners (with
which I don't agree - of course). If the responsibility of an
abstraction
is known, it does not make maintenance more difficult, on the
contrary.

Kind regards,

Werner

[werasm]

Alf P. Steinbach wrote:

But about the translate-bools-to-exceptions-and-back-to-bools code: as
part of being more complicated in the sense of figuring intended
functionality (versus unintended), it swallows null_ptr_error exceptions
from the two functions called, replacing with bool return.

Agreed (in retrospect). The intention of that pointer wrapper is
mostly to
initialize pointers that might be used as weak associations (I
would not want to have a lecture on shared_ptrs or scoped_ptr
as alternative). In this context it could have been used to
determine the nullness, but not necessary. I made the initial
assumption that the pointers should never be null (that it
is an error), but as you rightly stated, then we should
terminate or throw (if it was an error).

A useful place for ExistentPtr is as wrapper for a
raw pointer member - guaranteeing initialisation to
NULL. Then throwing if the pointer is used without
pointing to something. This prevents the sloppy pro-
grammer (they do exist, unfortunately) from not
initialising a pointer to NULL in the initialisation list.
The problem with scoped_ptr is that the behaviour
is undefined if it is not initialised. In practise though,
it probably does the same thing.

So I think of it as over-abstraction (also a little inefficient!); if we
want always non-null arguments, that's better expressed as references.

Yes, although a reference requires initialization at construction. We
don't
know whether in this case, references were plausible and that the re-
lationships were actually known at the time of construction. IME I
have
found that good work- arounds exist most of the time when
relationships
aren't known at the time of construction. References aren't one of
them.

Regards,

Werner

[Kai-Uwe Bux]

cp********@googlemail.com wrote:

A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it?

Sure.

What would be the advantage?

Don't know. Some people think it makes it easier to argue correctness. I am
not so sure about that, though. However, I can see the point: return
statements are like goto statements; they jump to the end of the current
routine. As with goto statements, you need to be careful.
[snip]

bool f()
{
if( !pointer1) return false;
pointer1->doSomething();

if( !pointer2) return false;
pointer2->doSomething1();

return true;
}

vs.

bool f()
{
bool retVal=true;
if( pointer1)
{
pointer1->doSomething();
}
else
retVal=false;

if( pointer2)
{
pointer2->doSomething();
}
else
retVal=false;

return retVal;
}

I am not really certain about that example. I tend to have multiple exits in
situations like this:

/*
triple is like pair, except that it has fields "first",
"second", and "third".
*/
template < typename A, typename B, typename C >
bool operator< ( triple< A, B, C const & lhs,
triple< A, B, C const & rhs ) {
// try deciding using first:
if ( lhs.first < rhs.first ) {
return ( true );
}
if ( rhs.first < lhs.first ) {
return ( false );
}
// first was tied. try second:
if ( lhs.second < rhs.second ) {
return ( true );
}
if ( rhs.second < lhs.second ) {
return ( false );
}
// first and second tied. try thrid:
if ( lhs.third < rhs.third ) {
return ( true );
}
if ( rhs.third < lhs.third ) {
return ( false );
}
// all tied:
return ( false );
}

or

template < typename ForwardIter, typename Pred >
bool forall ( ForwardIter from, ForwardIter to, Pred p ) {
// returns true iff p is not false for all *i in the range [from,to)
while ( from != to ) {
if ( ! p(*from) ) {
return ( false );
}
++ from;
}
return ( true );
}

Best

Kai-Uwe Bux

[Jim Langston]

<cp********@googlemail.comwrote in message
news:11**********************@o61g2000hsh.googlegr oups.com...

>A colleague told me that there is a rule about good stype that a
function in C++ should have only one point of return (ie. return
statement). Otherwise there might be trouble.
I never heard about it and doubt it.
Anybody heard of it? What would be the advantage?

Regards,
Marc

Example:

bool f()
{
if( !pointer1) return false;
pointer1->doSomething();

if( !pointer2) return false;
pointer2->doSomething1();

return true;
}

vs.

bool f()
{
bool retVal=true;
if( pointer1)
{
pointer1->doSomething();
}
else
retVal=false;

if( pointer2)
{
pointer2->doSomething();
}
else
retVal=false;

return retVal;
}

This was one of the fundamentals of "top down design" that came out of
trying to prevent spagetti code. The reasoning being it is much easier to
understand which code is executed and which isn't without having to look
farther up the function. It is a good idea to get used to, but I find it
better to return earlier on exceptions when it's rather obvious.

With short functions, however, it should not be a problem. I find that if
I'm checking on the validity of something (returning bool) that it is easier
to read with many return statements. Consider, which is easier to
understand at first glance?

bool CheckValue( Foo bar )
{
if ( bar.value )
return false;
if ( bar.otherval 0 )
return false;
if ( bar.value == bar.othervalue )
return false;
return true;
}

bool CheckValue( Foo bar )
{
bool ReturnVal = true;
if ( bar.value )
ReturnVal = false;
if ( bar.othervalue )
ReturnVal = false;
if ( bar.value == bar.othervalue )
ReturnVal = false;
return ReturnVal;
}
}

[Jerry Coffin]

In article <13*************@corp.supernews.com>, al***@start.no says...

* cp********@googlemail.com:

[ ... ]

Example:

bool f()
{
if( !pointer1) return false;
pointer1->doSomething();

if( !pointer2) return false;
pointer2->doSomething1();

return true;
}

Bad, because it uses global variables, and is unclear.

Are you really want it to do one of the things without doing both?

I think you're right that the usual intent is more like this:

bool f() {

if ( pointer1 == 0 || pointer2 == 0)
return false;

pointer1->doSomething();
pointer2->doSomething();
return true;
}

But if so, try

bool f()
{
if( pointer1 != 0 )
{
pointer1->doSomething();
if( pointer1 != 0 )

Presumably you meant pointer2 here.

{
pointer2->doSomething1();
return true;
}
}
return false;
}

Personally, I think I'd generally prefer something like this:

bool f() {
if (pointer1 != 0) {
pointer1->doSomething();
if (pointer2 != 0)
pointer2->doSomething();
}
return (pointer1 != 0) && (pointer2 != 0);
}

--
Later,
Jerry.

The universe is a figment of its own imagination.