Intention...

According to most structured programming standards and theories, a method or function should have just one return point at the end of the code (I could expose a huge number of reasons if someone doesn't see why this is the right way to do it).

Because of this, the latest intention added, though the code it generates is better than the original one, is not the right way to do it.

Instead of replacing:
ifelse with ifsomething

I think it should be:
if{return xxx}else{[return yyy]}
with
if{returnVar=xxx}else{[returnVar=yyy]} return returnVar;

Well, this is just my opinion anyway ;)

18 comments

Luis Estrada wrote:

According to most structured programming standards and theories, a method or function should have just one return point at the end of the code (I could expose a huge number of reasons if someone doesn't see why this is the right way to do it).

Instead of replacing:
ifelse > with > ifsomething


The opinion on this is not unanimous. Many well-known programmers
(specifically Kent Beck) recommend this pattern. It's called a Guard
Clause (see http://c2.com/cgi/wiki?GuardClause). I use them all the
time. It makes code much more legible and understandable in my opinion;
it's a pragmatic decision. I can often cut complicated methods down to a
few simple guard clauses and a short, simple main-flow. This is more
valuable to me than any theoretical 'purity'.

I've only encountered one problem with guard clauses, and that is that
they are sometimes tricky to perform Extract Method on. However, in
practice, they are much easier to extract method on than if the code was
a complicated tangle of conditionals.

--
Rob Harwood
Software Developer
JetBrains Inc.
http://www.jetbrains.com
"Develop with pleasure!"

0

Rob Harwood (JetBrains) wrote:

>

Many well-known programmers (specifically Kent Beck) recommend this
pattern. It's called a Guard Clause



I'm not well known, but I use it all the time too.

I've only encountered one problem with guard clauses, and that is that
they are sometimes tricky to perform Extract Method on.



Inlining them is also a problem:

IDEA can't inline a method that ends with

if (cond)
return 1;
return 2;


, but it works fine if you rewrite it to

if (cond)
return 1;
else
return 2;


As I use guard clauses a lot, I - too - often have to repeat the
sequence above, and
- remove the guard clauses
- ExtractMethod
- restore the guard clauses,
and the same for inlining.

JetBrains, should spend some time on polishing the basics, like extract
method and inline.

Alain

0

Rob, you are absolutely right, but i might not have been clear in what i mean.

Guard clauses are great and really make the code a lot more legible, but you may use them with just one return point too. I didn't mean to say the intention should be changed for guard clauses but for general use.

Also as I said you can use guard clauses and have just one return point, this code is from the page you refer to.

public Foo merge (Foo a, Foo b) {
Foo result;
if (a == null) result = b;
else if (b == null) result = a;
else
{
result = // complicated merge code goes here.
}


return result;
}

Although in the case of a Guard Clause it wouldn't matter to have one or many return points in most cases you shouldn't have more than one. Mainly because you may modify some code thinking it will execute and have a return point somewhere above (as you see this doesn't apply for Guard Clauses as you won't want to execute the modified code anyway).

Btw, thanks for the reply, made me think of a good way of using the intention.

0

Luis Estrada wrote:

public Foo merge (Foo a, Foo b) {
Foo result;
if (a == null) result = b;
else if (b == null) result = a;
else
{
result = // complicated merge code goes here.
}
return result;
}


Personally, I much prefer this:

public Foo merge (Foo a, Foo b) {
if (a == null) return b;
if (b == null) return a;
return // complicated merge code goes here.
}

The reason is for the same reason I prefer Java 5's new for loop: Less
boiler-plate code, much easier to read and understand. Also some very
minor refactoring benefits: If I change merge()'s return type, I don't
also have to change result's type.

However, I can understand your point if someone has to work with a
standard of single return points. That's fine, but I don't think IDEA
should force people to use that standard when there are other valid ways
of programming that have their own benefits.

--
Rob Harwood
Software Developer
JetBrains Inc.
http://www.jetbrains.com
"Develop with pleasure!"

0

Well, i agree there are benefits in both codes (in my way you can add code before the return that will always execute, even if the clauses match, while you'd have to change the clauses in the multi-return way).

I also agree that the new for loop is way better, actually I think most of the code syntax inherited from C is messy and poorly structured (most people use the old for loop as a while loop)

About the intention we are arguing about, if you use the a one return point standard you should never have to use this intention, so I think it's alright to have it if it fits your needs.

Nice writing with you, I really had fun with this topic.

0


I use gaurd clauses all the time as well (InspectionGadgets basically consists of a bunch of parse tree visitors with nothing but gaurd clauses at each node), and would not use as "enforce single return point" refactoring. What I would like is improved handling of gaurd clauses. In particular, extending "Extract Method" to handle gaurd clauses (http://intellij.net/tracker/idea/viewSCR?publicId=37823) and the obvious converse improvement for "Inline Method" would rule.

--Dave Griffith

0

As you already have said, unfortunately IDEA does not support inlining
methods with multiple return statements. Independent of this, I fully
agree, that

if (cond) {
return 1;
}
return 2;


is better to read than

if (cond) {
return 1;
}
else {
return 2;
}

To make the return (or continue or break) statement more clear, we add a
newline, too:

if (cond) {
return 1;
}

return 2;

Tom

0

Personally, I much prefer this:

public Foo merge (Foo a, Foo b) {
if (a == null) return b;
if (b == null) return a;
return // complicated merge code goes here.
}

The reason is for the same reason I prefer Java 5's new for loop: Less
boiler-plate code, much easier to read and understand.


Same for me. Unfortunately IDEA is a little bit dumb, when inlining such
a method.

Tom

0

Dave Griffith wrote:

I use gaurd clauses all the time as well (InspectionGadgets basically consists of a bunch of parse tree visitors with nothing but gaurd clauses at each node), and would not use as "enforce single return point" refactoring. What I would like is improved handling of gaurd clauses. In particular, extending "Extract Method" to handle gaurd clauses (http://intellij.net/tracker/idea/viewSCR?publicId=37823) and the obvious converse improvement for "Inline Method" would rule.

--Dave Griffith

Is there a request for this (Inlining)? I think it would be great too,
and would gladly add some votes.

--
Rob Harwood
Software Developer
JetBrains Inc.
http://www.jetbrains.com
"Develop with pleasure!"

0

Personally, I much prefer this:

public Foo merge (Foo a, Foo b) {
if (a == null) return b;
if (b == null) return a;
return // complicated merge code goes here.
}


This is all nice if your method is 3-10 lines long. but as soon as your guard clause get scattered... which they will after 2 or 3 generations of coders have their way with your code... guard clauses go bad. I have seen methods where guard clauses have gotten seperated by dozens of lines of code... at that point all the arguments for them go out the window, IMO!

And if you argue that the guard clause should always be up top near the beginning of the method... then I would argue that something not very efficient is happening in your code if you are often entering methods to leave them right away.

just adding my 2 cents :)
Florian

0


Dunno. I thought Alain submitted one a while ago (not that that's any help in narrowing it down) but I can't find it.

It's also somewhat related to an old request of mine (http://intellij.net/tracker/idea/viewSCR?publicId=13383 ), which has only been partially addressed.

--Dave Griffith

0

Florian Hehlen wrote:

This is all nice if your method is 3-10 lines long. but as soon as your guard clause get scattered... which they will after 2 or 3 generations of coders have their way with your code... guard clauses go bad. I have seen methods where guard clauses have gotten seperated by dozens of lines of code... at that point all the arguments for them go out the window, IMO!

And if you argue that the guard clause should always be up top near the beginning of the method... then I would argue that something not very efficient is happening in your code if you are often entering methods to leave them right away.

All guard clauses should go at the top. ;)

I don't agree with the efficiency argument because the point of a Guard
Clause is not efficiency, but conceptual clarity. I doubt your code has
no guard clauses or the equivalent conditionals. It's a nearly universal
pattern to check some 'abnormal' condition before proceeding with the
'normal' case.

Anyway, this has all been debated on the C2 Wiki, so you can find the
pros and cons there.

--
Rob Harwood
Software Developer
JetBrains Inc.
http://www.jetbrains.com
"Develop with pleasure!"

0

Florian Hehlen wrote:

This is all nice if your method is 3-10 lines long. but as soon as your guard clause get scattered... which they will after 2 or 3 generations of coders have their way with your code... guard clauses go bad. I have seen methods where guard clauses have gotten seperated by dozens of lines of code... at that point all the arguments for them go out the window, IMO!


And the "only one return" style will suffer from the same generational
decay. IMHO, it would decay even worse/faster since it's harder to
understand than the guard-clause style.

Besides, if you're still working on the code, you can clean up the
scatter (you do read summaries of all checkins to code for which you're
responsible, right?). And if you're not responsible for the code, what
do you care? ;)

Ciao,
Gordon

--
Gordon Tyler (Software Developer)
Quest Software <http://www.quest.com/>
260 King Street East, Toronto, Ontario M5A 4L5, Canada
Voice: (416) 933-5046 | Fax: (416) 933-5001

0

I prefer much shorter version:

return cond ? value1 : value2;

"Thomas Singer (MoTJ)" <nomail@nodomain.com> wrote in message
news:cre9o7$34v$1@is.intellij.net...

As you already have said, unfortunately IDEA does not support inlining
methods with multiple return statements. Independent of this, I fully
agree, that

>

if (cond) {
return 1;
}
return 2;

>
>

is better to read than

>

if (cond) {
return 1;
}
else {
return 2;
}

>

To make the return (or continue or break) statement more clear, we add a
newline, too:

>

if (cond) {
return 1;
}

>

return 2;

>

Tom



0


"Gordon Tyler" <gordon.tyler@quest.com> wrote in message
news:crh523$91m$1@is.intellij.net...

Florian Hehlen wrote:

This is all nice if your method is 3-10 lines long. but as soon as your

guard clause get scattered... which they will after 2 or 3 generations of
coders have their way with your code... guard clauses go bad. I have seen
methods where guard clauses have gotten seperated by dozens of lines of
code... at that point all the arguments for them go out the window, IMO!
>

And the "only one return" style will suffer from the same generational
decay. IMHO, it would decay even worse/faster since it's harder to
understand than the guard-clause style.


One return is a Pascal style, bringing to the table a lot of conditions and
indentations (which are limited in some shops). Multiple returns is C style
and is much easier to read and understand. Too bad that some shops require
both single return and no more than say 5 levels of indentation.


0

Michael Jouravlev wrote:

>I prefer much shorter version:
>
>return cond ? value1 : value2;
>

>

I like my shorter versions to take more space, for clarity :


return someVeryLongExpressionContion
? result1
: result2 ;



Alain

0

I prefer much shorter version:

return cond ? value1 : value2;


As you might have noticed, these were just small-enough examples. Typically
there are more commands before each return statement.

Tom

0

You wrote

... guard clauses go bad.


and

Multiple returns
is much easier to read and understand.


Isn't that a contradiction in itself?

Tom

0

Please sign in to leave a comment.