db-torque-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Fischer <tfisc...@apache.org>
Subject checkstyle configuration
Date Sat, 04 Nov 2006 12:28:15 GMT
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: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org


Mime
View raw message