Design

不適切なコードデザインに関するルールセットです。

UseSingleton

staticメソッドしか持たないクラスで、Singleton化されていないものを検出します。

SimplifyBooleanReturns

単純化できるboolean値のreturn文を検出します。

if (bar == x) {
  return true;
} else {
  return false;
}
// 上のコードは return bar = x; と単純化できる

SimplifyBooleanExpressions

単純化できるboolean式を検出します。

if (b == true) { // これは if (b) と単純化できる
  ...
}

SwitchStmtsShouldHaveDefault

default節の無いswitch文を検出します。

AvoidDeeplyNestedIfStmts

深過ぎるif文のネストを検出します。

problemDepth

検出するネスト数の最小値を指定します。
デフォルトは 3 です。

AvoidReassigningParameters

メソッドパラメータへの代入箇所を検出します。

public void foo(String bar) {
  bar = "reassign"; // パラメータへの代入はNG
}

SwitchDensity

caseブロックに存在する長いステートメントを検出します。

switch (x) {
  case 1:
    // ここに大量のコードを記述するのは好ましくない
    break;
}

minimum

検出するステートメントの最小数。
デフォルトは 10 です。

ConstructorCallsOverridableMethod

コンストラクタ内で、オーバーライドされたメソッド呼び出しを検出します。
例を挙げて説明します。

public class SeniorClass {
  public SeniorClass(){
      toString();
  }
  public String toString(){
    return "IAmSeniorClass";
  }
}
public class JuniorClass extends SeniorClass {
  private String name;
  public JuniorClass(){
    super(); // NullPointerExceptionの発生
    name = "JuniorClass";
  }
  public String toString(){
    return name.toUpperCase();
  }
}

このとき、

new SeniorClass();

の呼び出しは成功しますが

new JuniorClass();

の呼び出しをすると NullPointerException が発生します。

JuniorClass のコンストラクタ内で super() がコールされると
SeniorClass() に処理が移り toString() がコールされます。
このとき、呼び出されるメソッドは SeniorClass.toString() ではなく
JuniorClass.toString() になります。
ここで参照している name フィールドはこの段階ではまだ初期化されていない為
NullPointerException が発生してしまうという訳です。

こういったバグはなかなか発見が難しいので、
コンストラクタ内で呼び出すメソッドには充分注意する必要があります。

AccessorClassGeneration

privateコンストラクタの呼び出し箇所を検出します。
具体的には、以下のような場面です。

public class Outer {
  void method(){
    Inner ic = new Inner(); // この箇所
  }
  public class Inner {
    private Inner(){}
  }
}

Inner クラスのコンストラクタは private ですが、
Outer クラスのメソッド内から呼び出すことが出来ます。
このとき、理由はよくわかりませんがアクセッサクラスというものが生成されて
それを検出するのが目的のようです。

FinalFieldCouldBeStatic

finalフィールドなのに static 宣言されていないフィールドを検出します。

CloseResource

閉じられていないDBリソース(接続やステートメント等)を検出します。

types

検出対象となるクラスを定義します。
デフォルトは Connection,Statement,ResultSet です。

NonStaticInitializer

staticでないイニシャライザを検出します。

public class Foo {
  
  static {
    // これは、staticイニシャライザ
    // クラスがロードされるときに一度だけ実行される
  }
  
  {
    // これが、staticでないイニシャライザ
    // インスタンスが生成される度に実行される
  }

}

これは正しい構文ですが、あまり使われない上に紛らわしいので
使わない方が良いでしょう。
インスタンス単位の初期化処理はコンストラクタ内に記述すれば良いのです。

DefaultLabelNotLastInSwitchStmt

default節がswicthブロックの最後以外に出現することを検出します。
default節はブロックの最後に記述した方が見やすいです。

NonCaseLabelInSwitchStatement

switchブロック内に、caseではないラベル文が出現することを検出します。

switch (x) {
  case 1:
    break;
  mylabel: // これは単なるラベルであり、紛らわしい
    ...
}

OptimizableToArrayCall

最適化されていない toArray メソッドの呼び出しを検出します。

x.toArray(new Foo[0]); // これでもいいが、後者の方が効率が良い
x.toArray(new Foo[x.size()]);

BadComparison

Double.NaN との比較箇所を検出します。

if (x == Double.NaN) {
  // この式は常にfalseを返す。正しくはDouble.isNaN(x)
}

EqualsNull

equalsメソッドの引数にnullを指定している箇所を検出します。

if (x.equals(null)) {
  // nullと同値になるxは存在しない。よってこのロジックは意味が無い
}

ConfusingTernary

紛らわしい if-else 文を検出します。

if (x != y) {
  diff();
} else {
  same();
}

これは以下のように記述した方が読みやすいです。

if (x == y) {
  same();
} else {
  diff();
}

InstantiationToGetClass

不必要な getClass() 呼び出しを検出します。

new Object().getClass(); // これは、次のようにシンプルに出来る
Object.class;

IdempotentOperations

意味の無いオペレーションを検出します。

x = x;

SimpleDateFormatNeedsLocale

SimpleDateFormatのインスタンス生成時にロケールが指定されていない箇所を検出します。

new SimpleDateFormat("pattern"); // ロケールが指定されていない
new SimpleDateFormat("pattern", Locale.US);

ImmutableField

変更されないフィールドを検出します。
通常これはfinalで宣言できます。

private int x; // このフィールドは final 宣言すべき
public Foo() {
    x = 7;
}
public void foo() {
   int a = x + 2;
}

UseLocaleWithCaseConversions

toUpperCase / toLowerCase 呼び出しに
ロケールが指定されていない箇所を検出します。

AvoidProtectedFieldInFinalClass

finalクラスに定義された protected フィールドを検出します。

AssignmentToNonFinalStatic

static フィールドへの代入を検出します。

private static int v;
public Bar(int v) {
  this.v = v; // この代入は安全ではない
}

MissingStaticMethodInNonInstantiatableClass

staticメソッドおよびpublicコンストラクタが存在しないクラスを検出します。
このクラスは外部から利用できません。

public class Foo {
  private Foo() {}
  void foo() {}
}

AvoidSynchronizedAtMethodLevel

メソッドレベルでの synchronized 構文の使用を検出します。
このようにすると、そのメソッドに処理が追加された場合に
それら全ても同期化されてしまいます。
出来る限り同期化の範囲を限定することを望むならば、これはあまり良くない事です。

synchronized void foo() { // メソッドレベルでの同期はNG
}
void bar() { // 以下のように、ブロックレベルでの同期はOK
  synchronized(this) {
  }
}

MissingBreakInSwitch

caseブロックの中でbreak文が記述されていない箇所を検出します。

UseNotifyAllInsteadOfNotify

notify() メソッドが使用されている箇所を検出します。
代わりに notifyAll() メソッドを使用しましょう。

AvoidInstanceofChecksInCatchClause

catchブロックの中でinstanceof演算子を使用している箇所を検出します。

try {
  ...
} catch (Exception e) {
  if (e instanceof IOException) {
    // このような振り分けは良くない
  }
}

try {
  ...
} catch (IOException e) {
  // このように振り分けた方が良い
}

AbstractClassWithoutAbstractMethod

abstractメソッドが存在しないabstractクラスを検出します。
このようなクラスはabstractにする必要がありません。

SimplifyConditional

instanceof演算子と同時にnullチェックを行っている箇所を検出します。

if (x != null && x instanceof String) {
  // instanceof演算子はxがnullでも正常に働くので、明示的なnullチェックは必要無い
}

CompareObjectsWithEquals

オブジェクトを == 演算子で比較しています。
オブジェクトの比較には equals メソッドを使用します。

PositionLiteralsFirstInComparisons

オブジェクトとリテラル(定数など)の比較時に、リテラルが左辺にない箇所を検出します。

if (str.equals("abc")) {
  // strがnullの場合に例外が発生してしまう
}

if ("abc".equals(str)) {
  // このようにすれば、strがnullでもOK
}

UnnecessaryLocalBeforeReturn

不必要なローカル変数宣言を検出します。

int x = getX();
return x; // これは return getX(); で良い

NonThreadSafeSingleton

スレッドセーフになっていないSingletonクラスを検出します。

private static Foo foo = null;
public static Foo getFoo() {
  if (foo == null) {
    foo = new Foo();
  }
  return foo;
}

このようにすると、同時に複数の Foo.getFoo() 呼び出しがあった場合に
それらは別々のインスタンスを返す可能性があります。
つまり、Singleton ではなくなってしまいます。
これを解決するには、上記ロジックを同期化するか foo を予め初期化しておきます。

checkNonStaticMethods

Singleton クラスに存在するstaticメソッドを検出します。

checkNonStaticFields

Singleton クラスに存在するstaticフィールドを検出します。

UncommentedEmptyMethod

コメントが付いていない空メソッドを検出します。

UncommentedEmptyConstructor

コメントが付いていない空コンストラクタを検出します。

AvoidConstantsInterface

いわゆる「定数インターフェイス」を検出します。

public interface IConstants {
  public static final int CONSTANT1 = 0; 
  // このようなインターフェイスは使用しない方が良い
}

UnsynchronizedStaticDateFormatter

static で定義された SimpleDateFormat を同期化して使用していない箇所を検出します。
このクラスはスレッドセーフではない為、本来はstaticで定義せずに
スレッド毎にインスタンスを作成するべきです。
複数のスレッドから同一のインスタンスを使用する場合は、
同期化する必要があります。

PreserveStackTrace

catchした例外のスタックトレースを捨てている箇所を検出します。

try {
   ...
} catch (IOException e) {
  throw new MyException(e.getMessage());
}

このようにすると、例外が発生した箇所がわからなくなってしまい
デバッグ作業の効率が非常に悪くなります。

UseCollectionIsEmpty

コレクションが空かどうかを判定するのに size() == 0 を使っている箇所を検出します。
Collectionには isEmpty() というメソッドがあるので、これを使いましょう。

if (list.size() == 0) ... // これはNG
if (list.isEmpty()) ... // これがOK

[コメント（0）]

Designルール