Hi,
One of the reports we use in generating the site is the checkstyle report.
It is pretty useful to keep the code clean, however, the configuration
needs to be tweaked a little in my opinion. I'd like to change the
following:
- At the moment, Checkstyle checks for local variables which hide fields.
For example, it reports the following as error:
private String driver = null;
public void setDriver(String driver)
{
this.driver = driver;
}
because the parameter driver hides the field driver in the settings. In my
opinion, using the same variables in setters is perfectly good practice.
However, one still wants to be warned about other local variables which
hide fields. So I'd like to change the configuration
module name="HiddenField"/>
to
<module name="HiddenField">
<property name="tokens" value="VARIABLE_DEF"/>
</module>
See also http://checkstyle.sourceforge.net/config_coding.html#HiddenField
- At the moment, checkstyle reports nested blocks as errors. For example:
someMethod()
{
....
{
File outputDirectory = new File(outputDir);
getLog().debug("generating torque java sources into: "
+ outputDirectory.getAbsolutePath());
outputDirectory.mkdirs();
generatorTask.setOutputDirectory(outputDirectory);
}
....
}
The extra curly brackets serve as a method to reduce the scope of the
variable outputDirectory to the region where it is needed. In my opinion,
this reduces the possibility for bugs and should not be reported as an
error. So I'd like to remove the configuration tag
<module name="AvoidNestedBlocks"/>
from the checkstyle configuration.
See also
http://checkstyle.sourceforge.net/config_blocks.html#AvoidNestedBlocks
for the rationale for using this module.
- In the check for the license, line 1 and line 6 are ignored at the
moment:
<module name="Header">
...
<property name="ignoreLines" value="1,6"/>
</module>
Line 1 must be ignored because this contains the package definition. Line
6 contains the following:
* Licensed under the Apache License, Version 2.0 (the "License")
which needs not be ignored (all our files are under the Apache 2.0
license).
Instead, I'd like to ignore line 4 ,which says
* Copyright 2001-2006 The Apache Software Foundation.
which would allow the copyright year to be different across files, and
people would not be tempted to change the copyright year in files which
did not change in any other way.
See also http://checkstyle.sourceforge.net/config_header.html#Header
- While we are at checkstyle, I'm in doubt about the
<module name="MagicNumber"/>
module, http://checkstyle.sourceforge.net/config_coding.html#MagicNumber
It says that any fixed number used in code should be a public static final
constant. Although this makes sense in most cases, it does not make sense
when specifying initial sizes for arrays, maps etc. Any thoughts ?
If there are no objections, I'll change the HiddenFields,
AvoidNestedBlocks and Header checks as proposed.
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
