2 Replies Latest reply on Feb 2, 2011 9:26 AM by Keith Babo

    unnecessary checkstyle rules, IMO

    David Ward Master

      I really don't believe we need these checkstyle rules:

       

      Message: Redundant 'public' modifier.

      Flagged: public String getFoo();

      Suggested: String getFoo();

      Beef: I would rather my method signature on my interface reads exactly as it does in the implementation.

       

      Message: ',' is not followed by whitespace.

      Flagged: private Map<String,String> map;

      Suggested: private Map<String, String> map;

      Beef: I would rather have it more compact.

       

      Message: '{' is not followed by whitespace.

      Flagged: private Foo() {}

      Suggested: private Foo() {

      }

      Beef: I would rather have it more compact.

      PS: private constructors are useful for singletons or utility classes.

       

      Message: '}' is not preceded with whitespace.

      Flagged: private Foo() {}

      Suggested: private Foo() {

      }

      Beef: I would rather have it more compact.

      PS: private constructors are useful for singletons or utility classes.

       

      Message: 'cast' is not followed by whitespace.

      Flagged: B b = (B)a;

      Suggested: B b = (B) a;

      Beef: I would rather have it more compact.

       

      Message: '=' is not preceded with whitespace.

      Flagged: for (int i=0; i < length; i++)

      Suggested: for (int i = 0; i < length; i++)

      Beef: I would rather have it more compact.

       

      Message: Name '_child_thing' must match pattern '^[a-z][a-zA-Z0-9]*$'.

      Flagged: private Thing _child_thing;

      Suggested: private Thing _childThing;

      Beef: Who cares?

       

      Message: '1024' is a magic number.

      Flagged: StringBuffer buffer = new StringBuffer(1024);

      Suggested: StringBuffer buffer = new StringBuffer();

      Beef: Sometimes you want to set a buffer size for better performance.

       

      Message: '1024' is a magic number.

      Flagged: char[] c = new char[1024];

      Suggested: int length = 1024; char[] c = new char[length];

      Beef: Sometimes you want to set a buffer size for better performance.

       

      Message: '+' is not preceded with whitespace

      Flagged: return name.substring(pos+1, name.length());

      Suggested: return name.substring(pos +1, name.length());

      Beef: I would rather have it more compact.

       

      Message: '+' is not followed by whitespace.

      Flagged: return name.substring(pos+1, name.length());

      Suggested: return name.substring(pos+ 1, name.length());

      Beef: I would rather have it more compact.

       

      Can we please get rid of them?

        • 1. unnecessary checkstyle rules, IMO
          Tom Cunningham Master

          Definitely agree on the redundant public modifier.

          • 2. unnecessary checkstyle rules, IMO
            Keith Babo Master

            Ones that I agree with:

            • redundant modifier - personally, I prefer to not use public modifier in interfaces, but this is a matter of taste that does not affect maintainability, IMO.
            • cast is not followed by whitespace
            • _child_thing should be allowed, but we can't remove that rule altogether.  I want member variables prefaced by a "_".  It's the one super annoying rule that I'm going to make us follow.

             

            The magic number one can be fixed by just using a "final int BUF_SIZE = 1024".

            The constructor one can be fixed by just adding a space between "{}", like "{ }", can't it?  I'm not sure this is a super important rule to have if this is the only thing it checks.