You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Stefan Seelmann <se...@apache.org> on 2010/10/04 10:11:56 UTC
Checkstyle questions: inline conditionals, protected fields, Javadoc
for private members
Hi dev,
we are trying to fix remaining checkstyle errors in shared [1] and
have some questions:
1. Inline Conditionals
We have 151 inline conditionals, should we get rid of them or should
we allow them?
IMO 'simple' inline conditionals are ok:
return oid == null ? "" : oid;
Such constructs could be simplified
return ( ( byteArray[index] == car ) ? true : false );
to
return byteArray[index] == car;
However nested inline conditionals are hard to read and should be avoided:
return ( val < 0 ? -1 : ( val > 0 ? 1 : val ) );
So what is our policy regarding inline conditionals? With checkstyle
we can't configure that simple inline conditionals are allowed but
more complex ones are not allowed. My opinion here is to find and
eliminate the complex ones and then to allow inline conditionals.
2. Protected Fields
We have 135 fields with 'protected' modifier. Checkstyle complains
that instead the modifier should be private accessor methods should be
used. The rationale is to enforce encapsulation. Should we configure
checkstyle to allow protected and/or package modifiers?
3. Javadoc for Private Members
Checkstyle complains about missing Javadoc of private fields. I think
we should relax that rule and don't force Javadoc for private fields
because IMO the variable name should be descriptive. Thoughts?
Kind Regards,
Stefan
[1] https://hudson.apache.org/hudson/view/Directory/job/dir-shared-metrics/
Re: Checkstyle questions: inline conditionals, protected fields, Javadoc for private members
Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
Hi Stefan,
+1 on all three points.
100% agreed.
Regards,
Pierre-Arnaud
On 4 oct. 2010, at 10:11, Stefan Seelmann wrote:
> Hi dev,
>
> we are trying to fix remaining checkstyle errors in shared [1] and
> have some questions:
>
>
> 1. Inline Conditionals
> We have 151 inline conditionals, should we get rid of them or should
> we allow them?
>
> IMO 'simple' inline conditionals are ok:
> return oid == null ? "" : oid;
>
> Such constructs could be simplified
> return ( ( byteArray[index] == car ) ? true : false );
> to
> return byteArray[index] == car;
>
> However nested inline conditionals are hard to read and should be avoided:
> return ( val < 0 ? -1 : ( val > 0 ? 1 : val ) );
>
> So what is our policy regarding inline conditionals? With checkstyle
> we can't configure that simple inline conditionals are allowed but
> more complex ones are not allowed. My opinion here is to find and
> eliminate the complex ones and then to allow inline conditionals.
>
>
> 2. Protected Fields
> We have 135 fields with 'protected' modifier. Checkstyle complains
> that instead the modifier should be private accessor methods should be
> used. The rationale is to enforce encapsulation. Should we configure
> checkstyle to allow protected and/or package modifiers?
>
>
> 3. Javadoc for Private Members
> Checkstyle complains about missing Javadoc of private fields. I think
> we should relax that rule and don't force Javadoc for private fields
> because IMO the variable name should be descriptive. Thoughts?
>
>
> Kind Regards,
> Stefan
>
>
> [1] https://hudson.apache.org/hudson/view/Directory/job/dir-shared-metrics/
Re: Checkstyle questions: inline conditionals, protected fields,
Javadoc for private members
Posted by Stefan Seelmann <se...@apache.org>.
Thanks all for the feedback.
I already tweaked the checkstyle config, so protected fields are
allowed and no Javadoc is required for private fields.
I'll go through the inline conditionals and try to eliminate them.
Kind Regards,
Stefan
On Mon, Oct 4, 2010 at 10:11 AM, Stefan Seelmann <se...@apache.org> wrote:
> Hi dev,
>
> we are trying to fix remaining checkstyle errors in shared [1] and
> have some questions:
>
>
> 1. Inline Conditionals
> We have 151 inline conditionals, should we get rid of them or should
> we allow them?
>
> IMO 'simple' inline conditionals are ok:
> return oid == null ? "" : oid;
>
> Such constructs could be simplified
> return ( ( byteArray[index] == car ) ? true : false );
> to
> return byteArray[index] == car;
>
> However nested inline conditionals are hard to read and should be avoided:
> return ( val < 0 ? -1 : ( val > 0 ? 1 : val ) );
>
> So what is our policy regarding inline conditionals? With checkstyle
> we can't configure that simple inline conditionals are allowed but
> more complex ones are not allowed. My opinion here is to find and
> eliminate the complex ones and then to allow inline conditionals.
>
>
> 2. Protected Fields
> We have 135 fields with 'protected' modifier. Checkstyle complains
> that instead the modifier should be private accessor methods should be
> used. The rationale is to enforce encapsulation. Should we configure
> checkstyle to allow protected and/or package modifiers?
>
>
> 3. Javadoc for Private Members
> Checkstyle complains about missing Javadoc of private fields. I think
> we should relax that rule and don't force Javadoc for private fields
> because IMO the variable name should be descriptive. Thoughts?
>
>
> Kind Regards,
> Stefan
>
>
> [1] https://hudson.apache.org/hudson/view/Directory/job/dir-shared-metrics/
>
Re: Checkstyle questions: inline conditionals, protected fields,
Javadoc for private members
Posted by Alex Karasulu <ak...@apache.org>.
On Mon, Oct 4, 2010 at 11:29 AM, Emmanuel Lecharny <el...@gmail.com>wrote:
> On 10/4/10 10:11 AM, Stefan Seelmann wrote:
>
>> Hi dev,
>>
>> we are trying to fix remaining checkstyle errors in shared [1] and
>> have some questions:
>>
>>
>> 1. Inline Conditionals
>> We have 151 inline conditionals, should we get rid of them or should
>> we allow them?
>>
>> IMO 'simple' inline conditionals are ok:
>> return oid == null ? "" : oid;
>>
>> Such constructs could be simplified
>> return ( ( byteArray[index] == car ) ? true : false );
>> to
>> return byteArray[index] == car;
>>
>> However nested inline conditionals are hard to read and should be avoided:
>> return ( val< 0 ? -1 : ( val> 0 ? 1 : val ) );
>>
>
> IMO, we should avoid inline conditionals.
+1
Yeah I'd avoid all inline conditionals even if simple. It's a clear policy
instead of a soft one. Then someone might try to interpret what is simple.
>
>
> 2. Protected Fields
>> We have 135 fields with 'protected' modifier. Checkstyle complains
>> that instead the modifier should be private accessor methods should be
>> used. The rationale is to enforce encapsulation. Should we configure
>> checkstyle to allow protected and/or package modifiers?
>>
> I think 'protected' is useful to distinguish local fields from those that
> are contained in the class but can be used by the inherited classes. I don't
> think we should remove them.
>
>
+1
> All in all, we always use either private or public fields, except when we
> decide to use protected ones, so it's a decision we make based on a serious
> thought. Let's keep them.
>
>
>
>> 3. Javadoc for Private Members
>> Checkstyle complains about missing Javadoc of private fields. I think
>> we should relax that rule and don't force Javadoc for private fields
>> because IMO the variable name should be descriptive. Thoughts?
>>
> +1
>
>
+1
--
Alex Karasulu
My Blog :: http://www.jroller.com/akarasulu/
Apache Directory Server :: http://directory.apache.org
Apache MINA :: http://mina.apache.org
To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: Checkstyle questions: inline conditionals, protected fields,
Javadoc for private members
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 10/4/10 10:11 AM, Stefan Seelmann wrote:
> Hi dev,
>
> we are trying to fix remaining checkstyle errors in shared [1] and
> have some questions:
>
>
> 1. Inline Conditionals
> We have 151 inline conditionals, should we get rid of them or should
> we allow them?
>
> IMO 'simple' inline conditionals are ok:
> return oid == null ? "" : oid;
>
> Such constructs could be simplified
> return ( ( byteArray[index] == car ) ? true : false );
> to
> return byteArray[index] == car;
>
> However nested inline conditionals are hard to read and should be avoided:
> return ( val< 0 ? -1 : ( val> 0 ? 1 : val ) );
IMO, we should avoid inline conditionals.
> 2. Protected Fields
> We have 135 fields with 'protected' modifier. Checkstyle complains
> that instead the modifier should be private accessor methods should be
> used. The rationale is to enforce encapsulation. Should we configure
> checkstyle to allow protected and/or package modifiers?
I think 'protected' is useful to distinguish local fields from those
that are contained in the class but can be used by the inherited
classes. I don't think we should remove them.
All in all, we always use either private or public fields, except when
we decide to use protected ones, so it's a decision we make based on a
serious thought. Let's keep them.
>
> 3. Javadoc for Private Members
> Checkstyle complains about missing Javadoc of private fields. I think
> we should relax that rule and don't force Javadoc for private fields
> because IMO the variable name should be descriptive. Thoughts?
+1
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: Checkstyle questions: inline conditionals, protected fields,
Javadoc for private members
Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Stefan
+1 for all questions.
Regards
Felix
On 10/04/10 10:11, Stefan Seelmann wrote:
> Hi dev,
>
> we are trying to fix remaining checkstyle errors in shared [1] and
> have some questions:
>
>
> 1. Inline Conditionals
> We have 151 inline conditionals, should we get rid of them or should
> we allow them?
>
> IMO 'simple' inline conditionals are ok:
> return oid == null ? "" : oid;
>
> Such constructs could be simplified
> return ( ( byteArray[index] == car ) ? true : false );
> to
> return byteArray[index] == car;
>
> However nested inline conditionals are hard to read and should be avoided:
> return ( val < 0 ? -1 : ( val > 0 ? 1 : val ) );
>
> So what is our policy regarding inline conditionals? With checkstyle
> we can't configure that simple inline conditionals are allowed but
> more complex ones are not allowed. My opinion here is to find and
> eliminate the complex ones and then to allow inline conditionals.
>
>
> 2. Protected Fields
> We have 135 fields with 'protected' modifier. Checkstyle complains
> that instead the modifier should be private accessor methods should be
> used. The rationale is to enforce encapsulation. Should we configure
> checkstyle to allow protected and/or package modifiers?
>
>
> 3. Javadoc for Private Members
> Checkstyle complains about missing Javadoc of private fields. I think
> we should relax that rule and don't force Javadoc for private fields
> because IMO the variable name should be descriptive. Thoughts?
>
>
> Kind Regards,
> Stefan
>
>
> [1] https://hudson.apache.org/hudson/view/Directory/job/dir-shared-metrics/
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkypj20ACgkQ2lZVCB08qHFO/ACfcZmKwf0XgW+8dH0TylLuxK6K
PqMAninyyT4/Blbnd7G/n22tkNu45PcI
=BNWY
-----END PGP SIGNATURE-----