On 8/30/11 9:11 PM, Stefan Seelmann wrote:
Hi,
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.
Well, if the method is not hundreds of line long, then it's not
necessary a good thing to create a method.
Here, it's much more about being able to distinguish a block from the
flow at first sight. Call it an esthetic consideration...
- 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") )
That's a possibility, but I don't really mind if the test is written
like this :
if ( ( x != null )&& x.toLowerCase().startsWith( "prefix") )
Again, the use of ( and ) is more about grouping the operations to ease the
reading of such a 'if' statement.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
