directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Seelmann <seelm...@apache.org>
Subject Re: Coding rules, some more things to discuss
Date Tue, 30 Aug 2011 19:11:03 GMT
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.

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


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

Mime
View raw message