On Mon, Oct 4, 2010 at 11:29 AM, Emmanuel Lecharny <[email protected]>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
