Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > Only one point of return

Reply
Thread Tools

Only one point of return

 
 
cppquester@googlemail.com
Guest
Posts: n/a
 
      08-01-2007
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;
}

 
Reply With Quote
 
 
 
 
Ian Collins
Guest
Posts: n/a
 
      08-01-2007
http://www.velocityreviews.com/forums/(E-Mail Removed) 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.
 
Reply With Quote
 
 
 
 
Alf P. Steinbach
Guest
Posts: n/a
 
      08-01-2007
* (E-Mail Removed):
> 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.

C++ code, on the other hand, must be prepared to handle exceptions at
any point, i.e. it's multiple exit at the outset. 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.



> Regards,
> Marc
>
> 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;
}
}
return false;
}


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


Urgh.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
 
Reply With Quote
 
duane hebert
Guest
Posts: n/a
 
      08-01-2007

<(E-Mail Removed)> wrote in message
news:(E-Mail Removed) 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.)


 
Reply With Quote
 
duane hebert
Guest
Posts: n/a
 
      08-01-2007

"duane hebert" <(E-Mail Removed)> wrote in message
news:9p%ri.51852$(E-Mail Removed)...
> 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)


 
Reply With Quote
 
werasm
Guest
Posts: n/a
 
      08-01-2007

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<Type> p1( pointer1 );
ExistentPtr<Type> p2( 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

 
Reply With Quote
 
Alf P. Steinbach
Guest
Posts: n/a
 
      08-01-2007
* 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).


Both versions are SEME. Yours is less clear because you can't tell at a
glance under which conditions it will return true. You have to
laboriously analyse the complete code in order to establish that.


> I like getting the pre-conditions out of
> the way before handling the meat.


That's always necessary for precondition checking. Here we have no
function level preconditions. But if we had, then checking them after
they apply would not be a matter of like or dislike, it would simply be
incorrect with possible undefined behavior.


> SEME for me seems to do this better than SESE.


Often yes. Both versions above are SEME.


> 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<Type> p1( pointer1 );
> ExistentPtr<Type> p2( pointer2 );
>
> try
> {
> p1->doSomething();
> p2->doSomething();
> return true;
> }
> catch( const null_ptr_error& )
> {
> return false;//
> }
> }


Is both needlessly inefficient and unclear. Are the conditions under
which p2->doSomething() is executed, intentional or an arbitrary side
effect of using ExistenPtr? Impossible to tell, and that's a nightmare
for the one maintaining the code, who must then check all /calling/ code
for expectations about f's behavior (or lack of behavior).


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


What makes you think the original example was handling any errors? It
looked like normal case code to me. For errors, terminate or throw.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
 
Reply With Quote
 
werasm
Guest
Posts: n/a
 
      08-01-2007

Alf P. Steinbach wrote:


> Both versions are SEME. Yours is less clear because you can't tell at a
> glance under which conditions it will return true. You have to
> laboriously analyse the complete code in order to establish that.


Yes, true (both SEME), but I don't agree on the clarity. I prefer
(that is probably subjective) completing the code that may cause the
rest not to execute (as in the example), where in your case
your if nesting becomes deeper (and for slightly longer functions
very deep quite soon).

> That's always necessary for precondition checking. Here we have no
> function level preconditions.


Well yes, perhaps they cannot be considered function level pre-
conditions,
but they certainly are pre-conditions to the code that follow, and
prevent
the function from continuing if they are not met.

> Often yes. Both versions above are SEME.


Yes, no arguing there, true.

> > 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<Type> p1( pointer1 );
> > ExistentPtr<Type> p2( pointer2 );
> >
> > try
> > {
> > p1->doSomething();
> > p2->doSomething();
> > return true;
> > }
> > catch( const null_ptr_error& )
> > {
> > return false;//
> > }
> > }

>
> Is both needlessly inefficient and unclear. Are the conditions under
> which p2->doSomething() is executed, intentional or an arbitrary side
> effect of using ExistenPtr?


In the example the condition under which p2->doSomething() is executed
is always a function of p1->doSomething completing successfully and
of p2's existence. The pointer wrapper should make the second
pre-condition clear. Of course, p1->doSomething()'s successful
completion is a function of p1's existence, which is naturally a
pre-condition to p2.

> Impossible to tell, and that's a nightmare
> for the one maintaining the code, who must then check all /calling/ code
> for expectations about f's behavior (or lack of behavior).


Not if ExistentPtr's behavior is known to throw null_ptr_error if the
pointer it wraps temporarily does not exist.

> What makes you think the original example was handling any errors? It
> looked like normal case code to me. For errors, terminate or throw.


Yes, there is truth in this. The fact that the weak associations did
not exist
may imply that that part of functionality does not apply. I may have
considered it erroneous prematurely. If it was handling an error, then
code like
this would have made sense to me:

ExistentPtr<Type> p1( pointer1 );
ExistentPtr<Type> p2( pointer2 );

p1->doSomething();
p2->doSomething();

Regards,

Werner

 
Reply With Quote
 
Alf P. Steinbach
Guest
Posts: n/a
 
      08-01-2007
* werasm:
> Alf P. Steinbach wrote:
>
>
>> Both versions are SEME. Yours is less clear because you can't tell at a
>> glance under which conditions it will return true. You have to
>> laboriously analyse the complete code in order to establish that.

>
> Yes, true (both SEME), but I don't agree on the clarity. I prefer
> (that is probably subjective) completing the code that may cause the
> rest not to execute (as in the example), where in your case
> your if nesting becomes deeper (and for slightly longer functions
> very deep quite soon).


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

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
 
Reply With Quote
 
James Kanze
Guest
Posts: n/a
 
      08-02-2007
On Aug 1, 11:09 am, Ian Collins <(E-Mail Removed)> wrote:
> (E-Mail Removed) 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:(E-Mail Removed)
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Share-Point-2010 ,Share-Point -2010 Training , Share-point-2010Hyderabad , Share-point-2010 Institute Saraswati lakki ASP .Net 0 01-06-2012 06:39 AM
What is the point of having 16 bit colour if a computer monitor can only display 8 bit colour? How do you edit 16 bit colour when you can only see 8 bit? Scotius Digital Photography 6 07-13-2010 03:33 AM
IP Route Tables - Point to Point Connection - Only Routing 1 way SallyBridges Cisco 3 12-06-2007 06:06 PM
Scenario 5: IS-IS routing on Frame Relay Multi-point and Point-to-Point David Sudjiman Cisco 0 06-08-2006 09:11 AM
what value does lack of return or empty "return;" return Greenhorn C Programming 15 03-06-2005 08:19 PM



Advertisments