Mailing-List: contact dev-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Apache Directory Developers List" <dev@directory.apache.org>
Received-SPF: pass (nike.apache.org: domain of pajbam@gmail.com designates
 209.85.215.170 as permitted sender)
Sender: Pierre-Arnaud Marcelot <pajbam@gmail.com>
Content-Type: text/plain; charset=iso-8859-1
Mime-Version: 1.0 (Apple Message framework v1084)
Subject: Re: Coding rules, some more things to discuss
From: Pierre-Arnaud Marcelot <pa@marcelot.net>
In-Reply-To: 
 <CABzFU-cbPMAs0EF_11L_8EGt2TUEgcaBx2OPp+m4LX-xsKWCXw@mail.gmail.com>
Date: Wed, 31 Aug 2011 11:48:15 +0200
Content-Transfer-Encoding: quoted-printable
Message-Id: <6BC81C55-7B42-4BBB-B65A-BB6D9CCF5A67@marcelot.net>
References: <4E5CE665.2030008@gmail.com>
 <07AF0E26-47A5-4801-9050-6AB00992954A@marcelot.net>
 <CABzFU-cbPMAs0EF_11L_8EGt2TUEgcaBx2OPp+m4LX-xsKWCXw@mail.gmail.com>
To: "Apache Directory Developers List" <dev@directory.apache.org>


On 31 ao=FBt 2011, at 11:06, Kiran Ayyagari wrote:

> On Wed, Aug 31, 2011 at 12:38 PM, Pierre-Arnaud Marcelot
> <pa@marcelot.net> wrote:
>> On 30 ao=FBt 2011, at 15:32, Emmanuel Lecharny wrote:
>>=20
>>> Hi guys,
>>=20
>> Hi,
>>=20
>>> 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.
>>=20
>> 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).
>>=20
>>> I think we need to discuss a few things here.
>>>=20
>>> Currently, we have a small coding standard page : =
https://cwiki.apache.org/confluence/display/DIRxDEV/Coding+standards
>>>=20
>>> 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...)
>>=20
>> Same thing for Studio.
>> Some pretty old files may not be following *all* the rules.
>>=20
>>> 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.
>>>=20
>>> 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 '{'
>>=20
>> 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 =3D anotherVariable.getVariable();
>>>> if ( variable.hasFlag() )
>>>> {
>>>> [...]
>>=20
>> In that particular case, IMO, it helps grouping expressions for a =
better readability.
>>=20
>>> - get rid of trinary operator ( expr ? op1 : op2 )
>>=20
>> I would prefer keeping it as it's very handy for variable nullity =
checks.
>>=20
>> Here's an example:
>>>> return ( ( variable =3D=3D null ) ? "null" : variable );
>>=20
>>=20
>> I prefer the compact format instead of this:
>>>> if ( variable =3D=3D null )
>>>> {
>>>>     return "null";
>>>> }
>>>> else
>>>> {
>>>>     return variable;
>>>> }
>>=20
>> Now, if I'm the only one liking it, I will refrain myself from using =
it in the future... ;)
>>=20
>>> - add a blank line before each 'return'
>>=20
>> +1
>>=20
>>> - in if ( expr ), we should use '(' and ')' for expressions =
containing an '=3D=3D' or any other logical connector
>>=20
>> +1
>>=20
>>> We also may want to add some rules for pom.xml.
>>=20
>> +1
>> Even though I think we already share the same rules, having them =
written is always a plus. Especially for newcomers.
>>=20
>>> Typically, what I'd like to see is a blank line between each element =
like <dependency>. Here is an example :
>>>=20
>>> <dependencies>
>>> <!-- Apache Directory Studio library plugins dependencies -->
>>> <dependency>
>>> <groupId>org.apache.directory.shared</groupId>
>>> <artifactId>shared-ldap-model</artifactId>
>>> <scope>provided</scope>
>>> </dependency>
>>>=20
>>> <dependency>
>>> <groupId>org.apache.directory.shared</groupId>
>>> <artifactId>shared-util</artifactId>
>>> <scope>provided</scope>
>>> </dependency>
>>>=20
>>> This is to separate all the items which have the same dame, for =
clarity sake.
>>=20
>> 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.
>>=20
>> 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.
>>=20
> 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.
>>=20
>> 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... ;) ).
>>=20
>> Regards,
>> Pierre-Arnaud
>>=20
>>=20
>>> Any comments ?
>>>=20
>>> --
>>> Regards,
>>> Cordialement,
>>> Emmanuel L=E9charny
>>> www.iktek.com
>>>=20
>>=20
>>=20
>=20
>=20
>=20
> --=20
> Kiran Ayyagari