commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Herbert <alex.d.herb...@gmail.com>
Subject [rng][geometry][statistics][numbers] Updating Checkstyle rules
Date Tue, 21 May 2019 11:47:49 GMT
The checkstyle file for all these projects has a common origin (of 
[math]?). Checkstyle has advanced since the origin of these checks and 
there are many more checks that can be added to maintain the current 
coding style.

I've looked at this for RNG starting from a template for the Sun 
standards [1].

The code is maintained to high standard and the sun standards need 
little modification. Here are the changes:

- Removed FinalParameters

- Removed MagicNumber

- Removed InnerAssignment

- Change line length to 120

- Changed ParameterNumber to only check methods (not constructors)

- Changed NoWhitespaceAfter to remove checks for array initialisers 
allowing the whitespace after the { here:

double[] array = new double[] { 1, 2, 3 };

(arguably this could be left at the default Sun coding style to have 
'new double[] {1, 2, 3}'.)

- Changed WhitespaceAround to allow empty constructors (for private 
utility constructors) and empty types (for marker interfaces)

- Changed VisibilityModifier to allow protected fields

- Changed OperatorWrap to the current checkstyle config


When run over RNG this requires the following changes:

Some method javadoc required a missing end period.

Line length: Some wrapping to 120 characters is required.

Whitespace after: Some updates to change declarations of generics, e.g.

Map<Integer,String> to Map<Integer, String>

3 utility classes should be final. These may be a breaking API changes. 
The classes have private constructor so should not be inherited from by 
any user. Only one is public the other two are package private.

1 TODO comment is as yet undone.

Indentation: This picked up a few formatting errors.


All the changes are in a new PR [2] so you can view the new additions to 
the checkstyle file and the changes to the code that must be made.

I have just appended the additions to the current checkstyle config. 
However going forward it may be better to match the order of the 
reference checkstyle template provided by checkstyle and then add to 
that for commons specific requirements.


Statistics changes:

I tried this on statistics. There are 415 errors, 50 errors if the test 
sources are excluded. Most of these in the source are genuine formatting 
issues. The only rule that is broken is LocalFinalVariableName which 
requires variables to be named '^[a-z][a-zA-Z0-9]*$'.

I do not agree with naming conventions when the code is implementing an 
equation so I would either remove this rule or add checkstyle exceptions 
file for the two methods where the rule is broken.

See the modifications for statistics in this PR [3]. I've not fixed all 
the tests. They are mainly failures due to indentation or whitespace, e.g:

observedCounts[s-1]++;
vs
observedCounts[s - 1]++;


Fixing them would be easy but I am out of time for today.


[1] 
https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/sun_checks.xml

[2] https://github.com/apache/commons-rng/pull/44

[3] https://github.com/apache/commons-statistics/pull/10



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message