3

OK、PMDとFindBugsコードアナライザーでいくつかのコードをレビューした後、レビューしたコードに大きな変更を加えることができました。ただし、修正方法がわからないことがいくつかあります。以下でそれらを繰り返し、（より良い参考のために）各質問に番号を付けます。それらのいずれか/すべてに自由に答えてください。お待ち頂きまして、ありがとうございます。

1.いくつかのルールを削除したとしても、コードを再評価した後も、関連する警告は表示されたままです。なぜですか？

2.宣言を見てください：

    private Combo comboAdress;

    private ProgressBar pBar;

ゲッターとセッターによるオブジェクトへの参照：

    private final Combo getComboAdress() {
  return this.comboAdress;
 }

 private final void setComboAdress(final Combo comboAdress) {
  this.comboAdress = comboAdress;
 }

 private final ProgressBar getpBar() {
  return this.pBar;
 }

 private final void setpBar(final ProgressBar pBar) {
   this.pBar = pBar;
 }

さて、なぜ最初の宣言ではPMDに関する警告が表示されないのに、2番目の宣言では次の警告が表示されるのでしょうか。

Found non-transient, non-static member. Please mark as transient or provide accessors.

その警告の詳細については、こちらをご覧ください。

3.これもPMDによって与えられた別の警告です：

    A method should have only one exit point, and that should be the last statement in the method

その警告の詳細については、こちらをご覧ください。

今、私はそれに同意しますが、私がこのようなものを書いたらどうなりますか？

public void actionPerformedOnModifyComboLocations() {
    if (getMainTree().isFocusControl()) {
        return;
    }
    ....//do stuffs, based on the initial test
}

私はルールに同意する傾向がありますが、コードのパフォーマンスが複数の出口点を示唆している場合、どうすればよいですか？

4. PMDは私にこれを与えます：

Found 'DD'-anomaly for variable 'start_page' (lines '319'-'322').

私が次のようなものを宣言するとき：

String start_page = null;

nullへの割り当てを削除すると、この情報（警告のレベルはinfo）を削除しますが、コードの後半のある時点で、変数が初期化されていない可能性があるというエラーがIDEから発生しました。だから、私はそれに固執しています。警告を抑えることがあなたにできる最善のことですか？

5. PMD警告：

Assigning an Object to null is a code smell.  Consider refactoring.

これは、GUIコンポーネントのシングルトーン使用の場合、または複雑なオブジェクトを返すメソッドの場合です。catch（）セクションで結果をnullに割り当てると、不完全/一貫性のないオブジェクトが返されないようにする必要があるため、正当化されます。はい、NullObjectを使用する必要がありますが、それを実行したくない場合があります。その場合、その警告を抑制する必要がありますか？

6. FindBugsの警告＃1：

Write to static field MyClass.instance from instance method MyClass.handleEvent(Event)

メソッドで

@Override
public void handleEvent(Event e) {
    switch (e.type) {
        case SWT.Dispose: {
            if (e.widget == getComposite()) {
                MyClass.instance = null;
            }
       break;
         }
    }
}

静的変数の

private static MyClass instance = null;

この変数を使用すると、フォームがすでに作成されて表示されているかどうかをテストできます。場合によっては、フォームの再作成を強制する必要があります。ここに他の選択肢はありません。洞察はありますか？（MyClassはリスナーを実装するため、オーバーライドされたhandleEvent（）メソッドです）。

7. FindBugsの警告＃2：

Class MyClass2 has a circular dependency with other classes

この警告は、他のクラスの単純なインポートに基づいて表示されます。この警告を消すために、これらのインポートをリファクタリングする必要がありますか？または、問題はMyClass2に依存していますか？

OK、今のところ十分に言われています。より多くの発見やあなたの答えに基づいて、更新を期待してください。ありがとう。

4

3 に答える 3

2

  1. 何も思いつきません。あなたが何をしたとしても、それはあなたがやろうとしていたことではなかったようです！

  2. おそらく、宣言はSerializableクラスに表示されますが、その型は（たとえばComboProgress、それ自体はシリアル化できません）。これがUIコードの場合、それは非常に可能性が高いようです。クラスにコメントを付けて、シリアル化してはならないことを示します。

  3. これは有効な警告です。このようにして、コードをリファクタリングできます。 

    public void actionPerformedOnModifyComboLocations() {
    if (!getMainTree().isFocusControl()) {
        ....//do stuffs, based on the initial test
    }
}

  4. これが、静的分析ツールに耐えられない理由です。null割り当ては明らかにNullPointerException後であなたをsに開いたままにします。ただし、これが単に避けられない場所はたくさんあります（たとえば、を使用try catch finallyしてリソースのクリーンアップを行うために使用するCloseable）

  5. これも有効な警告のようであり、アクセスの使用は、ほとんどの開発者にとっておそらくコードの臭いstaticと見なされます。依存性注入を使用してリファクタリングを検討し、現在静的を使用しているクラスにリソーストラッカーを注入します。

  6. クラスに未使用のインポートがある場合は、これらを削除する必要があります。これにより、警告が消える可能性があります。一方、インポートが必要な場合は、次のような真の循環依存関係がある可能性があります。

    class A {
    private B b;
}
class B {
    private A a;
}

これは通常、混乱を招く状況であり、初期化の問題が発生する可能性があります。たとえば、インスタンスの初期化がA必要なコードを初期化に誤って追加する可能性Bがあります。同様のコードをに追加すると、循環依存関係は、コードが実際に壊れていることを意味します（つまり、またはBを作成できませんでした。AB

繰り返しますが、静的分析ツールが本当に好きではない理由を示しています。通常、静的分析ツールは、多くの誤検知を提供するだけです。循環に依存するコードは完全にうまく機能し、非常によく文書化されている可能性があります。

于 2009-10-17T16:08:57.523 に答える
2

これがあなたの質問のいくつかに対する私の答えです：

質問番号2：

プロパティを適切に活用していないと思います。メソッドはgetPBarおよびsetPBarと呼ばれる必要があります。

String pBar;

void setPBar(String str) {...}
String getPBar() { return pBar};

JavaBeans仕様には、次のように記載されています。

読み取り可能なプロパティの場合、プロパティ値を読み取るためのgetterメソッドがあります。書き込み可能なプロパティの場合、プロパティ値を更新できるようにするセッターメソッドがあります。[...] getFooおよびsetFooアクセサーメソッドを使用して、標準のJava規則に従うプロパティのPropertyDescriptorを構築します。したがって、引数名が「fred」の場合、readerメソッドは「getFred」であり、writerメソッドは「setFred」であると想定されます。プロパティ名は小文字で始める必要があることに注意してください。小文字はメソッド名で大文字になります。

質問番号3：

私はあなたが使用しているソフトウェアの提案に同意します。読みやすくするために、1つの出口点のみが適しています。効率を上げるために、「return;」を使用します。良いかもしれません。私の推測では、コンパイラは常に効率的な代替手段を選択するのに十分賢いので、バイトコードはどちらの場合も同じになると思います。

さらなる経験的情報

いくつかのテストを行ったところ、使用しているJavaコンパイラ（Mac OS X10.4ではjavac1.5.0_19）が期待した最適化を適用していないことがわかりました。

次のクラスを使用してテストしました。

public abstract class Test{

  public int singleReturn(){   
     int ret = 0;
     if (cond1())
         ret = 1;
     else if (cond2())
         ret = 2;
     else if (cond3())
         ret = 3;

    return ret;
  }

  public int multReturn(){
    if (cond1()) return 1;
    else if (cond2()) return 2;
    else if (cond3()) return 3;
    else return 0;
  }

  protected abstract boolean cond1();
  protected abstract boolean cond2();
  protected abstract boolean cond3();
}

次に、バイトコードを分析したところ、multReturn（）にはいくつかの'ireturn'ステートメントがありますが、singleReturn（）には1つしかありません。さらに、singleReturn（）のバイトコードには、returnステートメントへのいくつかのgotoも含まれています。

cond1、cond2、cond3の非常に単純な実装で両方のメソッドをテストしました。私は、3つの条件が等しく証明できることを確認しました。multReturn（）を支持して、3％から6％の時間の一貫した違いを見つけました。この場合、操作が非常に簡単なので、マルチリターンの影響が非常に目立ちます。

次に、異なるリターンの影響を目立たなくするために、cond1、cond2、およびcond3のより複雑な実装を使用して両方のメソッドをテストしました。結果にショックを受けました！現在、multReturn（）はsingleReturn（）よりも一貫して低速です（2％から3％の間）。残りのコードは同じである必要があるため、この違いの原因はわかりません。

これらの予期しない結果は、JVMのJITコンパイラが原因だと思います。

とにかく、私は私の最初の直感を支持します。コンパイラー（またはJIT）はこれらの種類のものを最適化でき、これにより開発者は読みやすく保守しやすいコードの記述に集中できます。

質問番号6：

インスタンスメソッドからクラスメソッドを呼び出し、その静的メソッドをそのままにしてクラス変数を変更することができます。

次に、コードは次のようになります。

public static void clearInstance() {
    instance = null;
}

@Override
public void handleEvent(Event e) {
    switch (e.type) {
        case SWT.Dispose: {
            if (e.widget == getComposite()) {
                MyClass.clearInstance();
            }
       break;
         }
    }
}

これにより、5で説明した警告が発生しますが、ある程度の妥協が必要です。この場合、これは単なる匂いであり、エラーではありません。

質問番号7：

これは単に起こりうる問題の匂いです。それは必ずしも悪いことでも間違っていることでもありませんし、このツールを使用するだけでは確信が持てません。

コンストラクター間の依存関係など、実際の問題がある場合は、テストで問題が示されるはずです。

別の、しかし関連する問題は、jar間の循環依存です。循環依存を持つクラスはコンパイルできますが、クラスローダーの動作方法により、JVMではjar間の循環依存を処理できません。

于 2009-10-17T16:32:20.207 に答える
1

ポイント3については、おそらく最近の開発者の大多数は、シングルリターンルールは単純に間違っていると言っており、平均してコードが悪化します。他の人は、それが歴史的な資格情報を備えた書き留められたルールであり、それを破るいくつかのコードは読みにくいので、それに従わないことは単に間違っていると見ています。

あなたは最初の陣営に同意しているようですが、そのルールをオフにするようにツールに指示する自信がありません。

覚えておくべきことは、チェックツールでコーディングするのは簡単なルールであり、それを望んでいる人もいます。したがって、ほとんどの場合、それらによって実装されます。

（もしあれば）より主観的なガードを強制するものはほとんどありません。体; 計算を返す; ' 一般的に、最も読みやすく、最も単純なコードを生成するパターン。

したがって、最悪のコードを単に回避するのではなく、優れたコードを生成することを検討している場合、それはおそらくオフにしたい1つのルールです。

于 2009-10-17T17:23:03.013 に答える