Too many return-statements = bad style?

[Marco Aschwanden]

Hi

I checked the other day a module of mine with pylint. And for some
function it told me, that I was using too many return-statements in my
code... there were about 5 or 7 of them in my function.

Through books I learned, that there should be only 1 return statement in a
function. This makes for clear code - they said. I tried to stick to this
principle for a very long time... ending up with deeper and deeper nested
if-then-else-clauses and hided code-sections with clever if-then-clauses
when the return value was already clear or when the code was not eligible
for this specific return value:

# Example 1: "Good" style according to the books
def func(value):
return_value = xyz
IF not wrong value:
do something does change return_value
IF do soemthing worked out:
do something else...

IF not return_value == xyz:
do something other with return_value

return return_value

Meanwhile I have adopted a style that jumps out of the function when some
condition is reached. I don't have to bother afterwards with this case. In
my eyes, this style produces more readable code:

# Example 2: "Bad" style
def func(value):
return_value = xyz
IF wrong_value:
return wrong_value

do something does change return_value
IF not do soemthing worked out:
return another_wrong_value

do something other with return_value

return return_value

This pylint made me feel guilty again... so I wonder: How do you handle
this? Is "my" style really bad style?

Thanks for your hints in advance,
Greetings,
Marco

 

[=?iso-8859-15?Q?Pierre-Fr=E9d=E9ric_Caillaud?=]

I do it just like your style.
I tend to put tests at the beginning of the function and evacuate all
error cases with returns or throwing exceptions.


This is readable :

if not something_to_do:
return
else:
big blob of code

Or even :
if not something_to_do:
return

big blob of code

This can get you lost if the big blob of code is large so the if is
offscreen :
if something_to_do:
big blob of code
else:
return

Basically I often arrange my if's so the 1-2 lines block is just below
the if and the 20 lines block is after the else...

 

[Peter Hansen]

I do it just like your style.

So do I. In addition, though, if the resulting code is still hard to
read because there is too much between the initial "exceptional"
returns, and the final return after the else, I would consider
refactoring so that some of the working code is in a subroutine...
Then the original function reads much more clearly as what it is:
a bunch of tests, many of which result in premature failure, and
then some work and a final return statement.

-Peter

 

[Rgemini]

So do I. In addition, though, if the resulting code is still hard to
read because there is too much between the initial "exceptional"
returns, and the final return after the else, I would consider
refactoring so that some of the working code is in a subroutine...
Then the original function reads much more clearly as what it is:
a bunch of tests, many of which result in premature failure, and
then some work and a final return statement.

-Peter

I do it that way too. But I put exit comments next to the returns so they
stand out more. Something like

return foo ##### exit because foo = bar #####

Rgemini

 

[Peter Hansen]

I do it that way too. But I put exit comments next to the returns so they
stand out more. Something like

return foo ##### exit because foo = bar #####

Wouldn't that be redundant if the code read this way?

if foo == bar:
return foo

-Peter

 

[Roy Smith]

Through books I learned, that there should be only 1 return statement in a
function. This makes for clear code - they said. I tried to stick to this
principle for a very long time... ending up with deeper and deeper nested
if-then-else-clauses and hided code-sections with clever if-then-clauses
when the return value was already clear or when the code was not eligible
for this specific return value:

You're talking about "Single Entry, Single Exit". It was a style of
coding that used to be popular. I hate it, for exactly the reasons you
state above. http://c2.com/cgi/wiki?SingleFunctionExitPoint has some
interesting things to say on that.

I even go so far as to intentionally write multiple-exit functions
during refactoring because they are often a neat way to clean up messy
logic. Let's say I've got:

for i in list:
if blah:
if i == 4:
foo = 1
else:
foo = 2
else:
foo = 3

that's a mess to read (especially if it's further embedded inside some
other complex logic). What I'll do is force that little bit of uglyness
down into a multi-return function:

def getFoo (i):
if blah:
if i == 4:
return 1
else:
return 2
return 3

and then just write in my main-line code:

for i in list:
foo = getFoo (i)

 

[Marco Aschwanden]

You're talking about "Single Entry, Single Exit". It was a style of

coding that used to be popular. I hate it, for exactly the reasons you
state above. http://c2.com/cgi/wiki?SingleFunctionExitPoint has some
interesting things to say on that.

Excellent link! This answers actually my question in very thorough way.

Thanks,
Marco

 

[Michael Geary]

def getFoo (i):
if blah:
if i == 4:
return 1
else:
return 2
return 3

That still has some unnecessary nesting. Since you're returning 3
whenever blah is false, there's no need to nest the other if statement
inside the "if blah". Simply reverse the test.

Also, I generally don't like "if...return...else...return" constructs
because the "else" is redundant.

Here is the same logic in a form that I find much easier to follow:

def getFoo (i):
if not blah: return 3
if i == 4: return 1
return 2

Or, you could write it this way (to me this is less readable, but still
better than the original):

def getFoo (i):
if not blah:
return 3
if i == 4:
return 1
return 2

-Mike

 

[george young]

That still has some unnecessary nesting. Since you're returning 3
whenever blah is false, there's no need to nest the other if statement
inside the "if blah". Simply reverse the test.

Also, I generally don't like "if...return...else...return" constructs
because the "else" is redundant.

Here is the same logic in a form that I find much easier to follow:

def getFoo (i):
if not blah: return 3
if i == 4: return 1
return 2

Hmm... contrariwise, I find:

def getFoo(i):
if not blah: return 3
elif i == 4: return 1
else: return 2

more compelling, the if--elif--else emphasizing that these are three
possible outcomes; the last return is in the same logical level as
the other returns, so it seems right that it be in a similar syntactic
position.
[I don't care if the 'return' is on the same or separate line, sometimes
I use one for brevity, sometimes the other for clarity]

-- George Young

 

[Andrew Koenig]

Hmm... contrariwise, I find:

def getFoo(i):
if not blah: return 3
elif i == 4: return 1
else: return 2

more compelling, the if--elif--else emphasizing that these are three
possible outcomes; the last return is in the same logical level as
the other returns, so it seems right that it be in a similar syntactic
position.

If I were going to bother to write the redundant "else"s, I would also be
inclined to give the code a single exit point:

def getFoo(i):
if not blah:
result = 3
elif i == 4:
result = 1
else:
result = 2
return result

That way, if I want to come back later and print the result before returning
(for debugging purposes), I can do so by inserting a single line of code.

Of course, this whole argument is pretty silly without a more plausible
context