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 <[email protected]> > 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
