directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pierre-Arnaud Marcelot ...@marcelot.net>
Subject Re: Coding rules, some more things to discuss
Date Wed, 31 Aug 2011 08:06:35 GMT
On 31 août 2011, at 09:43, Emmanuel Lecharny wrote:
> On 8/31/11 9:12 AM, Pierre-Arnaud Marcelot wrote:
>> On 30 août 2011, at 21:11, Stefan Seelmann wrote:
>> 
>>>> - add a blank line before each 'return'
>>>> - in if ( expr ), we should use '(' and ')' for expressions containing an
>>>> '==' or any other logical connector
>>> Same here, we should try to split up complex if conditions and extract
>>> them into self-speaking methods, for example:
>>> 
>>>    if ( x != null&&  x.toLowerCase().startsWith( "prefix") )
>>> 
>>> can be transformed into
>>> 
>>>    if ( hasPrefixCaseIgnore( x, "prefix") )
>> Big +1...
>> So much easier for code readability... :)
> 
> Well, at first, it seems a good idea. But  if you thing a bit more about it, there are
hundreds (thousands ?) of tests in the current code that use more than one operand in a 'if'.

I'm not classifying this one as an obligation, or a must do...
But it's much more pleasant to the eye and doesn't require any "mind-computation" to evaluate
the purpose of the test.

> Woudl we decide to create a method for each of those tests, it will first take a hell
of time, but more important, it has two big drawbacks IMHO :
> - first the method name must be very well chosen. It's not so easy for most of us, who
are not english speakers

True. It can be...

> - and second, it seems to be more readable, but if you have to jump to the method each
time you want to know what is exactly tested, it's quickly a PITA.

That's because the name chosen for the method is not explicit... :p haha...
Hopefully, nowadays, Eclipse really makes it easy to go back and forth in that situation.

> 
> There are some very well known situations where such approach is definitively good though.
One good example can be :
> 
> if ( Strings.isEmpty( str ) )
> {
>   ...
> }
> 
> where isEmpty does :
> 
> if ( ( str == null ) || (str.length() == 0 ) )
> 
> but IMO, this is a special case, because everyone of us would code this test the same
way, knowing that a String must not be null before being checked. (this is why Java 8 will
introduce some other way to deal with NPE in such cases)

I agree.
It shouldn't be an obligation to generalize this.
But sometimes it could be helpful... depending on the situation.

Regards,
Pierre-Arnaud


> -- 
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
> 


Mime
View raw message