On 31 août 2011, at 11:06, Kiran Ayyagari wrote: > 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
Hopefully, most of the pom.xml files are already almost following this path. I think this is somehow a natural thinking. At least for me. Even though, I admit it, I think I'm a little too maniac about ordering things in code (at home and in life too...) Maybe, I should consult someone... ;) > ( we have Ctrl + F anyway :) Haha, yeah, that's true. One big advantage I also see by the alphabetical ordering of dependencies is that it allows you to catch duplicated dependencies easily, which could then be a problem if you think you removed a dependency and it's still there because it was duplicated. I had this problem personally a few times already. >>> 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
