On Mon, Oct 4, 2010 at 11:29 AM, Emmanuel Lecharny <email@example.com> wrote:
On 10/4/10 10:11 AM, Stefan Seelmann wrote:IMO, we should avoid inline conditionals.
we are trying to fix remaining checkstyle errors in shared  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 );
return byteArray[index] == car;
However nested inline conditionals are hard to read and should be avoided:
return ( val< 0 ? -1 : ( val> 0 ? 1 : val ) );
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.
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?
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.+1
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?