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 07:12:39 GMT
On 30 août 2011, at 21:11, Stefan Seelmann wrote:

> Hi,

Hi Stefan,

> 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.
> 
>> - 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") )

Big +1...
So much easier for code readability... :)

>> We also may want to add some rules for pom.xml. Typically, what I'd like to
>> see is a blank line between each element like <dependency>. Here is an
>> example :
>> 
>> <dependencies>
>> <!-- Apache Directory Studio library plugins dependencies -->
>> <dependency>
>> <groupId>org.apache.directory.shared</groupId>
>> <artifactId>shared-ldap-model</artifactId>
>> <scope>provided</scope>
>> </dependency>
>> 
>> <dependency>
>> <groupId>org.apache.directory.shared</groupId>
>> <artifactId>shared-util</artifactId>
>> <scope>provided</scope>
>> </dependency>
>> 
>> This is to separate all the items which have the same dame, for clarity
>> sake.
>> 
>> For Studio, I let Stefan and Pierre-Arnaud define the rules they prefer to
>> use, as i'm not working often on its code.
> 
> Me neither ;-), but I don't see a reason why not to use the same rules
> for all projects.

+1

Regards,
Pierre-Arnaud




Mime
View raw message