はじめに
業務でリファクタリングをする機会が増えたため、これを機にリファクタリングの金字塔とされる『リファクタリング(第2版): 既存のコードを安全に改善する』を読んでみました。
特大ボリュームですが評判通りの素晴らしく勉強になる書籍でした。なんとか読み終わったので印象に残ったことをまとめます。
リファクタリングの原則を理解する
『第1章 リファクタリングー最初の例』『第2章 リファクタリングの原則』では、リファクタリングに関する基礎的な考え方を深めることができます。具体例を用いてどのようにリファクタリングを進めながら解説するとともに、リファクタリングの原則やトレードオフとの考え方などを理解することができました。
リファクタリングにおけるテストの重要性
リファクタリングを行うとき、最初にすることは常に同じです。対象となるコードにおいてきちんとしたテスト群を作り上げることです。テストは不可欠です。リファクタリングは非常に秩序だっていて、新たなバグを生み出しにくくなっていますが、人間が作業する以上、間違いを犯す可能性があります。（P5『リファクタリングの第一歩』より）
外側の見え方は変えずに内部だけを行うリファクタリングにおける重要な観点として、まず最初に本書が伝えるものです。普段の開発をしている上で、既存コードをリファクタリングしておきたいなあと思う時に、コードの改修から始めることはグッと我慢しなければならないということです。
この重要性については、本書の最初に登場するとともに、その後にも口酸っぱく出てくる印象でした。
これまでリファクタリングしようと思った箇所においてテストがなかった場合、わりとそのまま突き進むことが多かったので、この観点は以降真剣に考えるようになりました。また、リファクタリングのテストというと単体テストを想起することが多いと思います。もちろんそれを指すことが多い（本書でも単体テストが書かれている印象）ですが、本書において
テスト群を作り上げる
とテストの種類等については一切書いていないことからもわかるように、コードが壊れていないかを確認するためにどのようなテスト群の設計が必要かを考えるのは、とても意義があることのように思います。
本書を読むモチベーションになったものとして、実際に業務上で大規模なリファクタリングを推進しようとしている背景がありました。テストは必要とはいえ、リファクタリング対象にしたコードはコードレベルの単体テストを書ける状態ではなかったです。本書を読む前であれば、テストをすっ飛ばしてリファクタリングから力技で頑張ることを決断していたような気がしなくもないですが、本書を読んで考えた結果、一時的にE2Eを強化して、デグレを起こしていないことを担保する道を選ぶことができました。
リファクタリングにおいて基本的にはデグレはあってはならないため、大丈夫と自分を信じすぎず、常にテストを書くことを意識したいです。
コンパイル、テスト、コミット
リファクタリングでは小さなステップでコードを変更していく。そのため間違ってもバグを見つけるのは簡単である。（P9『statement関数の分割』より）
リファクタリングは、複数のことを一度にやらないことが重要です。万が一リファクタリングしてテストが落ちたら、切り戻して最初からやり直す。このペースで開発を行うことにより、リファクタリングにおけるバグの発生率を下げることができます。リファクタリングをしていると、あれもこれもそれもどれも直したくて、一気に大量の修正を伴うコミットを作りがちですが、コミット単位をわかりやすくするためにも意識したいと思いました。
リファクタリングと変更は分けること
ソフトウェアを開発しているとき、私は頻繁に二つの帽子をかぶり直しています。新たな機能を追加しようとしていると、コードの構造を少し変えれば簡単に機能追加できることに気づきます。そこでリファクタリングの帽子をかぶり直します。コードの構造が良くなったところで、また帽子を替えて機能追加を始めます。（P47『二つの帽子』より）
私も意識していることでした。具体的には、そもそも1つの機能追加のPRにあまり関係のないリファクタリング差分を入れないようにしています。これは（コメントにてリファクタリングという旨を伝えない場合はなおさら）PRレビュワーの認知負荷が高まってしまい、本来のレビューに集中してもらえなくなる可能性を感じているためです。コンパイル、テスト、コミットの部分でもそうですが、短く1つのことに集中するコーディングを意識するべきだと感じました。
ボーイスカウトルール
無駄なコードを見かけるたびに、少しずつ改善をしていけば、やがて問題はなくなっていくでしょう。リファクタリングの良い点は、小さなステップで作業を進めるためコードが壊れないことです。（P52『ゴミ拾いのためのリファクタリング』より）
メンテナンスされないまま放置されたコード群を見ると、つい自分もテストを書かなかったり、そのままのコードに継ぎ足したりしてしまったりする『割れ窓理論』の反対の概念だと感じました。また、あまりにもメンテナンスされていないコードを見かけると、気持ちがげんなりしてそのままにしてしまったり、つい「今回はこのままで、後でなんとかしよう」とサボったりしてしまうことがあります。コードとの付き合いが長くなると考えて、長い目で見て、少しずつ、1コミットからやってみる、という考え方をしてみるというのは、とても納得感がありました。
パフォーマンスとの闘い方
まずプログラムをきれいに整理された形で作り上げます。このとき、パフォーマンスは気にしません。そしてチューニングの段階にきて、初めてパフォーマンスを考慮します。（P67『リファクタリングとパフォーマンス』より）
乱暴な言い方をすれば、本書では一貫して「人間の読みやすさ重視でリファクタリングしていい。ほとんどの場合パフォーマンスの問題が起きることはごくまれ」のスタンスを取っているように感じます。上記の記述もその1つです。サーバーサイドと通信する立場であるフロントエンド寄りの場合はそうかも、と思いつつ、バックエンドについてはそのスタンスで良いだろうかと考えます。パフォーマンスの問題は後から発生するとボトルネックの発見に時間がかかる印象で、最初からコーディング時は一定遅くなる可能性がないかを意識して書く脳みそになっている気がします（特にバックエンド）。個人的には、どちらかというとパフォーンマンスに最適化されたコードを先に書いて、あまりにもシビアなコードになったらリファクタリングの脳に切り替える方が、自分は得意だと思いました。
リファクタリングの機運に気づくタイミングを理解する
『第3章 コードの不吉な臭い』では、こんなコードを見たら注意してほしい、という具体例がまとめられています。
不可思議な名前
名前の変更は、単に名前を付け替えるだけの作業ではありません。良い名前が思いつかないということは、設計がまだ固まっていないことの兆候でもあります。（P74『不可思議な名前』より）
名前に悩んだ時にぜひ思い出したいフレーズだなと思い、記憶に残りました。
仕様や設計が複雑になると、英字の名前だけでは意味が伝わり切らず、コメントを追加するケースがあります。そのような方法を取らない道を模索することを検討する癖をつけたいと思いました。
名前に関してはもう1つ思うところがあります。const a = xxxなんていう名前は論外だとして、よく練られた名前でも、時間の経過やコードの追加とともに最適ではない名前に変わってしまうことがあると感じます。命名をその時の状況に応じて最適化し直すリファクタリングの決断は、規模が小さいので後回しにしがちですが、ぜひ細かく注目しておきたいと思いました。
怠け者の要素
後に発展して広く使われるという想定で用意されたものかもしれませんが、その夢はかなわなかったのです。かつては役に立っていたものの、ときにはリファクタリングで存在意義が薄れていったものもあります。いずれの場合にせよ、そうした要素は丁寧に葬り去りましょう。（P82『怠け者の要素』より）
初めて作られて以降、メンテを介して長い時間を過ごしたコードに、よくありがちと感じます。リファクタリングの視点として、散らかっているものをまとめる観点は誰にでもあると思うのですが、逆にまとめていても意味のないものをインライン化するイメージは、あまり意識していないことでした。勉強になる視点だと感じました。
コメント
コメントの必要性を感じたときにはリファクタリングを行って、コメントを書かなくとも内容がわかるようなコードを目指すこと。（P87『コメント』より）
不可思議な名前と同じ視点です。さまざまな良いコードの例として、コメントはなんでもかんでも書くのではなく、補足的になぜを説明する時等に書くのが最適というのは、良くある話です。ですが、コメントを書かざるを得ないその状態を疑ってみる、コードベースでコメントがいらないかどうかを考えてみる、というのはなるほどという印象でした。
リファクタリング手法
『第6章 リファクタリングはじめの一歩』『第7章 カプセル化』『第8章 特性の移動』『第9章 データの再編成』『第10章 条件記述の単純化』『第11章 APIのリファクタリング』では、具体的なリファクタリング手法について紹介がされています。
関数の抽出
今も昔もこの業界でよく耳にするのが、いつコードを独立した関数として取り出すかという議論です。（略）しかし、最も納得できるのは意図と実装の分離です。何をしているのか調べなければわからないコードの断片があるとしたら、「何」をしているのかを示す名前の関数として抽出すべきです。（P112『関数の抽出』より）
ベタ書きになっている手続型の処理群において、一部の処理に名前をつけて切り出すことを、関数の抽出と言います。私たちにとって馴染みの深いリファクタリング手法であり、本書でも基本として最初に紹介をされていました。
勉強になったのは、引用した分離タイミングに関する考え方です。1つのことだけやらせる、というルールは聞いたことがありますが、共通認識が違ったり、細切れになりすぎたりその逆になったりして、安定しない指針だと感じていました。良い関数名を思いつけたら分離することをやってみたいと思います。
フロントエンドにおけるコンポーネント分離においても参考になる考え方で、印象に残りました。
変数の抽出
「変数の抽出」を検討するのは、コードの式に名前を付けたいときです。（P125『変数の抽出』より）
return order.quantity * order.itemPrice - Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 + Math.min ....;
上記のような魔法陣のようなコードではなく、人間が理解しやすいように1つ1つ名前をつけて切り出しましょうということです。人が読みやすくなることは自明ですし、デバッグのしやすさにもつながりそうです。変数に切り出す範囲は、関数の抽出でもあった名前をつけられるかどうかで判定するのが筋が良さそうです。
一方で上記に加えて、マジックナンバーの利用をやめろという話と同じように、変数はより小さなただの数字や文字列につけることも有用なので、たとえば上記でいう0.05が何かをわかりやすくしたければ、この手法を意識するのが良さそうです。関数の抽出は短すぎるとどこへ潜っても一向に結論が得られないコードになりがちですが、変数の場合は短くても大きな効力を発揮するように思います。
委譲の隠蔽
カプセル化によって、各モジュールはシステムの他の部分について知るべきことが少なくなります。何かが変更されても、影響を受けるモジュールの数は少なくなるため、変更がしやすくなります。（P196『委譲の隠蔽』より）
以下のコードは特にクラス化せずにconst manager = hoge.department.managerと書くこともできそうです。ただしクラスに閉じ込めることで、呼び出すモジュール本体のノイズは少なくなります。
const manager = parson.manager;
class Person {
get manager() {
return this.department.manager
}
}
特にAPI設計においてこの考え方が役に立ちそうです。バックエンドとフロントエンド間の通信や、外部連携先との通信等において、自分たちが欲しい理想の型と相手が要求する/返却される型が異なったりすることがあります。うまくモジュールに隠蔽することで、通信先の細かいパース事項やフィールド情報を1つのロジックに閉じ込めます。呼び出す側は自分たちが望むコードの形だけを意識すれば良いことになり、煩わしさふぁ減りそうです。
条件記述の分解
大きなブロックのコードに対しては常に、コードを分解し、それぞれを意図に沿って名付けた条件ごとの処理に対してこれを行うのがお勧めです。これにより、条件を強調し、何を分岐させているのかを名確認します。また、分離の理由も強調します。（P268『条件記述の分解』より）
条件分岐はその性質的に注意が必要です。1つ2つくらいであればなんら問題ないですが、多くなってくると途端に混乱していくものですよね。この感覚を忘れずに、条件分岐する場所は丁寧に意図を含めるのが良さそうと、本書を読んで思いました。
const charge = summer() ? summerCharge() : regularCharge();
ガード節による入れ子の条件記述の置き換え
私がif-then-else構文を使うときは、then節にもelse節にも同じウェイトを置きます。これにより読み手に対して、両方が等しく起こり得ること、および等しく重要であることを伝えます。逆に、ガード節は「シユような処理ではないため、起きた時には何がしかのことをやって脱出する」ことを伝えます。（P275『ガード節による入れ子の条件記述の置き換え』より）
1つ前の条件記述の分解と似た感じですが、ガード節を通して意図を明確にするという考え方が、なるほどと感じました。私はガード節はネストが深くならないようにする意図で使うことが多かったですが、if-then-else OR ガード節を上記のようなニュアンスを持ってコーディングできると、より表現力の高いコードになりそうです。
フラグパラメータの削除
フラグ引数は好ましくありません。どの関数呼び出しが使えるか、それをどう呼び出せばよいかを理解するプロセスが煩雑になるからです。（P323『フラグパラメータの削除』より）
以下はnameがフラグ引数になっているという好ましくないコード例ですが、その通りだと感じます。
function setDimension(name string, value string): void {
if (name === "height") {
this._height = value;
return;
}
if (name === "width") {
this._width = value;
return;
}
}
enumや真偽値がフラグのようなイメージで入ってくるのであれば、関数は分けるべきだと思いました。関数の抽出でも話した、責務は1つの方が良さそうという感覚であると同時に、何をしているかの命名が難しくなりがちという点も感じます（あるいは長くなったりもしそう）。フラグが入っている場合や、フラグにるさまざまな条件分岐が紛れ込んでいる場合には、積極的にリファクタリングで分離するようにしたいです。
おわりに
本書を読んで強く感じたのは、「リファクタリングは日常業務である」ということです。
リファクタリングと聞くと「汚いコードを綺麗にする」をイメージするのではないでしょうか。また、このニュアンスはあたかも過去のコントリビューターに対する冒涜に思えてしまいます。だから、リファクタリングについて開発者と会話する時は、多少忍び足になってしまう感覚がありました。
しかしそうではなく、コードは経年劣化するものであり、当時のコントリビューターの改修としてはベストだったものが、時が経つにつれて周辺の実装が変わるにつれて、適合しなくなり改修すべきものになっていくだけなのです。リファクタリングは本質的には、コードがいつまでも現在の状況に最適化されているために、絶えずリファクタリングと機能拡張等を行わなければいけないということを、強く感じました。
また、綺麗なコードは審美的観点だけで成立してしまっている場合は、リファクタリング対象になることもあります。まさに今日どこかで載せていた、不必要なまとめ方をされているコードは、インライン化すべき可能性、といった観点からです。
どんなプロダクトもリファクタリングするべき、ということを考えると、コードと向き合う時間がある限り本書を読む意味がたくさんありそうと思えました。
掲載したリファクタリングの知識や原則、手法については、ほんの一部です。
まだ読んでみたことがない方は、ぜひ本書に目を通してみてください。400ページを超える長編のため読み応えたっぷりであり、それゆえに読んだ方にとって納得した、面白い、意識したいと感じるリファクタリング手法、印象に残る考え方は、きっとそれぞれだと思います。
最後まで読んでいただき、ありがとうございました。
Discussion