Zenn
📚

リファクタリング　既存のコードを安全に改善する（第2版）の個人的まとめ

2021/07/28に公開
技術書
リファクタリング
tech

https://amzn.to/3rpHRxR

この記事について

タイトルの本について、それぞれの章でざっくり概要と所感を述べた個人的なまとめです。

もちろん、原著のエッセンスは全て網羅しておりませんので、もっと興味が沸いたらぜひ読んでいただけると私としては嬉しい限りです。（私も理解の点ではまだまだですし、英語の原書まで読めていないので偉そうなことは言えませんが）

誰のため？

  • 読みたいけど高いくて買うのが気が引ける人
  • なかなか読む時間がない人
  • 読もうと思ってるので一旦全体を把握したい人
  • 読んだので他の人の感想とか聞いてみたい人
  • 読んだけど内容もはや覚えてない人

なんのため？

個人的な読んだ中身のまとめと所感を書き留めたかったのと、後で見返す用です。
ついでに誰かの参考になればと思い。

本書の構成

この本は、リファクタリングの手法やサンプルコード、リファクタリングを行うケースを明示したガイドブックになってます。

JavaScriptをサンプルコードとしてますが、JavaScript固有のスタイルは避けて記載されているので、JavaScriptを知らなくても他の言語をある程度使えるのであれば特に問題なさそうなコードです。
ただ、オブジェクト指向の考えはよく出てくるので、そこは雰囲気分かってる必要はあるかと。

オススメの読み方が「はじめに」に述べられており、これがとてもありがたいので抜粋します。

  • リファクタリングが何か知りたい
    第１章（リファクタリング－最初の例）を読むとリファクタリングの過程をざっと知ることができます。

  • なぜリファクタリングするか知りたい
    １章（リファクタリング－最初の例）と２章（リファクタリングの原則）を読めばなぜすべきなのかがわかります。

  • どこをリファクタリングするか知りたい
    ３章（コードの不吉な臭い）を読めばリファクタリングの必要性を示す兆候がわかります。

  • 実際にリファクタリングをしたい
    １章（リファクタリング－最初の例）〜４章（テストの構築）をじっくり読んでから、５章〜１２章最後まで（カタログ部分）をざっと読みましょう。詳細については、実際にリファクタリングを行う必要が生じた時に改めて読めば良いです。

というわけで、全てをきっちり最初から読む必要はないのです。やったぜ！
ガイドブックなので、実際にリファクタリングをする時に手元に置いておくと心強いかもしれない。（が渦中にいると忘れるよね、わかる）

各章のざっくり概要と所感

Chap.1 リファクタリング－最初の例

比較的簡単な例でリファクタリングの例が示されています。

段階としては、

  1. 大きな関数を分解
  2. ビューとロジックに分けていく（この例では計算部分とフォーマット部分）
  3. ポリモーフィズムによるロジックの分離

この章だけに留まらない、いくつか大事な話が述べられています。（所感含みます）

良いコードとは?

関数を粒度や、関数の名前付けなどは好みによるものが大きく良し悪しは決められないことが多いです。
ただし、良いコードとは好みを超えて、変更が容易であることです。

変更が容易であるというのは、変更の見通しが立ちやすい・どこを修正すればいいのかすぐわかる・変更点の副作用が少ない・・などなどプログラマーの生産性を落とさないコードです。

自分はついつい、短く簡潔なコードが良いと思ってしまいがちですが、リファクタリングではコード量が増えてしまうことは往々にしてあるためこの点は気をつけたいです。

パフォーマンスは気にしない

変数をコピーしたり、関数やループが増えたりとパフォーマンスを気にする人はリファクタリング中に心配してしまうかもしれません。
ただ、ソフトウェアの速度性能に影響する箇所はごく一部であり、ほとんど影響しないです。

そうは言ってもパフォーマンスに影響を及ぼしてしまうリファクタリングもあります。
ただ、その時も構わず継続します。なぜなら、よく整理されたコードの方が何が起こってるか把握しやすくパフォーマンス最適化がしやすいからです。

これは自分も、パフォーマンスの影響はどうだろう？と読み進めてしまっていたので心を読まれたようでした。
確かに初めから闇雲にパフォーマンスの最適化を施そうとしても局所的かつ効果の薄いところばかりに目がいってしまいがち・・・。

テストは不可欠である

リファクタリングによって振る舞いを変えないということは必須条件です。
そのためにはこまめにテストを通しながら、もし通らなかったらどこで壊してしまったかすぐ分かってすぐ壊れる前の状態に戻せることが大事です。（動く状態でのリファクタリングの段階でのバージョン管理も大事な要素ですね）

この本ではリファクタリングで誤って元の振る舞いを変えないよう、とても丁寧にステップに分けて紹介してくれています。

そのステップごとに毎回、「コンパイル・テスト・コミット」を繰り返し述べています。（厳密にはJavaScriptはコンパイルではないですが）

そのうちのテストを毎回手動でやっていてはとてもじゃないですけど無理です。
ましてリファクタリングが必要なリアルワールドのコードなんてもっと煩雑で膨大なステップを踏む可能性の方が大きいので、自動テストはないととても厳しいですね。

Chap.2 リファクタリングの原則

エンジニア、もしくはそれに関わる者としてリファクタリングについて知っておきたいという人は正直ここまで読めばいいのでは？というくらいまとまっている内容です。
ポイントを挙げた上で、所感を述べていきます。

リファクタリングの定義

外部から見たときの振る舞いを保ちつつ、理解や修正が簡単になるように、ソフトウェアの内部構造を変化させること。

ということですが、例えば大規模なリファクタリングをしようとして数日間ずっとコードが壊れたままならそれはリファクタリングとは言えないそうです。上の定義に加えて、小さいステップでリファクタリングを進める、そして各々のステップでコンパイル・テスト・コミットを繰り返すことが大事なのかと思います。

なぜリファクタリングを行うのか

  • ソフトウェア設計を改善する

リファクタリングを定期的に行うことで、コードを良い状態に保つことができる。

最初の設計がどれだけ良かったとしても、追加変更のないソフトウェアは皆無なので、よっぽどコードがわかっているエンジニアが修正しない限りはコードが劣化していく。

劣化を止めるためにはリファクタリングの必要がある。

個人的に、できるエンジニアというのは息をするようにリファクタリングをしてる、もしくは変更とリファクタがナチュラルに行われている感はある。（元のコードが酷すぎなければ）

  • ソフトウェアを理解しやすくする

少しの時間をリファクタリングに充てるだけで、コードの目的がより伝わるようになり、実現したいことを明確に表現できるようになる。

ちょっと前のコードでも油断すると「なんだこれ？」となってしまうことは往々にあるので、理解しやすい状態に保つというのは他の人のためでもありますが自分のためでもありますよね。

  • バグの発見を助ける

コードが理解しやすいということは、バグを見つけやすいということである。

自分の経験則の限りでも、違和感感じるところは大体バグってる・・・。

  • プログラミングを早める

すばやく機能を追加し続けるには、リファクタリングが不可欠となっています。

これまで述べたことから当然の結果なんですが、リファクタリング自体に工数がかかるということからなかなかそのことに納得してもらえない現場もあるよなというぼやきです。

じわじわとブラックボックス化していくのでなかなか臭い物に蓋をする精神で放置してしまいがちです。（反省）

いつリファクタリングをすべきか

準備のため

機能追加を容易にするため、機能追加などのタイミングで行います。

理解のため

コードを分かりやすくするために行います。

個人的にはこの一文が非常にいいなと思いました。

頭の中にある理解をコードに移し替えていくのです。

ゴミ拾い

理解のためのリファクタリングの派生として、書き方が今ひとつというときに行います。

改善は僅かでも、みんながちょっとずつ改善すればコードは良くなる・・はず。立つ鳥跡を濁さず精神ですね。

計画して行うリファクタリングと、機に応じて行うリファクタリング

リファクタリングは、過去の過ちや醜いコードをきれいにする活動のように考えてしまうのも、よくある間違いです。

素晴らしいコードもまた、リファクタリングは必要なのです。ソフトウェアは「完成」という状態になることはなく、新たな機能を実装する時は追加しやすい形にコードを変えていく必要があります。

なので、ほとんどのリファクタリングは機に応じて行われることが多く、活動としては目立たない行為です。

長期のリファクタリング

1週間もしくはそれ以上の時間をかけないといけないものもときにはあります。コンポーネント化や依存性が大きいコードなど。

ただ、チームをリファクタリングに専念させるというのはおすすめできないので、少しづつ改善していくことが勧められています。
（リファクタリングはコードを壊さないという利点を生かす）

ライブラリを置き換えるときは、ライブラリのインターフェースに抽象レイヤを導入すると良さそうです。

コードレビュー時

リファクタリングは、他人の書いたコードのレビューをするのにも役立ちます。リファクタリングによって、自分の考えがコードに反映されるとどうなるかをはっきり把握することができるからです。

筆者によると、コードの作者と並んでコードをレビューしてリファクタリングをする形が最もうまくいきます。つまりペアプログラミングです。

管理者を説得するには

管理者がリファクタリングを、過去のエラー修正あるいは価値の生まない作業と思い込んでいる作業現場もあります。
リファクタリング活動と言いながら、コードベースを壊しかねない再構築をしているチームの場合、さらに状況は悪化していきます。

管理者や顧客によっては、リファクタリングの重要性を技術的に理解できない場合もあります。が、ソフトウェア開発者は「だまってリファクタリングをする」べきです。

これは反乱でもなんでもなく、ソフトウェア開発者はプロフェッショナルとして、効果的なソフトウェアをできるだけ早く構築することが求められています。機能追加の際に、どのような手段で実施するかは開発者の責任なのでその手段の一つがリファクタリングであるというだけです。

息をするようにリファクタリングできるようになりたい。スーハースーハー。

リファクタリングを避けるとき

時にはリファクタリングの価値がない場合もあります。

修正の必要がなければリファクタリングの必要性もありません。例えば単なるAPIとみなせるのであれば放置することもあります。理解しなければならない状況で初めてリファクタリングの利点が出てきます。

また、最初から書き直した方がリファクタリングより簡単な時もあります。ただこの決定は非常に難しいです。

リファクタリングの問題点

リファクタリングを行う時のいろいろな問題点と対応策。

  • 新機能の些細な場合など、実装が遅くなる ➡️ 追加しようとしている機能が非常に些細な場合は後回しすることもあります。トレードオフの関係になります。
  • コードの所有者の境界がリファクタリングが妨げになる ➡️ 強いコードの所有権が細かく設定されていない組織が理想ですが、オープンソース的なPRを受け入れるやり方が大規模システムだと良い妥協案になります。
  • バージョン管理のブランチ運用でマージ問題を悪化させる ➡️ ブランチが長くなるほどマージの問題が大きくなっていきます。メインブランチ結合の頻度は高くすべきです。
  • 自己テストコードのハードルが高い ➡️ 自己テストコードはリファクタリングを可能にするだけでなく、新機能の追加もはるかに容易にしてくれるので、価値のある努力です。自動リファクタリング機能をサポートする開発環境もオプションとしてはあり。（自己テストコードは依然としてあったほうがいいが）
  • レガシーコードである ➡️ レガシーコード改善ガイドをどうぞ
  • データベースにどう対応するか ➡️ 正式に至るまで複数のリリースに分解するのがベストです。新旧のフィールドを容易して同時運用をするなど、すぐ変更を取り消す仕組みを用意しておきます。

リファクタリングとアーキテクチャ

ウォーターフォール開発な現場では、アーキテクチャは最初に決定したら固定されることが普通でした。

リファクタリングがアーキテクチャに与えたもっとも大きな影響は、要求の変化にしなやかに対応できる、すぐれた設計のコードベースを作り上げる方法を示したことです。

ソフトウェアは、当初の要求が事前に十分把握できているということはほぼ無く、作ってみてから達成できないということがわかることもよくあります。

そのためにある程度の柔軟性を持って開発する方法もありますが、あらゆるパターンを把握・予測することはできません。
代わりに、ユーザ要求の変化に合わせて、アークテクチャを新たな要求に適合するようにリファクタリングしていくことでアーキテクチャ自体を柔軟に進化させていくという方法をとることができます。

リファクタリングと開発プロセス

自己テストコードはリファクタリングを支える基盤のひとつです。自己テストコードがあることで、プログラムのミスがあったら、テストは失敗するという確信することができます。

また、チーム内でリファクタリングを行う時、継続的インテグレーションを採用することを勧めています。
開発プロセスにリファクタリングを組み込むために、各メンバが他のメンバの作業と干渉することがなく、いつでもリファクタリングが行えるようになっていることが重要だからです。

リファクタリングとパフォーマンス

パフォーマンスで興味深いのは、プログラムを解析してみると、ほとんどの時間がごく一部の処理で集中的に消費されているという事実です。そのため、コード全体に渡って均等に最適化を行ったとしても、その活動の９０％は無駄になるのです。

ということで、まずはそのごく一部の遅くしている処理を特定するためにまずはプログラムを整理された状態にする必要があるとのことです。初めはリファクタリングで短期的には遅くなってしまうこともありますが、チューニングをしやすくすることで最終的には得になります。

Chap.3 コードの不吉な臭い

この章では、リファクタリングが必要となりそうな兆候を列挙しています。
※兆候の名前は本文から引用しましたが、なんのことか分かりにくいものは補足します。

テーブルにする。

名前 事象 対応策
不可思議な名前 変数名や関数名、フィールド名に適切な名前がついてない 適切な名前に変更
重複したコード 同じ構造のコードが２箇所以上に存在する 関数を抽出する、ステートメントを移動、メソッドをベースクラスに引き上げるなど
長い関数 関数内の処理が長すぎる いろいろあるが、99%は関数の抽出
長いパラメータリスト 計算に必要なパタメータが多い 問い合わせによってパタメータを置き換えたりオブジェクトを受け渡すようにする
グローバルなデータ グローバル変数、アクセス制御ができてないクラス変数やSingletonが存在する 変数をカプセル化する
変更可能なデータ 予期せぬデータの変更が存在する 変数のカプセル化、変数の分離などで値の変更を制限したり監視する
変更の偏り 一つのモジュールが異なる方法で変更される（ある一つの追加のために毎回複数の部分を修正しなければならない） 関数の移動やフェーズを分離する
変更の分散 変更を行う旅にあちこちのモジュールが少しつづ書き変わる 関数やフィールドの移動をしてモジュールをまとめたり、似たようなデータ構造を扱う関数をクラスに集約するなど
特性の横恋慕 あるモジュールの関数が、内部よりも外部と多くやりとりしている 関数を移動したり分離する
データの群れ いくつかのデータがつるんで、クラスフィールドやメソッドのシグネチャなど様々な所に現れる クラスに抽出してオブジェクトに発展させ、まとめてデータの受け渡しをできるようにする
基本データ型への執着 座標、範囲、貨幣などの型の導入を避け、プリミティブ型を使用する オブジェクトを導入する
重複したスイッチ文 コードの様々な箇所に同じ条件分岐が存在する ポリモーフィズムによる条件分岐の書き換えを行う
ループ forやwhileなどのループが存在する（パイプライン操作の方が要素と処理内容を素早く確認できることもあり、現在はあまり重要でなくなってきている） filter, mapなどによるパイプラインへの置き換え
怠け者の要素 本体に書かれた処理と同じ名前の関数があったり、メソッドが一つしかないクラスが存在する 関数やクラスのインライン化、継承がある場合はクラス階層を平坦化する
疑わしき一般化 いつか必要になるであろうと期待された（だが使われていない）機能や必要としていない凝った仕掛けが存在する 大した動きをしていない抽象クラスならば平坦化したり、意味のない以上はインライン化によって削除する。未使用のパタメータやデッドコードも除去すべき
一時的属性 インスタンス変数の値が、特定の状況でしか設定されないクラスが存在する クラスの抽出によってその変数を切り出すか、関数の移動で新たなクラスを作成したり特殊ケース用の代替クラスを用意する
メッセージの連鎖 getXXXのようなメソッド呼び出しが長々と連なっていたり、getXXXの結果を一連の一次変数で受け取っているようなメッセージの過剰な連鎖が発生している 委譲の隠蔽や関数を抽出、移動して連鎖を短くまとめる
仲介人 カプセル化によってもたらされた権限の委譲が過剰にある（メソッドの大半が別のオブジェクトに委譲しているだけのクラスなど） 本当に仕事するオブジェクトに直接処理させる、関数のインライン化によって呼び出し側で行う
インサイダー取引 モジュール間の結合が強固である（互いに依存するものが多い） 関数の移動やフィールドの移動、別のモジュールを共通データの管理役として追加し、結合を疎にする。継承により発生している場合は、委譲によってサブクラスもしくはスーパークラスの置き換えを行う
巨大なクラス 一つのクラスがあまりにも多くの仕事をしている クラスを抽出したり、スーパークラスの抽出やサブクラスによってタイプコードを置き換えする。
クラスのインターフェース不一致 関数宣言の変更や関数の移動、もしくはスーパークラスの抽出をしてインターフェースを一致させる
データクラス getとset以外何もないクラス 変更されて困るものはsetterの削除する、関数を移動することでデータクラスに振る舞いを移せないか検討する
相続拒否 サブクラスが親の属性の一部しか利用していない 兄弟クラスを作成してメソッドの押し上げ、押し下げによって使われていないコードを兄弟クラスへ移す
コメント 分かりにくいコードを補うためにコメントが書かれている そもそものコードをリファクタリングして分かりやすくする

Chap.4 テストの構築

リファクタリングの時に限らず、自動テストが存在していることで生産性は大きく高まります。

この本ではざっくりサンプルコードが書かれているのみで、テストの詳細では語り尽くされていませんが、テストの実施については繰り返し述べられており、TDD（テスト駆動開発）が推奨されています。

なお、サンプルコードはJavaScriptでMochaです。

個人的にテストを学ぶとしたらこの辺りかなという本を列挙しておきます。（自分がエンジニア１年目の時に読んだもの）

https://amzn.to/3i248P7

https://amzn.to/371cYqg

https://amzn.to/3iSdI6K
※JUnitはJavaのテストフレームワーク

Chap.5 カタログの紹介

６章以降、具体的なリファクタリングのカタログを紹介しているので、この章ではどのように紹介しているのか述べているのみです。

Chap.6 リファクタリングはじめの一歩

  • 関数の抽出
  • 関数のインライン化
  • 関数宣言の変更
  • 変数のカプセル化
  • 変数名の変更
  • パラメータオブジェクトの導入
  • 関数群のクラスへの集約
  • 関数群への変換への集約
  • フェーズの分離

最初に身につけるべきリファクタリングカタログとあり、一般的な名前で紹介されているので、詳細を読まなくてもなんとなく何をしているのかわかるものばかりです。

Chap.7 カプセル化

  • レコードのカプセル化
  • コレクションのカプセル化
  • オブジェクトによるプリミティブの置き換え
  • 問い合わせによる一次変数の置き換え
  • クラスの抽出
  • クラスのインライン化
  • 委譲の隠蔽
  • 仲介人の除去
  • アルゴリズムの置き換え

他のモジュール、クラスからの情報隠蔽は特に大規模なシステムになる程重要な観点だと多います。アルゴリズムの置き換えがここに存在することに少し違和感がありましたが、関数も実装をカプセル化しているという観点からは確かにここに含まれると思いました。

Chap.8 特性の移動

  • 関数の移動
  • フィールドの移動
  • ステートメントの関数内への移動
  • ステートメントの呼び出し側への移動
  • 関数呼び出しによるインラインコードの置き換え
  • ステートメントのスライド
  • ループの分離
  • パイプラインによるループの置き換え
  • デッドコードの削除

パイプラインは見慣れないと逆に何をやっているかわからない時もありますが、個人的には上手く使いこなしているとカッコいいというイメージで割とパイプラインに置き換えるのは好きだったりします。
Javaは最近まで（とは言っても数年前ですが）ラムダ式の記述ができなかったので、パイプラインへの置き換えはしょっちゅう使わなかったのですが、最近は広く受け入れられている気がします。

不要なステートメントに火炎放射を浴びせるのは最高です。

この一文に笑いました。

Chap.9 データの再編成

  • 変数の分離
  • フィールド名の変更
  • 問い合わせによる導出変数の置き換え
  • 参照から値への変更
  • 値から参照への変更

個人的には動的型付の言語の場合、この辺がなあなあになっていて分かりにくくなっていることが多いイメージです。
言語仕様によっては、参照と値の区別がつきにくかったり厳密だったりするので、まずは言語仕様がちゃんと把握できていないといけないよなぁ、と自分はRustとJavaScriptを行ったり来たりしてた時に混乱しかけていました。

Chap.10 条件記述の単純化

  • 記述条件の分解
  • 記述条件の結合
  • ガード節による入れ子の条件記述の置き換え
  • ポリモーフィズム節による入れ子の条件記述の置き換え
  • 特殊ケースの導入
  • アサーションの導入

コードのネストで一番読みにくいコードが発生しやすいところだと思っています。

条件式の書き方まで言及すると、そもそもの仕様をわかりやすい自然言語で表現できる能力が大きく関わっているような気がします。かつ、このようなわかりやすい条件式を記述できると素晴らしいですね。

特殊ケースの導入やポリモーフィズムによる条件記述の置き換えは割と効力を発揮するという個人的見解です。

Chap.11 APIのリファクタリング

  • 問い合わせと更新の分離
  • パタメータによる関数の結合
  • フラグパラメータの削除
  • オブジェクトそのものの受け渡し
  • 問い合わせによるパラメータの置き換え
  • パラメータによる問い合わせの置き換え
  • setterの削除
  • ファクトリ関数によるコンストラクタの置き換え
  • コマンドによる関数の置き換え
  • 関数によるコマンドの置き換え

問い合わせと更新の分離はWEB APIなど設計をしたことのある人なら基本なのかなと思いますが、APIは公開されて使う人が多い上に、一つの呼び出し方法の変更が他のいろんな箇所のソースコードに影響する可能性があるので接合部分はできるだけ初期によく考えて固めたいところですよね・・・

Chap.12 継承の取り扱い

  • メソッドの引き上げ
  • フィールドの引き上げ
  • コンストラクタ本体の引き上げ
  • メソッドの押し下げ
  • メソッドの押し上げ
  • サブクラスによるタイプコードの置き換え
  • サブクラスの削除
  • スーパークラスの抽出
  • クラス階層の平坦化
  • 委譲によるサブクラスの置き換え
  • 委譲によるスーパークラスの置き換え

継承については、他のカタログよりも逆パターンが多く存在します。（押し上げ⇆押し下げ等）
そして、機能追加などでその時々の最適なものが変わりやすいのもここだと思ってます。

どこのクラスに持たせるべきか？継承すべきか委譲すべきか？その辺りはドメイン知識と経験が求められるところですが、誤用してしまうことも多い部分なので強力な仕組みである反面、気をつけたいところです。

全体の所感

記載されているコード自体はそんなに難しいことは一切していないですし、理解しやすい内容ではあったのですが

  • 常日頃からこういうリファクタリングに対する意識を持っているか
  • 機能追加やパフォーマンスの改善で、全体の見通しが立たないコードのままその場しのぎの対応を繰り返していないか

そんなことを考えさせてくれました。

リファクタリングのカタログに関しては豊富なゆえに、慣れない人だと使いどころは一旦立ち止まってソースコードを読みながら一旦考察してみる必要がありそうです。

さらに、実際のコードはもっとぐちゃくちゃで、どうやってステップを踏んでリファクタリング後の状態に持っていくかの手順は実践を踏みながら体得していくしかないなと思ったので、手元においておきたい系の本だなと感じました。

時には逆行するリファクタリングもあり（例としては、抽象クラスに持たせるかどうかなど）、どちらを選択するかについて完全な正解というものは存在せず、今の状態から適切そうなものを選択していくことしかないのだと思います。

機能追加をただただコードを追加していくことだけで対応していると、ソフトウェアはどんどん廃れていってします。特にウォータフォール型ではなかなか開発が厳しい変化の多い時代だからこそ、こうやってリファクタリングすることの重要性はますます増していくのなと感じました。

というわけでいい感じにかっこいいこと言ったのでこの辺りにします。
ここまでお付き合いいただいた方々、ありがとうございました！

Discussion