50 Lines of Codeを読むか、Extract Method Refactoringを使うか？ - デレク・コマーティンからの洞察

例チャットシステムでのユーザーサインアップ

ビデオの冒頭で、デレクはチャットシステムのサインアップ機能の例を紹介します。 コンパクトですが、いくつかのタスクを実行する約50行の現実的なコードブロックです：

• ユーザー名が空白でないことを確認

• ユーザー名がすでに使用されていないか確認

• 年齢制限のあるチャンネルに対応

• 新しいユーザーオブジェクトを保存します。

• アクティベーションリンクをメールで送信

このコードは1つの関数に存在し、一見すると、抽出メソッドのリファクタリングに最適な候補のように見えるかもしれません。 しかし、デレクが警告しているように、影響を理解せずにやみくもにリファクタリングすると、かえってわかりやすさが損なわれてしまいます。

リファクタリングから始める：抽出メソッドを選択する

デレクは、多くの開発者が行っているように、コードの断片を細かく分割することから始めます。 ほとんどのIDEやコードエディターで、コンテキストメニューやキーボードショートカットからExtract Methodを選択する方法を紹介しています。

抜粋しています：

• validateUsernameで、ユーザ名が空白でないことを確認してください。

• existingSignUpNotActivated で、有効化されていないアカウントを確認します。

• validateExistingUserは、すべての既存ユーザーチェックを処理します。

• filterAgeRestrictedChannels で、18歳未満のユーザーのチャンネルを処理します。

• sendEmail でウェルカムメールを送信します。

彼は、新しい関数にそれぞれ意味のある名前を付けています。これは、クリーンコードの実践でよく推進される最重要ヒントの1つです。 しかし、デレクは、これらの修正されたバージョンを見ていくうちに、機能性ではなく、読みやすさや制御フローにおけるロジックの亀裂を指摘し始める。

課題1: 隠された実装の詳細

デレクが最初に指摘する赤信号の1つは、実装の詳細が抽出されたメソッドの後ろに隠されていることです。

例えば、validateUsernameとvalidateExistingUserメソッドは、実際には例外をスローします。 しかし、リファクタリングされたコードを読む開発者としては、その内部にアクセスしない限り、見当もつかないだろう。

このようなリファクタリングは、制御ロジックを隠し、バグや検証漏れにつながる可能性があります。 範囲と流れはもはや明白です。 コードを明確にする代わりに、抽象化の迷路を作り、例外や変更された変数などの副作用が、ロジックが元々書かれていたフォームでは見えなくなります。

課題2: インダイレクトと連鎖抽出

次にデレクは、ある抽出メソッドが別の抽出メソッドを呼び出すといった、インダイレクトの問題を指摘する。 彼は、validateExistingUserメソッド自体がどのようにexistingSignUpNotActivatedで構成されているかを示している。

もはや、トップダウンの単純なコードブロックを読んでいるわけではありません。 メソッド、ファイル、クラスを行き来して、何が起こるかをトレースしてください。 そして、エディターはこの流れをナビゲートするのに役立つかもしれませんが、読者の認知負荷の負担になります。

これは、リファクタリングが複数のファイルやコンポーネントにまたがるような大規模なシステムでは、さらに骨の折れる作業となります。 突然、あなたの "きれいなコード "は、元の "ごちゃごちゃした "50行よりもわかりにくく見える。

課題 3：ローカル変数と状態の変更

このビデオで最も重要なレッスンの1つは、ローカル変数と状態変異の扱いから得られます。

Derekは、filterAgeRestrictedChannelsメソッドにスポットを当てています。 結果を返すのではなく、渡されたチャンネルリストを直接変更します。 これは、別のメソッド内からローカル・ステートを変更していることを意味し、メソッドをよく調べない限り、この変更は隠されています。

これは、関数が純粋な操作であるか、物事を変更するときに明確に知らせるかのどちらかであるという期待を打ち破るものです。 値を返さず、内部で値を変更する新しいメソッドでロジックを置き換えると、リスクと混乱を招くことになります。

デレクのリファクタリングされた代替

では、デレクは実際にどのように古いコードをリファクタリングしているのでしょうか？

彼はもっとシンプルなアプローチを提案する：

1.自己説明的なロジックはインラインにしてください。 ユーザー名が空であるかどうかの最初のチェックは、わかりやすくコードベースが乱雑にならないように、メインメソッドのままにしています。

2.変異の代わりに結果を返す。 チャンネルリストを変更する代わりに、filterAgeAppropriateChannels関数はフィルタリングされたリストを返すようになりました。これにより、データの流れが明確になり、予期せぬ副作用を防ぐことができます。

3.シンプルで予測可能な抽出方法を使用してください。 唯一抽出されたメソッドはisExistingUserAlreadyActivatedで、例外を投げずにブール値を明確に返します。 詳細を隠すことなく、ロジックをカプセル化します。

4.メール送信のようなインラインの副作用は避けてください。 Derekは、デモンストレーションのためにEメールのロジックをそのままにしていますが、実際のシステムでは、これはユーザーフォームの送信に直接結びつけられるものではなく、別のプロセスやスレッドのイベントを通して処理されるべきであると提案しています。

Derekは、2つの抽出メソッドのみを使用し、残りのロジックはインラインのままにしています。

抽出メソッド リファクタリングの最後のヒント

Derek 氏のビデオでは、抽出メソッド リファクタリングを効果的に使用するための実用的なガイドラインをいくつか紹介しています：

• メソッドが何をするかを正確に説明する意味のある名前を使用してください。

• 明らかでない限り、状態の変異や例外のスローのような副作用は避けてください。

• 入力パラメータを変更する代わりに値を返します。

• ロジックを何重にも抽象化して隠さないこと。

• メソッドが原形で読めるようであれば、リファクタリングのためだけにそれを複数の関数に無理やり押し込めないようにしてください。

時には、最高の抽象化とは、抽象化しないことである。

結論

Derek Comartinのアプローチは、リファクタリングは常にコードを改善するという概念に挑戦している。 抽出メソッドのリファクタリングでは、より少ないことがより多くなることがよくあります。 Extractメソッドを使いすぎてロジックを切り刻むのではなく、何が付加価値を生むのか、何がコードを理解しやすくするのか、何が重要な詳細を隠すのかを評価してください。

動画のデレクは、明確な例と実際のコードへの直接的な洞察を用いて、1つのメソッドに50行を記述し、ストーリーのようにトップダウンで読む方が、コードベース全体に広がる10個の小さなメソッドよりも優れている場合があることを示しています。

新しいメソッドを作成するためにキーボードのショートカットに手を伸ばしたことがあるなら、デレクのアドバイスを思い出してください：一時停止し、評価し、リファクタリングがIDEだけでなく読者のためになることを確認してください。