On Wed, Aug 31, 2011 at 12:38 PM, Pierre-Arnaud Marcelot <[email protected]> wrote: > On 30 août 2011, at 15:32, Emmanuel Lecharny wrote: > >> Hi guys, > > Hi, > >> 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. > > As we've seen in our private convo with Emmanuel, the divergence is very very > subtle and it's mostly a divergence on "unwritten" rules that we can't find > in our coding standards documentation... > The rest of rules are clearly well followed (except some very very old parts > of code that haven't been touch for years now). > >> 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...) > > Same thing for Studio. > Some pretty old files may not be following *all* the rules. > >> 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 '{' > > In most cases I agree, but I find some cases where I prefer leaving the if > close to the previous expression. > Especially in cases where I get a variable and I want to test something on it > just after. > Here's an example: >>> // Testing variable >>> SomeType variable = anotherVariable.getVariable(); >>> if ( variable.hasFlag() ) >>> { >>> [...] > > In that particular case, IMO, it helps grouping expressions for a better > readability. > >> - get rid of trinary operator ( expr ? op1 : op2 ) > > I would prefer keeping it as it's very handy for variable nullity checks. > > Here's an example: >>> return ( ( variable == null ) ? "null" : variable ); > > > I prefer the compact format instead of this: >>> if ( variable == null ) >>> { >>> return "null"; >>> } >>> else >>> { >>> return variable; >>> } > > Now, if I'm the only one liking it, I will refrain myself from using it in > the future... ;) > >> - add a blank line before each 'return' > > +1 > >> - in if ( expr ), we should use '(' and ')' for expressions containing an >> '==' or any other logical connector > > +1 > >> We also may want to add some rules for pom.xml. > > +1 > Even though I think we already share the same rules, having them written is > always a plus. Especially for newcomers. > >> 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. > > Why not. > I liked the idea of grouping a set of dependencies under a common "label" > like this "Apache Directory Studio library plugins dependencies" in your > example. > But adding a blank line doesn't really break either... > So, +1. > > One more thing I'd like to add to pom.xml guidelines, I really like when > dependencies are ordered in alphabetical order. > In Studio, we deal with a lot of dependencies for each project (mostly > Eclipse dependencies + a few others) and having them ordered REALLY helps > when looking for something, IMO. > won't this be a bit laborious to order the dependencies by name ( we have Ctrl + F anyway :) >> For Studio, I let Stefan and Pierre-Arnaud define the rules they prefer to >> use, as i'm not working often on its code. > > For the sake of a better interaction and simplicity, I think we should share > the same rules across the whole Directory project. > As I'm mostly the only dissident on some of the facts above, I can and will > adapt myself. > Not a big deal (except for the trinary operator... ;) ). > > Regards, > Pierre-Arnaud > > >> Any comments ? >> >> -- >> Regards, >> Cordialement, >> Emmanuel Lécharny >> www.iktek.com >> > >
-- Kiran Ayyagari
