Spotbug 共通のエラーに関する考察

前文です。

ソフトウェア構築の実験は、デバッグのためのアイデアのサードパーティ製のプラグインを使用する必要があります 、デバッグ一般的に使用されるプラグインは2種類です：findbugとspotbug、どのspotbugもっと包括的に、著者のプログラミングに遭遇する様々な小さなエラーの次の要約を。(包括的ではありませんが、すべてのエラーは、実験に書かれている）。

I. 規範的な誤り

悪い習慣

1. 紛らわしいメソッド名 :



このエラーは、プログラム中のクラス名、ドメイン名、メソッドの命名に誤りがあることを示します（多くの場合、大文字と小文字が区別されます）。



(1) クラス名の最初の文字を大文字にする。



(2) クラス内のメソッドの頭文字は小文字にし、名称は toString(), getDistance(), getName() など、シンプルで直感的な動詞で表現し、メソッドの目的を表すことが望ましい。



(3) ドメイン名（Field）の最初の文字は小文字にする。

equals() メソッドは null 引数をチェックしない。



javaを学び始めた頃、いつもこの間違いを犯していました。nullポインタの場合を考えずに直接比較するようにequalsメソッドを書き換えたとき、nullポインタのコードはまずnullポインタかどうかの判定をしなければ、nullポインタが渡された時点でfalseを返すのではなく直接プログラムがクラッシュします。

3. AirplaneのEqualsメソッドは、引数がso-and-so型であると仮定します。



これは、上記のケースと同様にイコールで発生する悪い癖です。比較を行う際に、比較対象のオブジェクトに直接対応するObjectを強制することは、以下の例のようにクラスとそのサブタイプを直接想定できないため、間違っているのです。

@Override
public boolean equals(Object that){
    Airplane thatPlane = (Airplane) that;
   if(this.Name.equals(thatPlane.getName()){
        return true;
    }
    else return false;
}

上の図には2つのバグがあります。1つは前述のようにヌルポインタの判定ができていないこと、もう1つは型の判定ができていないことです。つまり、変換ができると決めつけるのではなく、入力されるオブジェクトが比較対象のオブジェクトなのか、比較対象のオブジェクトのサブタイプなのかを判定しなければならないのです。



そのため、修正したコードは次のようになります。

@Override
public boolean equals(Object another) {
   if (another == null)
      return false;
   if (! (another instanceof Plane))
      return false;
   Plane ap = (Plane) another;
   if (ap.getPlaneNo().equals(this.getPlaneNo()))
      return true;
   return false;
}

(まずヌルポインタを判定し、次にinstanceofで型を判定します)。



4. クラス名 + ストリームのクローズに失敗する可能性があります。



これは一般に、プロンプトが表示されるクラスまたはメソッドが IO ストリームオブジェクトを作成する場合です (例: FileReader、BufferedReader など)。これは、ファイル記述子のリークにつながる可能性があります。通常、最終的にブロックを使用して、ストリームが閉じられることを確認するのが最善です。



ほとんどの場合、プログラムに影響はありませんが、spotbugはこのようなエラーを拾ってしまうので、ReaderなどのIOストリームオブジェクトの読み込みが完了したら、close文を入れることをお勧めします。

FileReader reader = new FileReader("path");
BufferedReader br = new BufferedReader(reader);
// read when done reader.close(); br.close();

II. エラーコード(DodyCode)

1. あるケースが次のケースにフォールスルーするSwitchステートメントが見つかりました。



switch文で、次のケースに直結するbreakを書き忘れ、2回連続で実行されてしまったケースです。

2. デフォルトのケースがないSwitch文が見つかった



switch文は様々なケースの発生を考慮し、どのケースも発生しない場合はデフォルトに移行して処理する必要があります。



エラーコード

switch(op){ 
	case 1: System.out.println("Can't change to this state");
	case 2: plans.get(i).Allocate();break;
	case 3: plans.get(i).Run();break;
	case 4: plans.get(i).Finish();break;
	case 5: plans.get(i).Cancel();break;
	case 6: plans.get(i).Block(); break;
}

状態遷移関数を書くときにbreakを書き忘れるケース1はない。また、switch文のデフォルトケースに対する処理もない。

3. 疑問のあるforループ



通常は設計上の問題か、2レベルのforループのtypo（実際のプログラミングではtypoがほとんどです）



例えば



for-loopのインクリメントが複雑、微妙、または間違っている。



for (int i = 0; i < entries.size() - 1; i++)



for (int j = i + 1; j < entries.size(); i++)



このエラーは

4. readLine() の結果の即時再参照



一般的なjavaでファイルを読むには、2つの方法があります。



スキャナとBufferedReaderでステートメントを読み込む。



しかし、リーダーを使用する場合は、次の理由から、最初に入力が空であるかどうかを判断するために取得する必要がありました。



BufferedReader は通常、テキスト行を読み取る際に .trim() メソッドと組み合わせて使用されます。これは、行の先頭と末尾のスペースを削除することを意味します。



ただし、以下のように直接使用するとエラーが報告されます。

String Name;
Name = reader.readLine().trim();

これは、Nameの入力が空の場合、先頭と末尾のスペースを削除するためにtrim()メソッドを呼び出すと、ヌルポインタのためにプログラムが直接クラッシュしてしまうからです。



そこで、解決策として、読み込んで、入力が空かどうかを判断し、先頭と末尾のスペースを削除することにしました。

String Name = reader.readLine();
if(Name== null){
    System.out.println("Read in cannot be empty");
    return false;
}
Name = Name.trim();

5. ローカル変数へのデッドストア または



フィールドをシャドウするローカル変数へのデッドストア



このエラーは一般的に、プログラムの実行に使用されていない無駄なコードがプログラム内に存在することに対応しており、デバッグプログラム内で生成しやすく、削除し忘れてしまうことがよくあります。コードを直接削除するのは簡単で、プログラムには何の影響もありません。



未読欄



このエラーは上のエラーと同じで、プログラムには影響しませんが、どうでもいい余分なコードがあるので、それを削除してください。

III. 潜在的な性能上の問題（パフォーマンス）

1. ループ内で+を使用して文字列を連結するメソッド



toString() メソッドをオーバーライドする場合、2つ以上の文字列を結合する必要があることが多く、 "+" で直接追加するとプログラムが遅くなるため、 spotbug では StringBuilder と append(), substring() メソッドを併用して、文字列結合するよう推奨しています。



インスタンス

String s = "";
for (int i = 0; i < field.length; ++i) {
    s = s + field[i];
}

上記の代わりに次のように記述する必要があります。

StringBuffer buf = new StringBuffer();
for (int i = 0; i < field.length; ++i) {
    buf.append(field[i]);
}
String s = buf.toString();

2. 未読欄



このエラーは、上記のエラーと同じです。デバッグの過程で、プログラムの実行に影響を与えない無駄なコードがある場合がありますが、どうでもいい余計なコードがあるので、それを削除すればいいのです。



3. プリミティブをパースするためのボックス化/アンボックス化



エラーです。文字列からボックス化されたプリミティブを作成することは、ボックス化されていないプリミティブの値を取り出すだけです。静的な parseXXX メソッドのみを呼び出す方が効率的です。



これは、以下の理由により、一般にValueOfの不適切な使用となります。



Integer.parseInt(s) の効果は、文字列 s を符号付き int 基底型にパースすることです。



Integer.valueOf(s) は文字列 s を Integer オブジェクト型にパースし、返された整数はそのオブジェクトのメソッドを呼び出すことができます。

String x = "1";
test.setX(Integer.valueOf(x)); // setX() takes an argument of int

コード中の Integer.valueOf() は String - int - Integer - int の処理を経て、Integer.caseInt() で int を Integer に保存する処理です。



に修正する必要があります。

String x = "1";
test.setX(Integer.parse(x));

IV. クロスプラットフォームエラーの可能性

1. デフォルトエンコーディングへの依存を発見



読み込み/書き込みのメソッドを使用する場合。FileReader、BufferedReader、FileWriter FileReader などはエンコードされていないので、うまく指定しないとシステムのデフォルトエンコードになるが、一度使うとシステムのデフォルトエンコードは予測できないので、技術的負債になる



このエラーを回避したい場合は、エンコーディングを指定する必要があります。

BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));

例えば、BufferedReaderは直接エンコードすることができます。

BufferedReader reader = new BufferedReader(new InputStreamReader(System.in,"UTF-8"));~

FileReaderの場合、エンコーディングは直接設定するか、以下のようにBufferedReaderで定義することが可能です。

BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream("path"),"UTF-8"));

以上、spotbugセッションでソフトウェア構築の執筆実験で遭遇したエラーをまとめてみました：spotbugって本当にいいものですね～～～、自分のコードが標準化されておらず不注意だったことに気づかされました