リファクタリング: このクラスは大きすぎます

実際の(欠陥のある)コードベースからのリファクタリングの例。

この記事では、実際のコードベースからのリファクタリングのセットを紹介します。これは完璧さを示すことを意図したものではありませんが、現実を表しています。

2020年4月14日


Photo of Clare Sudbery

ライター、講演者、質問者、そしてエクストリームプログラミングの熱心な支持者 - クレアは元高校数学教師であり、20年のソフトウェアエンジニアリング経験を持つコンサルタントです。彼女は、あらゆる場所の人々、特に従来ギークアウトすることを奨励されてこなかった人々の内なるギークを目覚めさせるという使命を帯びています。


これはリファクタリングについての物語です。TDDのレッド・グリーン・リファクターサイクル[1]の3番目の項目であり、私たちが常に行っていることです。そうではない場合を除いて。

私には、リファクタリングが疎かになっている手に負えないコードベースがあるので、それを修正しています。この記事では、_大きすぎるクラス_を取り上げて、小さくしていきます。

問題の概要

この物語は、退屈な家事から始まります。私は個人会計ソフトウェア、Reconciliateを作成しました。これはコマンドライン上で動作し、以下のアクションを実行します。

  1. 独自のカンマ区切りデータを読み込みます
    • 最近の銀行取引とクレジットカード取引の記録です。
    • (中央スプレッドシートのデータに基づく)毎月の予測取引と年間の予測取引。
    • 以前に記録された未調整のデータ。
  2. サードパーティのカンマ区切りデータ(銀行やクレジットカード会社から)を読み込みます。
  3. サードパーティのデータを独自のデータと照合します。
  4. すべてを中央スプレッドシートに書き戻します。

修正が必要なバグと、追加する機能がいくつかあります。しかし、私の典型的なワークフローは、末っ子を寝かしつけた後、寝るまでのわずかな時間しかなく、最後にコードを見てから数週間経っていることが多い深夜にラップトップの前に座ることです。このような状況では、「このコードはめちゃくちゃなのは分かっているけど、今はそれを理解して修正する時間がない…」と考えてしまうのは、非常に簡単です。

明らかに、これは理想的ではありません。

特に1つのクラス、`ReconciliationIntro`クラスは、見るたびに頭痛の種になっています。それは肥大化し、複雑で、_頭の中で理解する_ [2]ことができません。これは厄介なフィードバックループを作り出してきました。「このコードは非常に分かりにくいため、リファクタリングには時間とエネルギーがかかりすぎるので、もっと長く我慢します。たとえ、コードの現在の状態を理解し、変更をどこに加えるべきかを判断するのに時間がかかりすぎて、小さな変更さえできなくなるとしてもです。」

例えば、別のクレジットカードを処理する機能を追加したいと考えています。多くの場所で、ポリモーフィズムストラテジーパターンを使用して、各クレジットカードの独自の動作をきちんとカプセル化しています。しかし、`ReconciliationIntro`クラスは、コードの設計が貧弱で、明確な関心の分離がないため、別のクレジットカードを追加すると、すでに肥大化したクラスがさらに悪化してしまう場所の1つです。4種類のデータ(銀行入金、銀行出金、クレジットカード1、クレジットカード2)それぞれに4つの重複したコードパスが含まれており、今すぐクレジットカード3を追加すると、同じアンチパターンに従うことになります。

私の全体的な目標は、ストラテジーパターン[3]を介して、このコードをより汎用的なコードに置き換えることです。しかし、私の前に立ちはだかる4つの問題があります。

  1. この1つの大きなクラス(`ReconciliationIntro`)は、責任が多すぎます。
  2. パブリックインターフェースがないため、単体テストが難しいプライベートなネストされたコードがたくさんあります。
  3. `ReconciliationIntro`内には、多すぎることをしている1つの大きなメソッドがあります。
  4. `ReconciliationIntro`には、同じ繰り返しパターンを使用しているが、詳細が異なるメソッドがいくつかあります。

私は、上記の順序で、これらの問題をすべて解決する予定です。この準備リファクタリングにより、各クレジットカード/アカウントの動作を簡単にカプセル化できるようになります。ケント・ベックが言うように、「変更を簡単にしてから、簡単な変更を行う」のです。

この記事は、上記のリストの最初の問題、_このクラスは大きすぎる_、に取り組むことについてです。

なぜリファクタリングするのか?

`ReconciliationIntro`クラスは責任が多すぎるため、簡単に理解できません。もともとはソフトウェアのエントランスホールとして設計されており、いくつかのメッセージを表示してから、主要な作業を行うクラスをインスタンス化するだけでした。しかし、時間の経過とともに、他の多くのコードが忍び込んできました。別のクレジットカードを追加しやすくしたいので、_この大きなクラスを小さなクラスに分割する_ことから始めます。

メリットはいくつかあります

  • メソッドのグループを別のクラスに移動することで、明確なコンテキストを作成し、密結合を最小限に抑えます。これは、
    • 変更を加えたい場合や問題を修正したい場合に、どこに行くべきかが分かるということです。
    • コードを修正する場合、頭の中で理解できる[2]小さな独立したセクションだけで作業すればよいので、システム全体を理解する必要はありません。
    • 状態の操作は、小さなコンテキストでのみローカルに行われるため、システムのある領域が別の領域の状態を変更することについて心配する必要はありません。

どのように行うのか?

私の考えでは、リファクタリング(および新しいコードを書く)際の最も重要な原則は、小さなステップで進むことです。私には、いくつかの関連する目標があります。

  • すべてのステップで、コードをコンパイルし、テストを実行できるようにしたいと考えています。
  • 変更によってテストが失敗した場合、すぐに修正できるようにしたいと考えています。小さな変更と小さなコミットを行うことで、どの変更がテストの失敗を引き起こしたかを確認でき、問題を見つけるために巻き戻したり、少量のコードを調べたりするだけで済みます。
  • 自分がどこにいるかを頭の中で追跡できます。比較的簡単なリファクタリングに着手したものの、当初の意図を超えた影響があることに気付くのは、非常に簡単なことです。この時点で、コードをコンパイルし、テストをすべてのステップで合格させる習慣がない場合、抜け出すのが難しいウサギの穴に迷い込んでしまう可能性があります。コードはコンパイルされず、テストは実行されません。ましてや合格することなどありません。.

これが機能するためには、リファクタリングされるコードがテストでカバーされている必要があります。これはリファクタリングを開始する前に行われていますが、将来のリファクタリングの目標の1つは、コードをさらにテストしやすくすることであることに注意してください。

マーティン・ファウラー著「リファクタリング」は、さらに読むことをお勧めします。リファクタリングの基本原則について説明しています。彼はまた、小さなステップで進み、すべての小さなコミットの後でコードをビルド/テストを実行することの価値を強調しています。

これらの基本原則に加えて、以下に概説する論理的な一連のステップに従うことを目指します。

1. メソッドの再配置

まず、リージョンを使用して、ReconciliationIntro クラス内のすべてのメソッドを意味のあるグループに再配置します(コミット f2d9932[4]

図1:メソッドをリージョンに再配置した後のReconciliationIntro

メソッドを妥当と思われる一連のコンテキストにグループ化したので、これらを別々のクラスに分割したいと思います。しかし、どこから始めればよいでしょうか?ファイル読み込みコードには重複が最も多く、最も問題となっています。最終的にはこのコードをより汎用的にしたいのですが、まずは邪魔されずに確認できるように、コードを抽出したいと思います。新しいFileLoaderクラスを抽出します。他のグループも別々のクラスになりますが、はるかに単純なので、この記事では主にファイル読み込みコードに焦点を当てます。

2. ファイル読み込みメソッド間の関係性の分析

これまでのところ、同じクラス内のコードを移動しただけです。コミットする前にコードをリビルドしてすべてのテストを実行しましたが、不器用でない限り、以前のコミットは些細なものになると予想されます。次に何をするかについて、もう少し考えなければなりません。

リージョンの1つを使用して、新しいFileLoaderクラスに抽出するメソッドを特定しましたが、これが機能することをどのように確認できるでしょうか?隠れた依存関係はありますか?移動したいメソッド間の関係を図示することで、これを発見します。移動されるメソッドは青色でマークされています。

図3:移動するファイル読み込みメソッド(略語

これは厳密に自己完結型のコンテキストではないことがすぐにわかります。青いメソッドの一部は、別のクラスに保持したいメソッド(黒と緑で表示)をコールバックしています。これを処理するには、いくつかの方法があります。これについては以下で説明します。しかし、ここで行動計画を定義することができます。

行動計画

完了するまでに、元の大きなReconciliationIntroクラスは、以下の図に示すように、削減されたバージョンと5つの新しいクラスに分割されます。手順7に到達すると、最初のグループ分けをより細かい境界に絞り込むことになるため、リージョンとクラス間に1対1のマッピングがないことに注意してください。

図4:行動計画

全体的な目標は、この大きなクラスを小さなクラスに分割することです。上記の手順1と2では、リージョンを使用して、コードの明確な領域を特定し、一部のメソッド間の関係を分析しました。これにより、この作業に小さなステップで取り組む方法を考えるのに十分な情報が得られました。

結果の計画を以下に要約し、さらに詳細に説明します。可能な限り小さなステップを使用してどのように進めるかを考えることで、この計画に到達しました。多くの場合、私が行っていることは、*次の*変更ができるだけ小さくシンプルになるように設定することです。小さな変更を加えて、さらに小さな変更を容易にしています - 再びKent Beckの格言に従って、「変更を容易にし、次に容易な変更を行う」としています。

状況によっては、同じ計画に従わない場合がありますが、進め方がわからない場合は、悪いテンプレートではありません。優先順位は、安全な増分変更を行うための準備をすることです

  1. メソッドの再配置

    上記で説明したように、クラス内のすべてのメソッドを意味のあるグループにグループ化します。

  2. ファイル読み込みメソッド間の関係の分析

    上記で説明したように、ファイル読み込みコードがReconciliationIntroクラスの残りのコードにどのように接続されているかを特定します。

  3. 残っているメソッドの変更

    親クラスに残りますが、現在移動中のメソッドによって呼び出されているメソッドがあります。これを機能させるには、いくつかの変更が必要になります。

  4. 新しいFileLoaderクラスの網羅的なテストの作成

    新しいFileLoaderクラスは、テストで網羅されている必要があります。移動するコードのテストはすでに存在しますが、新しいFileLoaderTestsクラスに移動されます。

  5. 新しいFileLoaderクラスの作成と2つのメソッドの移動

    小さなステップの原則に従って、残りを移動する前に、2つのメソッド(パブリックメソッドとそれを呼び出すプライベートメソッド)だけを移動します。

  6. 他のメソッドを新しいFileLoaderクラスに移動する

    ここで面白くなってきます。私が興味を持っているすべてのファイル読み込みコードは、ついに新しいホームを取得します!

  7. さらに新しいクラスを抽出する

    ファイル読み込みコードを処理したら、他のコードリージョンの新しいクラスをいくつか作成します。

新しいFileLoaderクラスは、コンマ区切りデータのさまざまなソースを読み込み、調整の準備をするためにそれらをマージする役割を担います。このファイル読み込みコードは、現在ほとんどの問題が発生している場所であるため、最も詳細に説明します。リファクタリング前の元のコードはこちらで確認できますが、そのリンクをたどっても、大きすぎて頭に入らないことがわかるだけです!リファクタリング後のスリムダウンバージョンはこちらです。

これで、計画を続行できます

3. 残っているメソッドの修正

ファイル読み込みメソッドによって呼び出される2つのメソッド、Set_pathRecursively_ask_for_budgeting_monthsを特定しました

図5:ファイル読み込みメソッドによって呼び出されるメソッド(略語

これらのメソッドがファイル読み込みクラスと密接に結合されていない限り、次のいずれかを実行できます

  • 新しいFileLoaderクラスからコールバックできるように、パブリックにします。
  • クライアント呼び出しをファイル読み込みコードから移動し、代わりに他のネイティブReconciliationIntroメソッドに移動します。

Recursively_ask_for_budgeting_monthsには最初のアプローチを使用し、Set_pathには2番目のアプローチを使用します。これにより、次の状況になります

図6:移動後のファイル読み込みメソッド(略語

図でFileLoaderメソッドとしてマークされているメソッドは、この手順の最後にはまだReconciliationIntroクラスにありますが、これにより、手順5および手順6で説明するように、FileLoaderクラスに移動できるようになります。

Recursively_ask_for_budgeting_monthsは最終的には別のクラスのパブリックメソッドになりますが、今は他の場所から呼び出すことができることを確認したいだけです。テストできるように、すでにパブリックになっていることがわかりました。これはそれ自体がコードの臭いです - 別のクラスのパブリックインターフェースの一部としてより良い場所にあるという兆候です。

ファイル読み込みコードから呼び出されるもう1つのメソッドはSet_pathです。これは内部パス変数の値を変更するため、オプション2を選択します。個別に呼び出し、結果のデータをパラメーターを介してファイル読み込みメソッドに渡します。

通常はこれらが別々のコミットとして残るわけではないことに注意してください(マイクロコミットを使用して、それらをより大きなコミットにスカッシュすることができます)。ただし、手順を明確にするために、小さなコミットはそのまま残しています。すべての手順でコンパイルとテストの実行を行います

  1. 呼び出しメソッド(Create_pending_csvs)を修正してパラメータを受け取るようにしますが、最初はデフォルト値(コミット acc3519[4] を指定して、コードが引き続きコンパイルされるようにします。

    private void Create_pending_csvs()
    {
      // Some code
    }
    
    private void Create_pending_csvs(string path = "")
    {
      // Some code
    }
    
  2. Create_pending_csvs を呼び出す前に、Set_path を個別に呼び出します。結果として得られる新しい _path メンバー変数(サイドバー を参照)を取得し、Create_pending_csvs に渡します(コミット c5ebc2f[4]

    case "1":
    {
      Create_pending_csvs();
    }
    break;
    
    case "1":
    {
      Set_path();
      Create_pending_csvs(_path);
    }
    break;
    
  3. Create_pending_csvs 内からの Set_path の呼び出しを削除し、メンバー変数の代わりに渡された値を使用します(コミット 6df8f97[4]

    private void Create_pending_csvs(string path = "")
    {
      try
      {
        Set_path();
        var pending_csv_file_creator = new PendingCsvFileCreator(_path);
    
    private void Create_pending_csvs(string path = "")
    {
      try
      {
        Set_path();
        var pending_csv_file_creator = new PendingCsvFileCreator(path);
    
  4. 最後に、Create_pending_csvs パラメータからデフォルト値を削除します。これにより、すべてのクライアントが値を渡すことを強制します(コミット 2be56ea[4]。この順序で作業を行うことで、コードは常にコンパイル可能な状態を維持できます。

    private void Create_pending_csvs(string path = "")
    {
      // Some code
    }
    
    private void Create_pending_csvs(string path = "")
    {
      // Some code
    }
    

4. 新しい FileLoader クラスの網羅的なテストを作成します。

最初に移動するメソッドは Bank_and_bank_out_ _Merge_bespoke_data_with_pending_file です。そして、最初にしたいことは、これから作成する FileLoader クラスの新しいテストクラスに、既存の網羅的なテストをコピーすることです。このメソッドにはすでに1つのテストがあります - M_MergeBespokeDataWithPendingFile_ WillAddMostRecentCredCardDirectDebits - このテストの役割は、このメソッドが新しい口座振替データを「保留中」ファイル(すべての新しいトランザクションデータを含むように構築されている)と正しくマージすることを確認することです。

テストを*移動*しているだけで、新しいテストは書いていないことに注意してください。TDD[1] の概念(コードを書く前にテストを書く)に慣れている人は、*コードを扱うときはいつでも*新しいテストを書く必要があると思い込んでしまうことがあります。リファクタリングを行う場合は、多くの場合そうではありません。理想的には、機能はすでにテストでカバーされているはずであり、リファクタリングでは機能を変更しません。そのため、新しいテストを作成する代わりに、既存のテストを使用して、機能が当初の意図どおりに動作することを確認しています。

このテストを見直す良い機会です。作成してからしばらく経っているので、すぐに意味がわかるはずです。テストは明確で読みやすいものにし、システムの動作に関するドキュメントとして機能する必要があります。最初に気付くのは、名前が不適切なアサートメソッド - Assert_direct_debit_details_are_correct - が含まれていることです。「正しい」の定義は何でしょうか?読みやすくするためにテストを書き直します。これにはかなりの変更が必要です。読みやすいように、変更内容については詳しく説明しませんが、コミット f090f26コミット 6a6cece で確認できます。[4]

テストをリファクタリングしたので、関連するプライベートヘルパーメソッドとともに、新しいテストクラスにコピーします。このテストと他のテストは、新しい FileLoader クラスで動作することを目的としていますが、新しいテストクラスに必要なものがすべて揃うまで、古い ReconciliationIntro クラスで動作することに注意してください。また、すべてが安全に移動されるまで、テストコードは重複しています。

最初に、元のファイルと同じファイルに新しいテストクラスを作成します(コミット 491c795[4]。これにより、コピーしている内容がわかりやすくなります。その後、Resharper [5] と Visual Studio を使用して、すべてを新しいファイルに移動できます(コミット c9317c0[4]

図7:BBOテストの移動

5. 新しい FileLoader クラスを作成し、2つのメソッドを移動します。

テストクラスが稼働したので、新しい FileLoader クラスを作成できます。

チェーンの最下部にあるメソッド - メソッドツリー の最も低いリーフ - は Bank_and_bank_out__Add_most_recent_credit_card_direct_debits です。これは、独立したテストのないプライベートメソッドです(パブリック呼び出しメソッドを介してテストされます)。そのため、このメソッドとその呼び出し元(Bank_and_bank_out__Merge_bespoke_data_with_pending_file)の両方を新しいクラスに移動します。これらは、新しいクラスに移動される最初の2つのメソッドになります。

ここでも、テストが赤くならず、コードが常にビルドされるように、小さなステップで移動します。以下の各ステップの後、コードがビルドされ、テストが成功することを確認します。

  1. これが開始時の状況です。FileLoaderTests クラスが作成されましたが、まだ ReconciliationIntro クラスにあるコードをテストしています。

    図8:新しいFileLoaderクラス パート1(略語

  2. 新しい FileLoader クラスを作成することから始めます。ファイル読み込みメソッドの1つ(Bank_and_bank_out__ Add_most_recent_credit_card_direct_debits)はプライベートであり、このプロセスの最後にもプライベートになります。これは、新しいクラスに移動される直前のメソッドによってのみ呼び出されます。個別にテストでカバーされていないため、新しいクラスにコピーして、呼び出し元が移動されたときに準備しておけば済みます(コミット 0341476 を参照)[4]。ただし、一時的にパブリックにする必要があります。

    図9:新しいFileLoaderクラス パート2(略語

  3. これで、ReconciliationIntro クラスに新しい FileLoader クラスのインスタンスを作成し、古いプライベートメソッドの代わりに新しいパブリックメソッドを呼び出すことができます。古いプライベートメソッドも削除できます(コミット bde2ae2 を参照)[4]

    図10:新しいFileLoaderクラス パート3(略語

  4. 元の呼び出し元を新しいクラスにコピーします(コミット f0a5a59 を参照)[4]。この時点では重複していることに注意してください。

    図11:新しいFileLoaderクラス パート4(略語

  5. テスト対象のオブジェクトを ReconciliationIntro インスタンスから新しい FileLoader インスタンスに変更します。テストを元の呼び出し元の新しいコピーに向けます。呼び出すプライベートメソッドもコピーされているため、テストは成功します(コミット 3d573e3 を参照)[4]

    図12:新しいFileLoaderクラス パート5(略語

  6. これで、ReconciliationIntro クラスから元の呼び出し元を直接呼び出すことができます(コミット 77c0b14 を参照)[4]

    図13:新しいFileLoaderクラス パート6(略語

  7. 元々プライベートだったメソッドを再びプライベートにし、元の呼び出し元を削除します。すべてのテストが FileLoaderTests に複製されたため、古いテストクラスも削除します。(コミット 27f1a59 を参照)[4]

    図14:新しいFileLoaderクラス パート7(略語

6. 他のメソッドを新しい FileLoader クラスに移動します。

これで、他のすべてのメソッドを移動できます。依存関係(メソッド間、およびメンバーデータへの依存関係)に注意しながら、最も単純なものから始めて、1つずつ処理します。以下を検討する必要があります。

  • メソッドの名前を変更しますか?
  • パラメータリストを新しいオブジェクトに置き換えますか?
  • 冗長なパラメータはありますか?
  • 内部のネストされた呼び出し先は状態を変更していますか?どのような影響がありますか?

チェーンの下位にあるメソッドをインライン化してから移動することもできますが、そうすると、パブリックメソッドとして呼び出すテストが壊れてしまいます。そのため、代わりに単独で移動します。以下の説明の順序で、一度に1つずつ、チェーンの最後にあるもの、つまり次のツリーの最も外側のリーフから開始して実行します。

図15:FileLoaderメソッドツリー(略語

それぞれについて、以下のアプローチを使用します。

  • 元のメソッドをそのままにして、コピーを宛先クラスに作成します。新しいメソッドをパブリックにします。
  • 元のメソッドの代わりに新しいメソッドを呼び出します。
  • 網羅的なテストを新しいテストクラスにコピーし、新しいコードをテストしていることを確認します。
  • 古いメソッドと古いテストを削除します。

Bank_and_bank_out__ Merge_bespoke_data_with_pending_fileBank_and_bank_out__ Add_most_recent_credit_card_direct_debits はすでに移動したので(コミット 0341476 から コミット 27f1a59)、 अब मैं अन्य सभी विधियों (कमिट 7cd53f6 से कमिट 7ab95f2)[4] के लिए समान क्रियाओं को दोहराता हूँ। मैं इस बार हर छोटे से कदम को कमिट नहीं करता, लेकिन मैं अभी भी चरणों के समान सेट से गुजरता हूँ। प्रत्येक चरण के बाद, मैं यह सुनिश्चित करता हूँ कि कोड बिल्ड हो और परीक्षण पास हो रहे हों (जब मैं जानबूझकर एक परीक्षण विफल करता हूँ तब के अलावा)।

以前、いくつかのテストをリファクタリングしました。同じパターンに従うテストに対して、これらの変更を繰り返すことができます。一部のメソッドはテストカバレッジがないため、移動は非常に簡単です。これがこのリファクタリングを行っている理由の一部であり、そのコードをより簡単にテストできるようにするためです。

7. 新しいクラスの抽出

最初にリージョンを追加する際に、独自の明確なコンテキストを作成する予算編成月を取得する機能を特定したので、これらのメソッドを新しいBudgetingMonthServiceクラスに抽出します。これらのメソッドには公開エントリポイントが1つしかないため、これは非常に迅速かつ簡単です(コミット6103f0bを参照)[4]

ReconciliationIntroはまだ大きすぎますが、すべてのメソッドが互いに呼び出し合っており、コードを再確認するのに時間を費やしたため、残りの2つのリージョンであるユーザー指示と入力デバッグスプレッドシート操作が残りのコードを分割する最良の方法であるとは確信できません。 それを考えるために、スプレッドシートを使用して呼び出し階層を簡単に示します。

図16:呼び出し階層

これにより、2つではなく、3つの自己完結型のコード領域、つまりユーザー指示ファイル/パス情報の収集、およびデバッグモード切り替えコードがあることがわかります。

元のリージョンを削除し(コミット4c57927)、4つの新しいリージョンに置き換えます(コミット3446a54[4]。メソッドを新しいリージョンに収まるように再配置します。これらのメソッドは、3つの新しいクラス、つまりCommunicatorPathSetter、およびDebugModeSwitcherに変換されます(FileLoaderBudgetingMonthServiceは、この図には表示されていません。すでに抽出されているためです)。

図17:最終的なReconciliationIntroクラスの識別

BudgetingMonthServiceと同じ原則を使用して、これらの新しいクラスを段階的かつ安全に作成し、役割を果たしたら新しいリージョンを削除します(コミット7f464a4からコミット7e118c1までを参照)。

大きなクラスから新しいクラスを抽出する場合、それらを独立させる必要があることに注意してください。このため、抽出されたクラスが元のクラスのコンストラクターに依存関係として自動的に注入される場合に発生する可能性のあるアンチパターンを意図的に回避しています。

PathSetterクラスは、それほど単純ではないことが判明しました - コミット2921220からコミット398539aまでを参照してください[4]。これは、ステップ3の最後で気づいた、パス設定コードの現在のやや面倒な性質によるものです。このコードを別のクラスに抽出して独自の明確なコンテキストを付与することにより、このコードはすでに少し改善されていますが、それでも注意が必要です。

最後に、私のReconciliationIntroクラスはよりクリーンでシンプルになり、元の41のメソッドは3つに減りました:StartReconciliate、およびDo_matching

図18:最終的なReconciliationIntroクラス

要約

私のクラスは大きすぎました。望ましくないコーディング習慣が身についていたので、新しい機能を追加する前に立ち止まって改善したかったのです。

大きすぎるクラスを修正するために、次のアクションを実行しました

小さなステップで移動し、すべてのステップでテストをコンパイルして実行しました。これで、他のいくつかの小さなクラスから機能を呼び出す、はるかに小さなクラスができました。それぞれのクラスは、頭の中で理解しやすくなっています。

次のステップ

この記事はリファクタリングの途中で終わることに注意してください。そのため、次のステップに進む前にコードの関連する状態を確認する場合は、チェックアウトするコミット6103f0bが必要です。また、ステップ7のほとんどを、このコミットの*後*にこの記事で完了したことに注意してください(説明はこちら)。

コミット6103f0bでは、ファイルの読み込みコードをFileLoaderクラスにプルアウトし、主な課題を明確に把握しました。次の4つのメソッド(ここでインサイチュで表示可能)は明らかに問題があります

  • Load_bank_and_bank_in
  • Load_bank_and_bank_out
  • Load_cred_card1_and_cred_card1_in_out
  • Load_cred_card2_and_cred_card2_in_out

それらには次の問題があります

  • 多くの重複があります - 一見すると、それらは同一に見えます。
  • 長すぎます。
  • 内部でオブジェクトを作成し、テストできない、相互に密接に依存する方法で互いに渡します。

これらはすべて解決したい問題ですが、さらにリファクタリングを行う前に、これらのメソッドに関するテストが必要です。それが次にやることです。フォローアップ記事でそれについて書きたいと思っていますが、その種のことについて約束をするよりも、学んだ方が良いので、今のところ読者にとっての練習です。


謝辞

この記事の初期草稿を読んで貴重なフィードバックと提案を提供してくれた次の皆様に感謝します:Paula PaulMartin FowlerPriti BiyaniRiccardo NovagliaDan Terhorst-NorthKevlin HenneySteve FreemanJon SkeetSal FreudenbergJoe RayChris ShepherdLuke MortonScott GiminianiRichard Foster、Marcos Bezerra、Sam Carrington

脚注

1: TDDはテスト駆動開発の略で、すべてのコードユニットがテストされ、テストがシステムの動作を記述することを保証する手法です。これは、コードを書く*前*にテストを書いて、それらのテストに合格するようにすることによって行われます。それについてはもっと多くのことが言えますが、この記事の焦点ではありません。こちらで詳細を読むことができます。

2: 「頭の中に収まる」は、Dan Terhorst-NorthのSoftware, Faster book(現在も進行中の作品)のパターンの1つです。彼は、「頭の中に収める」ことによって、あらゆるコードチャンクを理解できることの重要性について語っています。名前はJames Lewisに由来し、Danはこの講演で説明していますが、Danは私に、この概念はAlistair Jonesに由来する可能性があると語っています。

3: 戦略パターンへのリファクタリング:このリファクタリングの目的の多くは、戦略パターンの使用を可能にすることです。「パターンへのリファクタリング」のビジネスには、Joshua Kerievskyによるこの本全体が捧げられており、詳細を知りたい場合は読む価値があります。実際、Martin Fowlerが述べているように、「多くの人が、リファクタリングアプローチはパターンについて学ぶためのより良い方法であると述べています。なぜなら、問題と解決策の相互作用を段階的に確認できるからです。」

4: コミットリンク:コミットリンクをたどる義務はありません!詳細については、この記事の読み方サイドバーを参照してください。

5: Resharperは、コード編集などのために使用される、一般的に使用されるVisual Studio拡張機能です。ただし、無料ではなく、ネイティブのVisual Studioツールによってますます影が薄くなっています。

重要な改訂

2020年4月14日:初版