Repoweb ruleset :
| AccessorClassGenerationRule | Avoid instantiation through private constructors from outside of the constructor's class. |
| AvoidDeeplyNestedIfStmts | Deeply nested if..then statements are hard to read |
| AvoidDuplicateLiterals | The same String literal appears {0} times in this file; the first occurrence is on line {1} |
| AvoidReassigningParametersRule | Avoid reassigning parameters such as ''{0}'' |
| CouplingBetweenObjectsRule | High amount of different objects as memebers donotes a high coupling |
| CyclomaticComplexityRule | The {0} ''{1}'' has a Cyclomatic Complexity of {2}. |
| DontImportJavaLang | Avoid importing anything from the package 'java.lang' |
| DoubleCheckedLockingRule | Double checked locking is not thread safe in Java. |
| DuplicateImports | Avoid duplicate imports such as ''{0}'' |
| EmptyCatchBlock | Avoid empty catch blocks |
| EmptyFinallyBlock | Avoid empty finally blocks |
| EmptyIfStmt | Avoid empty 'if' statements |
| EmptySwitchStatements | Avoid empty switch statements |
| EmptyTryBlock | Avoid empty try blocks |
| EmptyWhileStmt | Avoid empty 'while' statements |
| ExcessiveClassLength | Avoid really long Classes. |
| ExcessiveImportsRule | A high number of imports can indicate a high degree of coupling within an object. |
| ExcessiveMethodLength | Avoid really long methods. |
| ExcessiveParameterList | Avoid really long parameter lists. |
| ExcessivePublicCountRule | A high number of public methods and attributes in an object can indicate the class may need to be broken up for exhaustive testing may prove difficult. |
| ForLoopShouldBeWhileLoop | This for loop could be simplified to a while loop |
| ForLoopsMustUseBracesRule | Avoid using 'for' statements without curly braces |
| IfElseStmtsMustUseBracesRule | Avoid using 'if...else' statements without curly braces |
| IfStmtsMustUseBraces | Avoid using if statements without curly braces |
| ImportFromSamePackage | No need to import a type that's in the same package |
| JUnitAssertionsShouldIncludeMessageRule | JUnit assertions should include a message |
| JUnitSpelling | You may have misspelled a JUnit framework method (setUp or tearDown) |
| JUnitStaticSuite | You have a suite() method that is not both public and static, so JUnit won't call it to get your TestSuite. Is that what you wanted to do? |
| JumbledIncrementer | Avoid using an outer loop incrementer in an inner loop for update expression |
| LongVariable | Avoid excessively long variable names |
| LooseCouplingRule | Avoid using implementation types like ''{0}''; use the interface instead |
| OverrideBothEqualsAndHashcodeRule | Ensure you override both equals() and hashCode() |
| ReturnFromFinallyBlock | Avoid returning from a finally block |
| ShortMethodNameRule | Avoid using short method names |
| ShortVariable | Avoid variables with short names |
| SimplifyBooleanReturnsRule | Avoid unnecessary if..then..else statements when returning a boolean |
| StringInstantiation | Avoid instantiating String objects; this is usually unnecessary. |
| StringToString | Avoid calling toString() on String objects; this is unnecessary |
| SwitchDensity | A high ratio of statements to labels in a switch statement. Consider refactoring. |
| SwitchStmtsShouldHaveDefault | Switch statements should have a default label |
| UnnecessaryConversionTemporaryRule | Avoid unnecessary temporaries when converting primitives to Strings |
| UnusedFormalParameter | Avoid unused formal parameters such as ''{0}'' |
| UnusedImports | Avoid unused imports such as ''{0}'' |
| UnusedLocalVariable | Avoid unused local variables such as ''{0}'' |
| UnusedPrivateField | Avoid unused private fields such as ''{0}'' |
| UnusedPrivateMethod | Avoid unused private methods such as ''{0}'' |
| WhileLoopsMustUseBracesRule | Avoid using 'while' statements without curly braces |
Repoweb Ruleset
Rule counts unique attributes, local variables and return types within an object. An amount higher than specified threshold can indicate a high degree of couping with in an object
Here's an example of code that would trigger this rule:
import com.Blah;
import org.Bar;
import org.Bardo;
//
public class Foo {
private Blah var1;
private Bar var2;
//followed by many imports of unique objects
void ObjectC doWork() {
Bardo var55;
ObjectA var44;
ObjectZ var93;
return something;
}
}
A high number of imports can indicate a high degree of coupling within an object. Rule counts the number of unique imports and reports a violation if the count is above the user defined threshold.
Here's an example of code that would trigger this rule:
import blah.blah.Bardo;
import blah.blah.Hardo;
import blah.blah.Bar;
import blah.net.ObjectA;
//imports over some threshold
public class Foo {
public void doWork() {}
}
Avoid instantiating String objects; this is usually unnecessary.
Here's an example of code that would trigger this rule:
public class Foo {
private String bar = new String("bar"); // just do a String bar = "bar";
}
Code containing duplicate String literals can usually be improved by declaring the String as a constant field.
Here's an example of code that would trigger this rule:
public class Foo {
private void bar() {
buz("Howdy");
buz("Howdy");
buz("Howdy");
buz("Howdy");
}
private void buz(String x) {}
}
Avoid calling toString() on String objects; this is unnecessary
Here's an example of code that would trigger this rule:
public class Foo {
private String baz() {
String bar = "howdy";
return bar.toString();
}
}
Unused Private Field detects when a private field is declared that is not used by the class.
Here's an example of code that would trigger this rule:
public class Something {
private static int FOO = 2; // Unused
private int i = 5; // Unused
private int j = 6;
public int addOne() {
return j++;
}
}
Unused Local Variables detects when a variable is declared, but not used (except for possibly initial assignment)
Here's an example of code that would trigger this rule:
public int doSomething() {
int i = 5; // Unused
int j = 6;
j += 3;
return j;
}
Unused Private Method detects when a private method is declared but is unused.
Here's an example of code that would trigger this rule:
public class Something {
private void foo() {} // unused
}
Avoid passing parameters to methods and then not using those parameters.
Here's an example of code that would trigger this rule:
public class Foo {
private void bar(String howdy) {
// howdy is not used
}
Detects when a field, local or parameter has a short name.
Here's an example of code that would trigger this rule:
public class Foo {
public void aboutWindow(Object k) { // NOK
k.toString();
try {
jbInit();
int i; // NOK
}
catch (Exception e) { // OK
e.printStackTrace();
}
catch (Exception e) { // OK
e.printStackTrace();
int i; // NOK
}
}
private int q = 15; // VIOLATION - Field
public static void main( String a[] ) { // VIOLATION - Formal
int r = 20 + q; // VIOLATION - Local
for (int i = 0; i < 10; i++) { // Not a Violation (inside FOR)
r += q;
}
}
}
Detects when a field, formal or local variable is declared with a big name.
Here's an example of code that would trigger this rule:
public class Something {
int reallyLongIntName = -3; // VIOLATION - Field
public static void main( String argumentsList[] ) { // VIOLATION - Formal
int otherReallyLongName = -5; // VIOLATION - Local
for (int interestingIntIndex = 0; // VIOLATION - For
interestingIntIndex < 10;
interestingIntIndex ++ ) {
}
}
Detects when very short method names are used.
Here's an example of code that would trigger this rule:
public class ShortMethod {
public void a( int i ) { // Violation
}
}
The suite() method in a JUnit test needs to be both public and static.
Here's an example of code that would trigger this rule:
import junit.framework.*;
public class Foo extends TestCase {
public void suite() {} // oops, should be static
private static void suite() {} // oops, should be public
}
Some JUnit framework methods are easy to misspell.
Here's an example of code that would trigger this rule:
import junit.framework.*;
public class Foo extends TestCase {
public void setup() {} // oops, should be setUp
public void TearDown() {} // oops, should be tearDown
}
JUnit assertions should include a message - i.e., use the three argument version of assertEquals(), not the two argument version.
Here's an example of code that would trigger this rule:
public class Foo extends TestCase {
public void testSomething() {
assertEquals("foo", "bar");
// not good! use the form:
// assertEquals("Foo does not equals bar", "foo", "bar");
// instead
}
}
Avoid duplicate import statements.
Here's an example of code that would trigger this rule:
// this is bad
import java.io.File;
import java.io.File;
public class Foo {}
// --- in another source code file...
// this is bad
import java.io.*;
import java.io.File;
public class Foo {}
Avoid importing anything from the package 'java.lang'. These classes are automatically imported (JLS 7.5.3).
Here's an example of code that would trigger this rule:
// this is bad
import java.lang.String;
public class Foo {}
// --- in another source code file...
// this is bad
import java.lang.*;
public class Foo {}
Avoid unused import statements.
Here's an example of code that would trigger this rule:
// this is bad
import java.io.File;
public class Foo {}
No need to import a type that's in the same package.
Here's an example of code that would trigger this rule:
package foo;
import foo.Buz; // no need for this
public class Bar{}
Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead
Here's an example of code that would trigger this rule:
import java.util.*;
public class Bar {
// should be "private List list"
private ArrayList list = new ArrayList();
// should be "public Set getFoo()"
public HashSet getFoo() {
return new HashSet();
}
}
Avoid unnecessary if..then..else statements when returning a boolean
Here's an example of code that would trigger this rule:
public class Foo {
private int bar =2;
public boolean isBarEqualsTo(int x) {
// this bit of code
if (bar == x) {
return true;
} else {
return false;
}
// can be replaced with a simple
// return bar == x;
}
}
Switch statements should have a default label.
Here's an example of code that would trigger this rule:
public class Foo {
public void bar() {
int x = 2;
switch (x) {
case 2: int j = 8;
}
}
}
Deeply nested if..then statements are hard to read.
Here's an example of code that would trigger this rule:
public class Foo {
public void bar() {
int x=2;
int y=3;
int z=4;
if (x>y) {
if (y>z) {
if (z==x) {
// this is officially out of control now
}
}
}
}
}
Reassigning values to parameters is a questionable practice. Use a temporary local variable instead.
Here's an example of code that would trigger this rule:
public class Foo {
private void foo(String bar) {
bar = "something else";
}
}
A high ratio of statements to labels in a switch statement implies that the switch statement is doing too much work. Consider moving the statements either into new methods, or creating subclasses based on the switch variable.
Here's an example of code that would trigger this rule:
public class Foo {
private int x;
public void bar() {
switch (x) {
case 1: {
System.out.println("I am a fish.");
System.out.println("I am a fish.");
System.out.println("I am a fish.");
System.out.println("I am a fish.");
break;
}
case 2: {
System.out.println("I am a cow.");
System.out.println("I am a cow.");
System.out.println("I am a cow.");
System.out.println("I am a cow.");
break;
}
}
}
}
Instantiation by way of private constructors from outside of the constructor's class often causes the generation of an accessor. A factory method, or non-privitization of the constructor can eliminate this situation. The generated class file is actually an interface. It gives the accessing class the ability to invoke a new hidden package scope constructor that takes the interface as a supplementary parameter. This turns a private constructor effectively into one with package scope, though not visible to the naked eye.
Here's an example of code that would trigger this rule:
public class OuterClass {
void method(){
InnerClass ic = new InnerClass();//Causes generation of accessor
}
public class InnerClass {
private InnerClass(){
}
}
}
public class OuterClass {
void method(){
InnerClass ic = new InnerClass();//OK, due to public constructor
}
public class InnerClass {
public InnerClass(){
}
}
}
public class OuterClass {
void method(){
InnerClass ic = InnerClass.getInnerClass();//OK
}
public static class InnerClass {
private InnerClass(){
}
private static InnerClass getInnerClass(){
return new InnerClass();
}
}
}
public class OuterClass {
private OuterClass(){
}
public class InnerClass {
void method(){
OuterClass oc = new OuterClass();//Causes generation of accessor
}
}
}
Excessive Method Length usually means that the method is doing too much. There is usually quite a bit of Cut and Paste there as well. Try to reduce the method size by creating helper methods, and removing cut and paste. Default value is 2.5 sigma greater than the mean. NOTE: In version 0.9 and higher, their are three parameters available: minimum - Minimum Length before reporting. sigma - Std Deviations away from the mean before reporting. topscore - The Maximum Number of reports to generate. At this time, only one can be used at a time.
Here's an example of code that would trigger this rule:
public void doSomething() {
System.out.println("I am a fish.");
System.out.println("I am a fish.");
System.out.println("I am a fish.");
System.out.println("I am a fish.");
System.out.println("I am a fish.");
// 495 copies omitted for brevity.
}
This checks to make sure that the Parameter Lists in the project aren't getting too long. If there are long parameter lists, then that is generally indicative that another object is hiding around there. Basically, try to group the parameters together. Default value is 2.5 sigma greater than the mean. NOTE: In version 0.9 and higher, their are three parameters available: minimum - Minimum Length before reporting. sigma - Std Deviations away from the mean before reporting. topscore - The Maximum Number of reports to generate. At this time, only one can be used at a time.
Here's an example of code that would trigger this rule:
public void addData(
int p00, int p01, int p02, int p03, int p04, int p05,
int p05, int p06, int p07, int p08, int p09, int p10) {
}
}
Long Class files are indications that the class may be trying to do too much. Try to break it down, and reduce the size to something managable. Default value is 2.5 sigma greater than the mean. NOTE: In version 0.9 and higher, their are three parameters available: minimum - Minimum Length before reporting. sigma - Std Deviations away from the mean before reporting. topscore - The Maximum Number of reports to generate. At this time, only one can be used at a time.
Here's an example of code that would trigger this rule:
public class Foo {
public void bar() {
// 500 lines of code
}
public void baz() {
// 500 more lines of code
}
}
Complexity is determined by the number of decision points in a method plus one for the method entry. The decision points are 'if', 'while', 'for', and 'case labels'. Scale: 1-4 (low complexity) 5-7 (moderate complexity) 8-10 (high complexity) 10+ (very high complexity)
Here's an example of code that would trigger this rule:
Cyclomatic Complexity = 12
public class Foo
{
1 public void example()
{
2 if (a == b)
{
3 if (a1 == b1)
{
do something;
}
4 else if a2 == b2)
{
do something;
}
else
{
do something;
}
}
5 else if (c == d)
{
6 while (c == d)
{
do something;
}
}
7 else if (e == f)
{
8 for (int n = 0; n < h; n++)
{
do something;
}
}
else
{
switch (z)
{
9 case 1:
do something;
break;
10 case 2:
do something;
break;
11 case 3:
do something;
break;
12 default:
do something;
break;
}
}
}
}
A large amount of public methods and attributes declared in an object can indicate the class may need to be broken up as increased effort will be required to thoroughly test such a class.
Here's an example of code that would trigger this rule:
public class Foo {
public String value;
public Bar something;
public Variable var;
//more public attributes
public void doWork() {}
public void doMoreWork() {}
public void doWorkAgain() {}
public void doWorking() {}
public void doWorkIt() {}
public void doWorkingAgain() {}
public void doWorkAgainAgain() {}
public void doWorked() {}
}
Empty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this swallows an exception which should either be acted on or reported.
Here's an example of code that would trigger this rule:
public void doSomething() {
try {
FileInputStream fis = new FileInputStream("/tmp/bugger");
} catch (IOException ioe) {
// not good
}
}
Empty If Statement finds instances where a condition is checked but nothing is done about it.
Here's an example of code that would trigger this rule:
if (absValue < 1) {
// not good
}
Empty While Statement finds all instances where a while statement does nothing. If it is a timing loop, then you should use Thread.sleep() for it; if it's a while loop that does a lot in the exit expression, rewrite it to make it clearer.
Here's an example of code that would trigger this rule:
while (a == b) {
// not good
}
Avoid empty try blocks - what's the point?
Here's an example of code that would trigger this rule:
// this is bad
public void bar() {
try {
} catch (Exception e) {
e.printStackTrace();
}
}
Avoid empty finally blocks - these can be deleted.
Here's an example of code that would trigger this rule:
// this is bad
public void bar() {
try {
int x=2;
} finally {
}
}
Avoid empty switch statements.
Here's an example of code that would trigger this rule:
public class Foo {
public void bar() {
int x = 2;
switch (x) {
// once there was code here
// but it's been commented out or something
}
}
}
Avoid jumbled loop incrementers - it's usually a mistake, and it's confusing even if it's what's intended.
Here's an example of code that would trigger this rule:
public class JumbledIncrementerRule1 {
public void foo() {
for (int i = 0; i < 10; i++) {
for (int k = 0; k < 20; i++) {
System.out.println("Hello");
}
}
}
}}
Some for loops can be simplified to while loops - this makes them more concise.
Here's an example of code that would trigger this rule:
public class Foo {
void bar() {
for (;true;) true; // No Init or Update part, may as well be: while (true)
}
}
Avoid unnecessary temporaries when converting primitives to Strings
Here's an example of code that would trigger this rule:
public String convert(int x) {
// this wastes an object
String foo = new Integer(x).toString();
// this is better
return Integer.toString(x);
}
Override both public boolean Object.equals(Object other), and public int Object.hashCode(), or override neither. Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly delegating to your superclass.
Here's an example of code that would trigger this rule:
// this is bad
public class Bar {
public boolean equals(Object o) {
// do some comparison
}
}
// and so is this
public class Baz {
public int hashCode() {
// return some hash value
}
}
// this is OK
public class Foo {
public boolean equals(Object other) {
// do some comparison
}
public int hashCode() {
// return some hash value
}
}
Partially created objects can be returned by the Double Checked Locking pattern when used in Java. An optimizing JRE may assign a reference to the baz variable before it creates the object the reference is intended to point to. For more details see http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html.
Here's an example of code that would trigger this rule:
public class Foo {
Object baz;
Object bar() {
if(baz == null) { //baz may be non-null yet not fully created
synchronized(this){
if(baz == null){
baz = new Object();
}
}
}
return baz;
}
}
Avoid returning from a finally block - this can discard exceptions.
Here's an example of code that would trigger this rule:
public class Bar {
public String bugga() {
try {
throw new Exception( "My Exception" );
} catch (Exception e) {
throw e;
} finally {
return "A. O. K."; // Very bad.
}
}
}
Avoid using if statements without using curly braces
Here's an example of code that would trigger this rule:
public class Foo {
public void bar() {
int x = 0;
if (foo) x++;
}
}
Avoid using 'while' statements without using curly braces
Here's an example of code that would trigger this rule:
public void doSomething() {
while (true)
x++;
}