1. ホーム

新ソナーコードのレビュー課題のまとめ

2022-02-19 18:53:06

主な問題点の一覧です。

<ブロッククオート

形式:課題名+課題出現回数

リソースをクローズする必要があります 2

リソースが閉じられておらず、開くと、使用されているIOストリームのうち2つが閉じられていないことが判明する


条件は、無条件に "TRUE" または "FALSE" に評価されないようにします。 1

if/elseの判定に重複があり、例えばif(a>10)の実行体内で、if(a<0)と判定して、確実に真になることはないでしょう


例外ハンドラは元の例外を保持する必要があります 13

例外処理時には、catch(Exception e)ではなく、元の例外を保存する必要があります。


Throwable.printStackTrace(...)は呼び出されるべきではありません。 7

e.printStackTrace()を直接呼び出すのではなく、Loggerで処理する(つまりLogを叩く)必要があります。

Loggersの利点は、ユーザーが簡単にログを取得できることです。

ログメッセージの形式が統一されており、ユーザーが簡単にログを閲覧することができます。


インスタンスメソッドは、quot;static" フィールドに書き込まないようにします。 6

インスタンスメソッドで静的メンバを変更しない。理想的には、静的変数は同期化された静的メソッドによってのみ変更される。


public static"フィールドは定数であるべきです。 1

public staticなメンバはfinalに加えるべき、つまりpublic static finalは一般的に区別がつかない。


Thread.run() と Runnable.run() は直接呼ばないでください。 1

ThreadおよびRunnaaleオブジェクトのrunメソッドを直接呼んではいけません。runを直接呼び出すと、runメソッドが現在のスレッドで実行されてしまい、新しいスレッドを開くという目的を破ってしまいます。しかし、時にはそれが可能な場合もあり、以下にその例を示します。


一般的な例外は決して投げてはならない 1

Error,RuntimeException/Throwable/Exceptionのような汎用例外を直接投げてはいけないという趣旨のことが書いてあるのですが、よく理解できていません。私の具体的なアプリケーションは、throw new Error("Error copying database")で、与えられた助言は次のとおりです。一般的なものを使用する代わりに、専用の例外を定義して投げる(define and throw a dedicated exception instead of using a generic one)


クラス変数フィールドはパブリックアクセシビリティを持つべきではありません 64

クラス変数をpublicにするのではなく、privateにした上で、getとsetのメソッドを用意します。


コードのセクションはコメントアウトされるべきではありません。 30

コメント欄にたくさんのコードスニペットを書かないでください。


パッケージの宣言はソースファイルのディレクトリと一致する必要があります 19

これは理解できません。パッケージの宣言はソースファイルのディレクトリと一致させる必要があります。


ユーティリティクラスはパブリックコンストラクタを持つべきではありません。 16

ツールクラスはパブリックコンストラクタを持つべきではありません。つまり、少なくともプライベートコンストラクタを持ち、そうでない場合はデフォルトコンストラクタがパブリックであることを意味します。


ダイヤモンド演算子("<>")が使用されます。 12

コレクションを定義する場合、等号の右側の <> の中に要素タイプを書く必要はなく、空欄のままにしておきます。


ラムダと無名クラスは行数が多すぎない方が良い 9

ラムダ 式や匿名インナークラスはあまり多くの行を書かないようにし、通常は20行までとします。


メソッドを1つだけ含む匿名インナークラスはラムダになるべき 8

メソッドを1つだけ含む匿名インナークラスは、次のように記述します。 ラムダ コードの可読性を高めるための式


Try-with-Resourcesを使用する必要があります。 8

try/catch/finally 形式を、後で学習する Try-with-resources 形式に置き換える。


メソッドは空であってはならない 7

この場合を除き、空のメソッドを書いてはいけません。抽象クラスは、子クラスにデフォルトの実装を提供するために、空のメソッドを持つことができます。


ソースファイルに重複するブロックがないこと 7

ソースファイル内のコードスニペットや行、文字列などが重複しないようにする。理解できなかった。


switch case"節は行数が多くなりすぎないようにする。 6

switch case"  各ケース内のコードはあまり長くならないようにし、長すぎる場合は代わりにメソッドを書くことを検討してください(主にコードの可読性を高めるため)。


ネストされたコードブロックは、空のままにしてはいけません。 6

ネストされたブロックは空であってはいけません。例えば if( a > 0 ) { doSomething() } else { } のように、その後に else{} を削除する必要があります。


メソッドは複雑すぎない方がいい 6

メソッドを複雑にし過ぎると、理解やメンテナンスが難しくなります。


未使用のプライベートフィールドは削除する必要があります 5

プライベートに使用されないメンバー変数は削除する。


デッドストアを削除する必要があります 5

ローカル変数など、使わないデッドストレージは削除すること。つまり、後でまったく使わないことが判明した変数を定義するメソッドを書いたら、その行を削除することを忘れないこと。


switch"文は、"default"節で終了する必要があります。 4

switch文はdefaultで終わるべきで、これはプログラミングの防御的な考え方である


未使用のメソッドパラメータは削除する必要があります 4

使用されないメソッドパラメータは削除されるべきである


制御フロー文 "if", "for", "while", "switch" and "try" はあまり深くネストしないでください。 4

if /for/while/tryのようなネストをあまり複雑にしないでください。


誤解を防ぐために、式の周りの無駄な括弧を削除すること。 3

誤解を招かないように、意味のない括弧をつけるだけでなく、オブジェクトの型が左右で同じ場合は"="のように、強く回さないようにしましょう。


ループ停止条件は不変であるべきです。 3

forループの結果条件は、変数ではなく、定数でなければなりません。


static" メンバは静的にアクセスされる必要があります。 2

static メンバは、クラスおよび静的メソッドに関連付けられます。


キャッチは結合されるべきである 2

具体的には以下の18を参照、まだ理解できていません。


プリミティブは、quot;String" 変換のためだけにボックス化されるべきではありません。 2

4+" "のようにint値を文字列に変換する代わりに、Integer.toString(4)のように使用します。

Integer.parseInt("I am string")と同じで、怠慢は禁物です。


クラスは空であってはならない 2

空のクラスを書かない


未使用のローカル変数を削除する必要があります 2

使用しないローカル変数は削除する


キーと値の両方が必要な場合は、quot;entrySet()" を反復する必要があります。 2

英語を直接見たほうがわかりやすいですね。ループの中でマップのキーだけが必要な場合、keySetを反復するのは理にかなっています。 代わりにentrySetを反復するのが効率的で、キーと値の両方にアクセスすることができます。

つまり、Mapのキーだけが必要な場合はMapのkeySetを繰り返し、キーと値の両方が必要な場合はMapを繰り返し処理するのが効率的です。19を参照。


メソッドパラメータ、キャッチした例外、foreach 変数は再割り当てしないでください。 2

method-parameters/caught-exceptions/ です。 foreach 変数を再代入してはいけません。


Collection.isEmpty() は、空っぽかどうかをテストするために使用されます。 2

コレクションが空かどうかを判断する場合、if (myCollection.size() == 0) の代わりに if (myCollection.isEmpty()) を使用すると、よりパフォーマンスが高くなります。


標準出力は、何かを記録するために直接使用しないでください。 2

標準出力は直接何も出力しません。つまり、ログを入力するときは、System.out.println("My Message")などではなく、logger.log("My Message")などで出力してください。


一般的なワイルドカードタイプはリターンパラメータに使用しないでください。 1

ワイルドカードは、return 宣言の中に入れてはいけません。例えば、こんな文章。リスト <? extends Animal>getAnimals(){...}. 犬や猫などを追加できるかどうか、知る由もありませんし、後でメソッドを使用するときに(引用符の内側で)考える必要もありません。


同期クラス Vector、Hashtable、Stack および StringBuffer は使用しないでください。 1

同期型Vector/を使用しない HashTable/Stack/StringBufferなど。初期のころはスレッドセーフの問題から ジャワAPI  はこれらのクラスを提供しました。しかし、同じスレッドで使用する場合でも、同期がパフォーマンスに大きな影響を与えることがあります。

と置き換えることができる場合が多い。

の代わりにArrayListまたはLinkedListを使用します。  ベクター

の代わりにDequeを使用します。  スタック

の代わりにHashMapを使用します。  ハッシュテーブル

の代わりにStringBuilderを使用します。  文字列バッファ

終了メソッドを呼んではいけない

system.exit()メソッドを呼び出さないようにする。


ローカル変数を宣言し、すぐに返したり投げたりしてはいけません。 7

代入後、ローカル変数の代入文を直接返す。


フィールド名は命名規則に従っている必要があります 6

ネーミングを規則正しくする


ローカル変数とメソッドのパラメータ名は命名規則に従っている必要があります。 6

ネーミングを規則正しくする


文字列リテラルは重複しないようにする。 5

文字列は繰り返さないようにし、同じ文字列を複数回使用する場合は、文字列定数として定義した上で参照することを推奨します。


boolean式の返り値は、"if-then-else"ステートメントにラップしてはいけません。 3

if ( a > 4 ) { return false } else { return true } のようなコードを書くのではなく、return a > 4 と書くだけでよいのです。


静的な非終端フィールド名は、命名規則に従うべきである 2

ネーミングを規則正しくする


修飾語は正しい順序で宣言されなければならない 2

修飾語などは、static publicの代わりにpublic staticのように、取り決めた順序で記述します。 


インターフェース宣言やクラスのメンバは、あらかじめ決められた順序で表示される必要があります。 2

前の質問と同様に、Oracleが定義したJavaコードの仕様によると、異なるコードの発生位置は以下のようになるはずです。

クラスとインスタンス変数--コンストラクタ--メソッド


配列の指定子 "[]" は、変数ではなく、型に指定する必要があります。 2

配列の括弧は変数の後ではなく、型の後に書きます。


複数の変数を同じ行で宣言してはならない 1

同じ行で複数の変数を定義しない


switch"文は、少なくとも3つのcase"節を持つ必要があります。 1

少なくとも3つ以上のケースがある場合はswitchを検討し、それ以外の場合はif/else形式を使用します。


メソッドのオーバーライドは、単にスーパークラスの同じメソッドを呼び出すだけでなく、もっと多くのことを行う必要があります。 1

サブクラスで親のメソッドをオーバーライドしているのだから、そのメソッドで親のメソッドと異なることをすればいい、そうでなければオーバーライドする必要はない。


ステートメントは別行動にする必要があります 1

if(someCondition) doSomething(); のようなコードを同じ行に書くのではなく、次のように記述します。

if(someCondition) { <未定義

doSomething()

}


メソッド名は命名規則に従っている必要があります 1

ネーミングを規則正しくする


TODOタグを処理する必要があります。     TODOタグは適時処理すること、やるべきことを忘れないこと


一部のルールの詳細説明

1. インターフェース宣言またはクラスのメンバは、あらかじめ定義された順序で表示されなければならない


正しい順番は、静的メンバ変数 → メンバ変数 → コンストラクタ → メソッドです。

public class Foo{ <未定義

public static final int OPEN = 4; //クラス変数とインスタンス変数

private int field = 0;

public Foo() {...}.    /Constructors(コンストラクタ

public boolean isTrue() {...}.    //メソッド

}

2. ダイヤモンド演算子("<>")を使用する必要があります。

コンプライアンス違反のコード例

List<String> strings = new ArrayList<String>(); // 非対応

Map<String, List<Integer>> map = new HashMap<String, List<Integer>>(); // 非適合

適合する解決策:仕様例

List<String> strings = new ArrayList<>()。

Map<String, List<Integer>> map = new HashMap<>();

3. コードのセクションはコメントアウトされるべきではありません。

コードのセクションをコメントで囲むべきではありません。

プログラマはコードをコメントアウトしてはいけません。プログラムを肥大化させ、可読性を低下させるからです。

未使用のコードは削除し、必要であればソース管理履歴から取得できるようにします。

4. ユーティリティクラスはパブリックコンストラクタを持つべきではない

ユーティリティクラスはパブリックコンストラクタを持つべきではない。つまり、ユーティリティクラスは少なくとも1つの非パブリックコンストラクタを定義すべきである。

静的メンバの集合体であるユーティリティクラスは、インスタンス化されることを想定していません。拡張可能な抽象的なユーティリティクラスであっても、パブリックコンストラクタを持つべきではありません。

Javaでは、少なくとも1つのコンストラクタを明示的に定義していないクラスには、暗黙のうちにパブリック・コンストラクタが追加されます。したがって、少なくとも1つの非パブリックコンストラクタを定義する必要があります。

<ブロッククオート

class StringUtils { // 非準拠のコード例

    public static String concatenate(String s1, String s2) { }. <未定義

          s1 + s2 を返します。

    }

}

<ブロッククオート

class StringUtils { /Compliantソリューション

    private StringUtils() { <未定義

    }

    public static String concatenate(String s1, String s2) {. <未定義

    s1 + s2 を返します。

    }

}

5. public static"フィールドは定数であるべきです。

パブリックな静的メンバはfinalで修正する

フィールドを "public" と "static" と宣言し、さらに "final" と宣言しない正当な理由はありません。ほとんどの場合、これは複数のオブジェクトの間で状態を共有するためのごまかしです。しかし、この方法では、どのオブジェクトも共有された状態に対して好きなことができます。しかし、この方法では、どのオブジェクトも共有された状態に対して好きなことができます。

public static Foo foo = new Foo();//unstandardized

public static final Foo FOO = new Foo();//canonical

6. クラス変数フィールドはパブリックアクセシビリティを持つべきではない

public class MyClass { <未定義

public static final int SOME_CONSTANT = 0; // 準拠 - 定数はチェックされません。

public String firstName; // 非対応

}

<ブロッククオート

public class MyClass { <未定義

public static final int SOME_CONSTANT = 0; // 準拠 - 定数はチェックされません。

private String firstName; // 準拠

パブリック String getFirstName() { <未定義

firstName を返します。

}

public void setFirstName(String firstName) { (パブリックボイドセットファーストネーム) <未定義

this.firstName = firstName;

}

}

7. 静的な非終端フィールド名は、命名規則に従うべきである

<ブロッククオート

public final class MyClass {//Noncompliantコード例

      private static String foo_bar;

}

class MyClass {//Compliant Solution

private static String fooBar;

}

8. "switch"文は、少なくとも3つの"case"節を持つべきである。

3つ以上のケースがある場合のみswitchを使用し、それ以外はif/elseを使用します。

switch文は、同じ式の値によって異なるケースが多数存在する場合に有効です。

しかし、1つか2つのケースだけなら、if文の方が読みやすいコードになります。

9. 文字列リテラルは重複してはいけない

prepare("action1"); // 非準拠 - "action1" が 3 回重複している。

execute("action1")を実行します。

release("action1")を実行します。


private static final String ACTION_1 = "action1"; // 適合しています。

prepare(ACTION_1); // 準拠

execute(ACTION_1) を実行します。

release(ACTION_1);

10. ブール式のリターンは、"if-then-else"文でラップしてはいけません。

if (expression) {//Noncompliantコード例

      をtrueで返す。

} else { <未定義

     はfalseを返します。

}


return expression;/Compliant Solution

return ! !式を返します。

11. メソッドパラメータ、キャッチした例外、foreach 変数は再代入してはいけません。

メソッドパラメータ、キャッチした例外、foreach 変数の再代入はできません。


class MyClass {//Noncompliantコード例。コンプライアンス違反のコード例

    public String name;

    public MyClass(String name) { { <未定義

            name = name; // 非準拠 - 無駄な ID の割り当て

    }

    public int add(int a, int b) {. <未定義

        a = a + b; // 非準拠

        return a; // パラメータがそのまま返されるようですが、何か意味があるのでしょうか?

   }

    public static void main(String[] args) { { <未定義

        MyClass foo = new MyClass();

        int a = 40です。

        int b = 2;

        foo.add(a, b); // 変数 "a" はこの呼び出しの後でも 40 を保持します。

    }

}


class MyClass {//Compliant Solution: canonical code example

    public String name;

    public MyClass(String name) { { <未定義

         this.name = name; // コンプライアンス

    }

    public int add(int a, int b) {. <未定義

        return a + b; // コンプライアンス

    }

    public static void main(String[] args) { { <未定義

    MyClass foo = new MyClass();

        int a = 40です。

        int b = 2;

        foo.add(a,b)を実行します。

     }

}

12. ローカル変数を宣言して、すぐに戻したり投げたりしてはいけません。

コンプライアンス違反のコード例 コンプライアンス違反のコード例

public long computeDurationInMilliseconds() { }. <未定義

long duration = (((時間 * 60) + 分)) * 60 + 秒 ) * 1000 ;

は持続時間を返します。

}

public void doSomething() { <未定義

RuntimeException myException = new RuntimeException()。

myException をスローします。

}


準拠ソリューション:カノニカルコードの例

public long computeDurationInMilliseconds() { }. <未定義

return (((時間 * 60) + 分)) * 60 + 秒 ) * 1000 ;

}

public void doSomething() { <未定義

新しいRuntimeException()をスローします。

}

13. Thread.run()とRunnable.run()は直接呼んではいけない

Thread.run() とRunnable.run() メソッドの目的は、コードを別の専用スレッドで実行することです。 は、そのコードが現在のスレッドで実行されるようになるので、意味があります。

ThreadとRunnableの中のrunメソッドは、runメソッドの中のコードを別のスレッドで実行できるようにするためのものです。runメソッドを直接呼び出すと、runメソッド内のコードが現在のスレッドで実行され、その意味が失われてしまいます

コンプライアンス違反のコード例 コンプライアンス違反のコード例

Thread myThread = new Thread(runnable);

myThread.run(); // 非対応


適合する解決策:正規のコードの例

Thread myThread = new Thread(runnable);

myThread.start(); // 準拠

この部分は個人的なものなので、省略可能です。

しかし、Runnableのrunメソッドが直接呼ばれるケースも存在する

次の postTaskSafely メソッドにより、タスクは常にメインスレッドで実行されるようになります。

public static void postTaskInMainThread(Runnable task) { (パブリックスタティックボイドポストタスクインメインスレッド) <未定義

     int curThreadId= android.os.Process.myTid();// カレントスレッドのIDを取得する

    if(curThreadId==getMainThreadId()) {/// もし現在のスレッドがメインスレッドである場合

            task.run();//直接実行

    }else{// カレントスレッドがメインスレッドでない場合

        getMainThreadHandler().post(task);// メインスレッドのHandlerを使用して投稿します。

}

ラムダと匿名クラスは行数が多すぎてはいけない

匿名クラスとラムダ(Java 8)は、専用のクラスを作成することなく振る舞いを注入するための非常に便利でコンパクトな方法です。匿名インナークラスとラムダは、注入される振る舞いが数行のコードで定義できる場合にのみ使用すべきです。

匿名クラス 行数:最大20行

15.リソースは閉じるべきである。閉じるべきものは常に閉じることを忘れない

Java のガベージコレクションは、すべてをクリーンアップするために頼ることはできません。特に、コネクション、ストリーム、ファイル、その他Closeableインターフェースやその上位インターフェースであるAutoCloseableを実装するクラスは、作成後に手動で閉じる必要があります。

<ブロッククオート

コンプライアンス違反のコード例 コンプライアンス違反のコード例

    OutputStream stream = null。

    try{ <未定義

        for (文字列プロパティ : propertyList) {. <未定義

        stream = new FileOutputStream("myfile.txt"); // 非対応

        // ...

        }

    }catch(Exception e){ <未定義

        // ...

    }finally{ <未定義

        stream.close(); // 複数のストリームがオープンされましたが、最後のストリームだけがクローズされます。


準拠ソリューション:正規化コードの例

    OutputStream stream = null。

    try{ <未定義

        stream = new FileOutputStream("myfile.txt");

        for (String property : propertyList) { {... <未定義

            // ...

        }

   }catch(Exception e){ <未定義

        // ...

   }finally{ <未定義

       stream.close()を実行します。

   }

16. 例外ハンドラは元の例外を保持すべき

コンプライアンス違反のコード例 コンプライアンス違反のコード例

// 非準拠 - 例外は失われる

try { /* ... */ } catch (Exception e) { LOGGER.info("context"); }.

// 非準拠 - 例外は失われる (メッセージのみが保存される)

try { /* ... */ } catch (Exception e) { LOGGER.info(e.getMessage()); }.

// 非準拠 - 例外は失われる

try { /* ... */ } catch (Exception e) { throw new RuntimeException("context"); }.


コンプライアンス・ソリューション 仕様のコード例

try { /* ... */ } catch (Exception e) { LOGGER.info(e); }.

try { /* ... */ } catch (Exception e) { throw new RuntimeException(e); }.

トライ { <未定義

/* ... */

} catch (RuntimeException e) {. <未定義

doSomething()です。

を投げる。

} catch (Exception e) { <未定義

// チェックされていない例外への変換も可

新しいRuntimeException(e)をスローします。

}

17. キャッチを合体させるべき

Java 7以降、複数の例外を一度にキャッチすることができるようになりました。したがって、複数のキャッチブロックが同じコードを持つ場合、それらを結合する必要があります。 したがって、複数のキャッチブロックが同じコードを持つ場合、それらを結合して読みやすくする必要があります。

注意事項  このルールは、プロジェクトの sonear.java.source が 7 未満の場合、自動的に無効になることを示します。

<ブロッククオート

コンプライアンス違反のコード例 コンプライアンス違反のコード例

catch (IOExceptionのe) {。 <未定義

    doCleanup()を実行します。

    logger.log(e);

}catch (SQLException e) { //非対応

    doCleanup()を実行します。

    logger.log(e);

 }catch (TimeoutException e) { // 準拠、ブロックの内容は異なる。

     doCleanup()を実行します。

     を投げる。

 }


準拠ソリューション:canonicalコードの例

catch (IOException|SQLExceptionのe) {。 <未定義

    doCleanup()を実行します。

    logger.log(e);

 }catch (TimeoutException e) {. <未定義

    doCleanup()を実行します。

    を投げる。

}

18."entrySet()" は、キーと値の両方が必要な場合に繰り返し実行される必要があります。

コンプライアンス違反のコード例 コンプライアンス違反のコード例

public void doSomethingWithMap(Map map) { }. <未定義

for (String key : map.keySet()) { // 非対応。

オブジェクトの値 = map.get(key)。

// ...

}   

}   


準拠ソリューションL:仕様コード例

public void doSomethingWithMap(Map map) { { <未定義

for (Map.Entryエントリ : map.entrySet()){。 <未定義

文字列キー = entry.getKey()。

Object value = entry.getValue();

// ...

}   

}   


本文/マクスウェル(ジェーンの作者)
元記事へのリンク: http://www.jianshu.com/p/b50f01eeba4d