4

私はプロジェクトを持っています（グラフアルゴリズムに関連しています）。それは別の人によって書かれています。

コードは恐ろしいです：

  • public フィールド、getter/setter なし
  • 巨大なメソッド、すべてパブリック
  • 一部のクラスには 20 を超えるフィールドがあります
  • 一部のクラスには 5 つ以上のコンストラクターがあります (これも巨大です)。
  • これらのコンストラクターのいくつかは、多くのフィールドを残すだけですnull
    (そのため、いくつかのフィールドを final にすることはできません。これは、1 つおきのコンストラクターがエラーを通知するためです)。
  • メソッドとクラスは双方向で相互に依存しています

これをクリーンでわかりやすい API に書き直す必要があります。

問題は次のとおりです。私自身、このコードで何も理解していません。

そのようなコードを分析して理解するためのヒントを教えてください。

おそらく、静的コード分析を実行し、グラフなどを呼び出すツールがあるのではないかと考えていました。

4

10 に答える 10

8

ああ、親愛なる:-)私はあなたをうらやましく思いますが、同時にではありません。これらのことのいくつかは、コード分析ツールを緩める前に自分で取り組むことができます。このようにして、単純なツールを使用するよりも理解を深め、さらに先に進むことができます。

  • パブリックフィールド、ゲッター/セッターなし
    • すべてをプライベートにします。あなたのルールは、可能な限りアクセスを制限することです
  • 巨大な方法、すべて公開
    • 分割して、そうすることが理にかなっている場所でプライベートにする
  • 一部のクラスには20を超えるフィールドがあります
    • ugh..Effective Java 2nd EdのBuilderパターンは、このための主要な候補です。
  • 一部のクラスには5つを超えるコンストラクターがあります（これも巨大です）
    • 伸縮式コンストラクターのように聞こえますが、上記と同じパターンが役立ちます
  • それらのコンストラクターのいくつかは、多くのフィールドをnullのままにしました
    • うん、それは伸縮コンストラクターです:)
  • メソッドとクラスは、双方向で相互に依存しています
    • これは最も楽しいことではありません。継承が必要であることが完全に明確でない限り、継承を削除し、該当する場合はインターフェイスを介して代わりに構成を使用してください。

幸運を祈ります。

于 2010-03-21T21:15:14.013 に答える
6

おお！

私はお勧めします：ユニットテストを書いてからリファクタリングを始めてください

* public fields, no getters/setters

それらを非公開にすることから始めて、コンパイラエラーに対する耐性をメトリックとして「感じ」ます。

* huge methods, all public

それらのセマンティクスを理解し、インターフェースを導入してみてください

* some classes have over 20 fields

複雑なアプリケーションでは非常に一般的で、心配する必要はありません

* some classes have over 5 constructors (which are also huge)

それらをbuider/creatorパターンで置き換えます

* some of those constructors just left many fields null

上記の回答を参照してください

* methods and classes rely on each other in both directions

すべてを書き直すかどうかを決定します (正直なところ、コードの 10% しか必要としないケースに直面しました)

于 2010-03-21T21:04:04.363 に答える
3

さて、Eclipse のクリーンアップ ウィザードは、かなりの割合のスラッジをこすり落とします。

その後、十分に長く生きていれば、ソナーをそれに向けて、それが不平を言うすべてを修正することができます.

于 2010-03-21T21:00:11.620 に答える
2

IntelliJのように、リファクタリングについて何かを知っているIDEを使用してください。IntelliJは必要なすべての変更を行うのに十分賢いので、1つのメソッドを移動して他の5つのクラスが文句を言うような状況はありません。

ユニットテストは必須です。ユニットテストなしでリファクタリングする人は、セーフティネットのないハイワイヤーパフォーマーのようなものです。長くてハードな登りを始める前に入手してください。

于 2010-03-21T21:15:49.207 に答える
2

静的分析とコール グラフ (グラフィックではなくグラフ構造) の場合は、依存関係ファインダーを使用できます。

于 2010-03-21T21:06:08.857 に答える
1

これは私がそれをする方法です：

  1. 他のクラスで使用されているかのように、たとえばメソッド内からコードの使用を開始しmainます-同じ引数、同じ呼び出し順序。このクラスが行う各ステップを見るとわかるように、デバッガー内でこれを実行します。
  2. その機能の単体テストの作成を開始します。妥当な範囲に達すると、このクラスにはおそらく多くの責任があることに気付くでしょう。

while ( responsibilities != 1 ) {

  1. そのクラスの1つの責任を表すインターフェースを抽出します。
  2. すべての呼び出し元が具象型の代わりにそのインターフェースを使用するようにします。
  3. 実装を別のクラスに抽出します。
  4. 新しいインターフェイスを使用して、すべての呼び出し元に新しいクラスを渡します。

}

于 2010-03-21T21:14:07.247 に答える
1

ソナーやFindBugsなど、すでに言及されているツールは役に立たないとは言わないが、魔法のトリックはない。理解していることから始めて、その単体テストを作成し、グリーンが実行されたら、少しずつリファクタリングを開始します。進むにつれて、依存関係をモックすることを忘れないでください。

于 2010-03-21T21:14:44.623 に答える
1

答えは、忍耐とコーヒーかもしれません。

于 2010-03-21T21:08:13.933 に答える
0

ゼロから書き直す方が簡単な場合もあります。この「恐ろしいコード」は意図したとおりに機能していますか、それともバグだらけですか? それは文書化されていますか？

私の現在のプロジェクトでは、前任者の作品をほぼ完全に削除し、最初から書き直すことが最も効率的な方法でした。確かに、これはコードの難読化、意味のあるコメントの完全な欠如、および完全な無能の極端なケースであったため、マイレージは異なる場合があります.

于 2010-03-21T21:30:24.093 に答える
0

一部のレガシー コードはほとんど理解できない場合がありますが、それでも段階的にリファクタリングして読みやすさを向上させることができます。Joshua Kerievsky のRefactoring To Patterns本を見たことがありますか? ――これでいい。

于 2012-01-25T23:11:15.033 に答える