directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel L├ęcharny <elecha...@apache.org>
Subject Re: Coding rules, some more things to discuss
Date Tue, 30 Aug 2011 22:12:19 GMT
On 8/30/11 9:11 PM, Stefan Seelmann wrote:
> Hi,
>
> On Tue, Aug 30, 2011 at 3:32 PM, Emmanuel Lecharny<elecharny@gmail.com>  wrote:
>> Hi guys,
>>
>> we just had a private convo with Pierre-Arnaud about coding rules. We are
>> not following exactly the same type of rules in Studio and in ADS, which is
>> quite normal. There are some reason why there is a divergence.
>>
>> I think we need to discuss a few things here.
>>
>> Currently, we have a small coding standard page :
>> https://cwiki.apache.org/confluence/display/DIRxDEV/Coding+standards
>>
>> It's pretty simple, with not a lot of rules. Both ADS and Studio are more or
>> less following those rules which were established a long time ago (there are
>> still some very old files in ADS which are not following those rules, but
>> with more than 3000 files on the project, we won't spend one month reviewing
>> all of those files one by one...)
>>
>> I'd like to add a few more rules, at least for ADS, and suggest that Studio
>> keep a slightly different sets of rules, but in any case, I'd like to see
>> all the rules added to the wiki.
>>
>> Here is what I think would be good for ADS :
>> - add a blank line before each 'if', 'for', 'do', 'switch', 'case' unless
>> the previous line is a '{'
> I have no problem with that. I guess your intention is to get a better
> overview if there are multiple of such blocks in a row. But maybe in
> such a case it makes sense to extract such a block into its own method
> and following the "Clean Code" rules.
Well, if the method is not hundreds of line long, then it's not 
necessary a good thing to create a method.

Here, it's much more about being able to distinguish a block from the 
flow at first sight. Call it an esthetic consideration...
>
>> - get rid of trinary operator ( expr ? op1 : op2 )
> I agree.
>
>> - 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") )

That's a possibility, but I don't really mind if the test is written 
like this :

if ( ( x != null )&&  x.toLowerCase().startsWith( "prefix") )

Again, the use of ( and ) is more about grouping the operations to ease the reading of such
a 'if' statement.



-- 
Regards,
Cordialement,
Emmanuel L├ęcharny
www.iktek.com


Mime
View raw message