Code guidelines for checks ?

Thread view:

nicolas.raoul@gmail.com - 12 Jul 2007 17:34 GMT

Hello all,

I am defining the guidelines for the readability of a Java project,
and I am hesitating between two ways of implementing imbricated if-
else in the case of checking conditions before doing something.

Let's say I have to check two conditions before doing something.
Here are the two kinds of implementations I have seen around in open
source projects:

=============== Implementation 1 ===============

if ( foo != null ) {
if ( bar != null ) {
doSomething ( foo, bar );
} else {
throw new NullBarException();
}
} else {
throw new NullFooException();
}

=============== Implementation 2 ===============

if ( foo == null ) {
throw new NullFooException();
}

if ( bar == null ) {
throw new NullBarException();
}

doSomething ( foo, bar );

================================================

Some drawbacks of Implementation 1:
- As the number of checks grows, the indentation becomes a problem.
- It is harder for the reader to see the cause-exception couples.

Some drawbacks of Implementation 2:
- Three blocks instead of one, so it does not look as a code entity.

Which implementation do you think is better for readability and
maintenance ?
Cheers,
Nicolas (http://nrw.free.fr)

Reply to this Message

Flo 'Irian' Schaetz - 12 Jul 2007 17:40 GMT

And thus spoke nicolas.raoul@gmail.com...

> Some drawbacks of Implementation 1:
> - As the number of checks grows, the indentation becomes a problem.
[quoted text clipped - 5 lines]
> Which implementation do you think is better for readability and
> maintenance ?

Personally, I always use the 2nd way. It's much more easy to read,
maintain, etc. - in my (not so) humble opinion, of course. I like the
advantage of first being able to check for all possible errors (null
pointers, etc.) and then proceed with the really important code...

Flo

Reply to this Message

Thomas Fritsch - 12 Jul 2007 21:54 GMT

> =============== Implementation 1 ===============
>
[quoted text clipped - 31 lines]
> Which implementation do you think is better for readability and
> maintenance ?

My vote is for Implementation 2.
I totally agree with your drawbacks of Impl 1.
I don't quite understand what you describe as the drawback of Impl 2.
The logic (both in Impl 2 and in Impl 1) is a sequence of 3 steps (two
sanity checks, and one real work-step). So why want it to appear as one?

Signature

Thomas

Reply to this Message

nicolas.raoul@gmail.com - 13 Jul 2007 11:26 GMT

Thanks for your answers :-)

I have been thinking about a serious drawback for Implementation 2:
let's say a failed condition does not lead to an exception but makes
the method return an error code. Then Implementation 2 would contain
three "return" keywords, and rumor has it that it is cleaner to have
as few "return"s as possible. Which leads me to:

=============== Implementation 3 ===============

if ( foo == null ) {
throw new NullFooException();

} else if ( bar == null ) {
throw new NullBarException();

} else {
doSomething ( foo, bar );
}

================================================

Implementation 3 is not as pretty as Implementation 2, but it makes it
possible to set a return code and only return it at the end of the
method.
A typical example can be found in the WIMPI project (seems to be
developed by Google) : http://www.koders.com/java/fid57D6AD8F77D867C8A96F08B7F88476BDA0AAF30F.aspx?s=pa
rameter+view

Meanwhile, I have found a typical example of my scenario in method
getCommand of the Spring MVC:
http://springframework.cvs.sourceforge.net/springframework/spring/src/org/spring
framework/web/servlet/mvc/AbstractFormController.java?revision=1.44&view=markup
I think that the Spring project is a modern project with strong
guidelines, so I would tend to think they kind of set up an example.
In that method, the typical scenario of checking for conditions before
doing something, they use Implementation 2.

If anyone favors Implementation 1, I would like to hear from you !

Cheers,
Nicolas RAOUL.

Thomas: You're right, it actually makes more sense to see it as
different steps rather than one big step.

Reply to this Message

Lew - 13 Jul 2007 13:50 GMT

> Thanks for your answers :-)
>
[quoted text clipped - 3 lines]
> three "return" keywords, and rumor has it that it is cleaner to have
> as few "return"s as possible. Which leads me to:

Vicious rumor. Not true.

> =============== Implementation 3 ===============
>
[quoted text clipped - 11 lines]
>
> Implementation 3 is not as pretty as Implementation 2, but it makes it

It's identical to number 2 if you drop the "else" keywords.

But you beg the question of whether you should throw an exception for those
conditions. It's not always the Right Thing to do.

> possible to set a return code and only return it at the end of the
> method.

That isn't what you illustrated. You illustrated not setting a return code,
thus avoiding all the messily deep indentation that your idea would necessitate.

Readability and maintainability are not always enhanced by some of the
anal-retentive conventions people suggest. The "single-return" philosophy is
in this category.

Signature

Lew

Reply to this Message

Roedy Green - 13 Jul 2007 17:47 GMT

>if ( foo == null ) {
> throw new NullFooException();
[quoted text clipped - 5 lines]
>
>doSomething ( foo, bar );

I hate nesting, so I prefer this style, though I suspect I will be in
the minority.

Nesting his a sort of procrastination, where you have keep in mind all
the things you plan to do in future.
--
Roedy Green Canadian Mind Products
The Java Glossary
http://mindprod.com

Reply to this Message

Code guidelines for checks ?

Free Magazines