メインコンテンツまでスキップ

リファクタリング

リファクタリング 最初の例

着手前のコメント

  • コンパイラと異なり、人間はコードの綺麗さを「気にする」
  • コンパイラがわかるコードは誰にでも書ける。優れたプログラマは人間にとってわかりやすいコードを書く
  • いつリファクタするか
    • コードの動作を理解して、流れを追わないといけない状況になった時には、リファクタが必要
    • コードが正常に動作していて、今後も変更をする予定が全くないときは、リファクタは不要

Step1. テストの作成

  • リファクタリングの第一歩は、まずテスト群を作り上げること。これは必須。
  • そうすれば、コードとテストの両方で失敗をしない限り、間違いはなくなる

Step2. 名前づけと構造化

  • コードを関数に抽出していこう
    • 意味のあるコードの塊を関数にして、何をしているかを端的に示す名前をつけていく
      • 間違った名前をつけると価値がなくなるので注意
    • これは、理解したことをコードに埋め込んでいく作業である
  • リファクタとパフォーマンスの捉え方
    • パフォーマンスの問題はほぼ発生しない
    • まずガンガンリファクタを進め、必要ならその後にパフォーマンス改善をやる。
  • 作業対象が複雑な場合は、なるべく小さなステップを刻んで作業していくこと
  • 一次変数を可能な限り排除しよう
    • 動機
      • 関数に抽出していく作業が楽になるから
        • 一次変数を使うと、その宣言場所と使用場所が「バインド」されてしまう(スコープを気にしなくてはいけない)
        • 一方、(問い合わせ)関数であればどこからでも呼べる
      • ルーチン内でしか使えないことにより、長く複雑なルーチンができがちだから
    • 手順
      • 問い合わせによる一時変数の置き換え p185
      • 変数のインライン化 p129
      • 関数宣言の変更 p130
  • 複雑なループ処理をリファクタしよう
    • ループの分離 p236
    • ステートメントのスライド p231
    • 関数の抽出 p112
    • 変数のインライン化
  • その他 tips
    • 関数の戻り値を表す変数名はresultにする
    • 型の名前を変数名に入れる(Performance型ならperformance
    • 不定冠詞(「不特定の 1 つ」を表す冠詞、aanなど)をつける
    • 変数名、関数名などをより良いものに積極的・継続的に修正する

Step3. 機能の変更

  • フェーズの分離をしよう
    • 一つのコードで異なる二つの処理を行なっている(or 行いたい)場合に、コードを段階ごとに分ける手法
      • 前段で整形や計算などの必要な処理を行なう
      • 後段に対して必要最小限の情報を受け渡す
    • 段階によって使用するデータや関数が決まっている場合に有効
      • e.g. コンパイラ
    • React でいうと下記のイメージに近いかも
      • 親コンポーネントで計算処理などを行う
      • 共通の props を受け取れる複数の子コンポーネントを用意する
      • 子コンポーネントを出し分けて見た目を切り替える
  • 方法
    • フェーズの分離 p160
    • 関数の抽出
    • 引数を中間オブジェクトに集約
    • 関数の移動 p206
  • リファクタと機能追加のバランス
    • 全てはバランス。リファクタのやりすぎも、やらなさすぎも、どちらもダメ。
    • ただし、最低でも「来た時よりも美しく」は守れ

Step4. 機能の追加

  • 方法
    • 「ポリモーフィズムによる条件記述の置き換え」の記載がある
    • しかし、これは今の時代には古いやり方な気がするし、単純に見づらい
    • 個人的には、以下のような関数型のアプローチ(ファクトリパターン)の方が好き
    • Qiita 参考記事
const performanceCalculatorFactory =
({ contextData, calcAmount, calcVolumeDiscount }) =>
() => ({
getAmount: () => {
const defaultAmount = contextData.price * 100;
if (calcAmount) {
return calcAmount(defaultAmount);
}
return defaultAmount;
},
getVolumeDiscount: () => {
const defaultVolumeDiscount = contextData.price * 200;
if (calcAmount) {
return calcVolumeDiscount(defaultVolumeDiscount);
}
return defaultVolumeDiscount;
},
});

const createCalculatorFor = (type, contextData) => {
switch (type) {
case 'tragedy': {
return performanceCalculatorFactory({
contextData,
calcAmount: (defaultAmount) => defaultAmount * 1.1,
// ここでは`calcVolumeDiscount`は上書きしていない
});
}
case 'comedy': {
return performanceCalculatorFactory({
contextData,
calcVolumeDiscount: (defaultAmount) => defaultAmount * 0.9,
// ここでは`calcAmount`は上書きしていない
});
}
default: {
throw new Error();
}
}
};

const main = () => {
const contextData = { price: 100 };
const calculator = createCalculatorFor('tragedy', contextData); // or 'comedy'
console.log(calculator.getAmount());
console.log(calculator.getVolumeDiscount());
};

まとめ

  • リファクタの良いループ
    • まずは、何が行われているかを理解できるようにする
    • 綺麗になるについてより踏み込んだ洞察が可能になり、さらに前向きな改善ループが続いていく
  • 良いコードかどうかは、変更がどれだけ容易なのかで決まる

リファクタリングの原則

リファクタリングの定義

  • ソフトウェアの内部構造を変化させること
  • ソフトウェアの理解や修正を容易にするために行う
  • 外部から見たときの振る舞いは保つ
  • いつでも中断が可能である
  • 小さなステップで行う

二つの帽子

ある時点では、どちらか一方だけをやること。交互に帽子をかけかえながら作業する。

  • 機能追加するとき
    • 新機能に関するテストの追加と、その実装のみを行う
    • 既存コードの再構築をしてはならない
  • リファクタリングするとき
    • 既存コードの再構築のみを行う
    • 新機能や、新機能のためのテストを追加してはいけない

リファクタリングを行う理由

良い設計(アーキテクチャ)を保つため

  • アーキテクチャが急速に劣化する負のループ
      1. 全体的な理解をせずに変更を行う
      1. コードが構造を失う (e.g. 無意味な重複コードが発生するなど)
      1. コードを読んで設計を把握するのが困難になる
      1. 1 に戻る

コードを理解しやすくするため

  • コードは書く時間より読まれる時間の方が圧倒的に多い、という視点が重要
  • わかりやすいコードを書けば:
    • 後から来た人は容易に短時間で修正ができる
    • 何を書いたか忘れてしまってよくなる。だって読めばすぐわかるから。

バグを見つけやすくする

構造が明確ならバグも無理なく発見できる

プログラミングを速める

  • デザインスタミナ仮説
    • 設計(≒ リファクタ)を入念に行えば、より長い期間、より早いペースで開発できる
    • リファクタせずに場当たり的な対応を重ねると、あっという間に走れなくなる

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

準備のためのリファクタリング

  • 機能を追加したり、バグを修正したりする前に行うリファクタリング
  • 森の中を真っ直ぐノロノロ歩いて目的地に突き進むよりも、森を迂回する高速道路を使って目的地行ったほうが、早いだろ

理解のためのリファクタリング

  • コード理解を妨げるものを取り除くリファクタリング
    • 不適切な名前
    • 長すぎる関数
  • 理解したことをコードに移していく作業と言える。これにより:
    • 意図を長期間保存できる
    • 仲間に伝えられる
    • 設計上の悪い点が見えてくる
  • 汚れた窓をまずはきれいにするようなもの。
    • この作業を、単に過去のコードをいじくっているだけと軽視する人は、混沌に埋もれた事実を永遠に見つけられない

ゴミ拾いのためのリファクタリング

  • 何をしているかは分かるものの、書き方が明らかにダメダメなコード
  • 「来た時よりも美しく」が大事。いつかは綺麗になるよ。
  • いま修正している時間がない場合は、せめてメモやコメントを残しておこう

計画してやるか、常にやるか

  • リファクタリングは常に行うもの
    • プログラミングと不可分
    • if 文を書くための専用の時間を取らないのと同じ
  • リファクタは全てのコードに必要
    • 酷いコードにも必要
    • 素晴らしいコードにも必要(<-ここ重要)
  • ソフトウェア開発は:
    • 累積のプロセスではない
    • 常に変容が求められるもの
    • 「完成」が存在しないもの

長期のリファクタリング

  • 1 週間以上かかるリファクタであっても、チームをリファクタリングに完全に専念させることはおすすめしない
  • 例えば 1 ヶ月くらいかけてチーム全体で徐々に変えていく合意をとった方が良い
  • これは、リファクタリングがコードを壊さないという利点を活かしている

コードレビュー時のリファクタリング

  • ペアプログラミング時にやるのがよい
  • Pull Request 形式のレビューではうまくいかない

管理者を説得するには

  • 理解のない管理者の場合、おすすめは「彼らには黙ってやる」
  • あなたがプロとしてベストと思うやり方を選択すればいい
  • 管理者も最も速く済む方法を望んでいるんだから、いいでしょ

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

  • 単なる API とみなせて、かつ今後の修正の必要がないとき
  • ゼロから書き直したほうが早いとき

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

新機能の実装が遅くなる?

  • 実装は遅くならない
    • なぜならリファクタの目的はプログラミングの速度を上げることだから
    • ただしトレードレードオフはあるので、このあたりの判断は経験がモノを言う。
  • リファクタする場合の例
    • 以後の作業が楽になるとき
    • 何度も同じ問題が起きているとき
    • 同じような醜いコードに何度も出くわしているとき
  • リファクタしない場合の例
    • 新機能が非常に些細なもので、リファクタを後回しにできるとき
    • めったに触らない箇所であるとき
    • 道徳的な理由でリファクタしてはならない
      • 「コードが美しくなる」だの
      • 「(しばしば原理主義的な)すばらしいプラクティスにコードを近づける」だの
      • 経済的な基準でのみ判断せよ

コードの所有権

  • コードの部分ごとにオーナーを設定し、変更を承認するようなやり方が良いのでは
  • これは以下の 2 つのあいだを取ったもの
    • 所有権が強すぎて、リファクタが困難な状態
      • e.g. 関数名を変えたときに、呼び出し元のコードを変更できない、など。こういったときは古い関数から新しい関数を呼び出すようにして、古い関数には deprecated を指定するなどするものの、最悪の場合、古い関数は永遠に残り続けることになる。
    • 所有権がなくて、誰もが好き勝手に変更を加えてカオスになる状態

ブランチ

  • Feature branch 方式はリファクタリングと相性がとても悪い
    • ブランチの生存期間とマージの困難さは指数関数的に相関する
    • マージが難しくなることを理由にリファクタを諦める場合もあるくらい
    • バージョン管理システムは意味的な変更にすこぶる弱い
      • 意味的な変更 ≒ リファクタリング。例えば関数名の変更など
  • Trunk-based なブランチ戦略がおすすめ
    • 1 日 1 回はメインブランチに統合する方法
    • ただし、大きな機能を小さな機能に分割したり、分割できない大きな機能を無効化する Feature flags の仕組みを作ったりするなどの準備が必要
    • Feature branch 方式と併用も可能。何れにせよなるべく頻繁にメインブランチにマージする。

テスト

  • テストは必須。テストがあれば:
    • リファクタが容易になる
      • 間違いにすぐ気づくので、見るべきコードの範囲が狭まるから
    • 新機能追加がやりやすくなる
      • 新たに埋め込んでしまったバグにすぐ気づくから
  • テストがない or 少ない場合は、IDE に安全が保証されている自動実行できる種類のリファクタであれば、実施できるかも
    • ただし注意深く手順に従う必要があるし、言語仕様にも依存するので比較的高度なやり方

レガシーコード

  • レガシーコード = テストのないコード、テストの追加も困難なコード
  • 「レガシーコード改善ガイド」を読めとのこと

データベース

  • 個々の変更は小さく、かつ完結したものにする
  • 正式リリースに至るまでに複数のリリースに分割する
    • 例えばフィールド名の変更であれば:
        1. 新規フィールドの追加(まだ使わない)
        1. 新旧両方のフィールドに書き込みを行うようコードを変更
        1. 古いフィールドを削除(使われていないことを確認した上で)

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

  • リファクタリングという武器を手にすると:
    • 現時点で判明している最低限の要件を満たすだけの、最もシンプルなコードを書きさえすればよい
    • 将来のために変に柔軟性を持たせたりしなくてすむ
      • e.g. 関数に無駄にたくさん引数持たせるなど
      • Yagni = You ain't going to need it = どうせ使わねえよ
      • リファクタリングできるんだから、必要になった段階で変更すれば足りる

リファクタリングとソフトウェア開発プロセス

  • 3 つのコアプラクティス(上から順にやるとよさそう)
    • 自己テストコード
      • CI とリファクタリングの前提条件
    • CI
      • リファクタリングの前提条件
      • ソフトウェアがいつでもリリース可能な状態であること
    • リファクタリング
  • コアプラクティスがもたらすもの
    • リリースを素早くできる
    • リリースのリスクを減らせる
    • リリースの技術的制約が減る
    • アイディアを最速で顧客に届けられる
    • ソフトウェアの信頼性を高める
    • 修正に時間のかかるバグを減らせる
    • Yagni を実践できる
  • アジャイル開発の必要条件
    • コアプラクティスが全て実践されていること
    • メンバーが:
      • リファクタリングの技能を持つこと
      • リファクタリングに熱心に取り組む人たちであること
      • リファクタリングを通常の作業に含めることに当然のように合意できること

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

パフォーマンス最適化には 3 つの方法がある

  • 実行スケジューリングする方法
    • 実行時間やメモリ使用量の制約を与えたうえで開発する
    • ハードリアルタイムシステム向き
  • パフォーマンスを常に気にしながら開発する方法
    • この方法はあまりうまくいかない。なぜなら:
      • パフォーマンス問題はごく一部のコードが引き起こすので、均等に努力しても大半は無駄に終わる
      • パフォーマンス最適化のためのコードが至るところに散らばる
      • コードがわかりにくくなる
  • きれいに作って後でチューニングする方法
    • まずはプログラムをきれいにつくる
      • この際パフォーマンスは気にしない
    • チューニングの段階まできたら以下を行う
      • プロファイラによる計測をする
      • 最も悪影響を及ぼしている箇所から順に最適化していく
      • この際、なるべく小さくステップを積み重ねること(コンパイル->テスト->計測)
    • 短期的にはパフォーマンス悪化があるかもしれないが、チューニング段階からは加速し、最終的には最短で目的を達成できる方法である

コードの不吉な匂い

  • リファクタを「いつ」始めるべきか?
    • 美意識といった曖昧な感覚ではなく匂いで判定しろ。以下、匂いのリスト。

不可思議な名前

  • 名前付けの対象
    • 関数名
    • 変数名
    • フィールド名
  • 適切な名前付けは:
    • 明快なコードにするために最も重要
    • 難しい
    • リファクタリングに置いて最も多く行われる作業
  • 適切な名前付けがでできれば:
    • コードがずっとシンプルになる
    • 将来の時間を大きく節約できる
  • 適切な名前付けができないときは問題を理解できていない兆候

重複したコード

  • 似ているけど違う部分はないかという間違い探しを毎回やる羽目になる
  • 関数の抽出 p112
    • 複数メソッドに同じ式がある時
  • ステートメントのスライド p231
    • 似ているけど完全に同一ではないときに、似た箇所を寄せておく。せめて。
  • メソッドの引き上げ p358
    • サブクラスに重複したコードがある時

長い関数

  • 小さな関数の寿命は最も長い。なぜなら以下が高いから。
    • 自己記述性
    • 共有性
    • 選択可能性
  • 関数の分割をすると読むときのオーバーヘッドが増えないか?
    • 関数名をわかりやすくすれば増えない
      • そうすれば中身を読まずに読み進められる
      • コードが何をするのかという「意図」を表す名前をつける
      • コードが長くなっても意図が明確になれば良し
  • やること
    • 関数の抽出 p112 (コレが 99%)
      • 重複コード、まとめられるコード、コメントがあるコード(≒ わかりにくいコード)を抽出する
        • わかりやすい名前をつけていくこと
        • 例えコードが 1 行でも必要なら抽出を躊躇しないこと
      • 巨大な switch 文の分岐後の処理を抽出する
      • ループ部分とループ内部を抽出する
        • 抽出した関数にうまく名前がつけられないときはループの分離 p236が必要かも
    • 問い合わせによる一時変数の置き換え p185
      • 一時関数が多すぎる場合
    • 引数が多すぎる場合
      • 後述の「長いパラメータリスト」を参照
    • コマンドによる関数の置き換え p345
      • 前述の全てをやっても、それでも一時変数やパラメータが残る場合
    • 条件記述の分解 p268
      • 条件分岐やループの記述が複雑なとき
    • ポリモーフィズムによる条件記述の置き換え p279
      • 同じような構造の swtch 文が複数あるとき
    • ループの分離 p236
      • ループ内で異なる別のことをやっている場合

長いパラメータリスト

  • 問い合わせによるパラメータの置き換え p332
  • オブジェクトそのものの受け渡し p327
  • パラメータオブジェクトの導入 p146
  • フラグパラメータの削除 p322
    • フラグパラメータは悪
  • 関数群のクラスへの集約 p150
    • これは関数型プログラミングでいうと部分適用された一連の関数群の作成にあたる
    • カリー化、部分適用についてはこちらがわかりやすい

グローバルなデータ

  • どこからでも変更できる、どこで変更したか知るすべもないデータ
  • 変数のカプセル化 p138により、どこで参照・変更されているのか把握した上で、範囲を狭めていく

変更可能なデータ

  • 変数のカプセル化 p138
    • 監視して改良する
  • 変数の分離 p248
    • 変数に複数の意味が持たされているとき
  • ステートメントのスライド p231関数の抽出 p112
    • 副作用のある処理(データの変更)のコードと、それ以外のコードに分けておく
  • 問い合わせと更新の分離 p314
    • 本当に必要なときだけ副作用のある処理(データの変更)を行うようにする。API での話。
  • setterの削除 p339
    • 変数のスコープを特定したうえで狭める
  • 問い合わせによる導出変数の置き換え p256
    • いつでも計算で導出できるのに変更可能になっているデータを取り除く
  • 関数群のクラスへの集約 p150関数群の変換への集約 p155
    • 変数の値を変更しなければならない箇所を減らす
  • 参照から値への変更 p260
    • 部分的に内部の値を修正せずに、まるっと入れ替える

変更の偏り

  • 「変更の偏り」が発生するのは、1 つのモジュールに混ぜ込むべきでない複数のコンテキストが含まれていることの兆候
  • e.g.
    • テーブル変更時に 3 つのメソッドを、商品の追加時に 4 つのメソッドを変更する必要がある、など
    • 本来、テーブルと商品の話は全く別のコンテキストなので、メソッドを分けるべき
  • 解消法
    • 処理順序が決まっている場合はフェーズの分離 p160
    • 呼び出し順序が前後するなら関数の移動 p206で処理を独立させていく
      • 内部がごちゃついているなら事前に関数の抽出 p112が必要かも
    • モジュールがクラスの場合クラスの抽出 p189で秩序だった形で分割できる

変更の分散

  • あちこちのモジュールを少しずつ書き換えなければならない状況のこと
  • 解消法は 1 つのモジュールにまとめること
    • 関数の移動 p206
    • フィールドの移動 p215
    • 似たようなデータを扱う関数群があるなら関数群のクラスへの集約 p150
    • データ構造の変換等を行う関数群は関数群の変換への集約 p155
    • 不適切に分割されたロジックには関数のインライン化 p121クラスのインライン化 p193

特性の横恋慕

  • あるモジュールの関数が、内部モジュールよりも外部モジュールとやり取りしている状態
  • モジュール化の原則は、内部でのやり取りは最大に、外部とのやり取りは最小にすること
  • 解消法
    • 多くの場合、関数同士が遠く離れていることが原因なので、関数の移動 p206で正しい場所に置く
    • 関数内の一部のロジックだけが横恋慕したがっている場合は関数の抽出 p112からの関数の移動 p206で正しい場所に置く

データの群れ

  • 数個のデータがつるんで、メソッドのシグネチャやクラスのフィールドに現れること
  • 解消法
    • クラスの抽出 p189でデータの群れをオブジェクトにする。その後に:
    • パラメータオブジェクトの導入 p146
    • オブジェクトそのものの受け渡し p327

基本データ型への執着

  • 貨幣、座標、範囲などを基本データ型で扱おうとするプログラマが後を絶たない
  • 解消法
    • オブジェクトによるプリミティブの置き換え p181した後に必要に応じて:
      • サブクラスによるタイプコードの置き換え p329
      • ポリモーフィズムによる条件記述の置換え p279

重複した switch 文

  • 全ての重複した条件分岐を探して更新していかなければいけなくなってしまう
  • ポリモーフィズムによる条件記述の置き換え p279を使え

ループ

  • パイプラインによるループの置き換え p240

たわけ者の要素

  • 過去にはなにか意味があったのかもしれないが、現在は何もしていないコードのこと
  • 関数のインライン化 p121
  • クラスのインライン化 p193
  • クラス階層の平坦化 p387

疑わしき一般化

  • 大した働きをしていない抽象クラス -> クラス階層の平坦化 p387
  • 意味のない委譲 -> 関数のインライン化 p121, クラスのインライン化 p193
  • 未使用のパラメータ -> 関数宣言の変更 p130
  • 消して通過しない分岐 -> 関数宣言の変更 p130
  • テスト時のみ使用されているプロダクトコード -> デッドコードの削除 p246

一時的属性

  • 限られた状況でのみセットされるインスタンス変数のこと
  • コードを非常に読みづらくする
  • 解消法
    • クラスの抽出 p189で一時的属性の居場所を作り
    • 関数の移動 p206で一時的属性を扱っているコードをクラスにまとめたのち
    • 特殊ケースの導入 p296(ポリモーフィズム)で条件分岐を排除する

メッセージの連鎖

  • 伝言ゲーム状態の関数群
  • 解消法
    • 委譲の隠蔽 p196で連鎖を隠蔽する
    • 最終使用者を関数の抽出 p112で取り出し、関数の移動 p206で連鎖を短くまとめる

仲介人

  • カプセル化というより、単に丸投げしているだけのクラスは仲介人の除去 p199する
  • コード量がわずかなら関数のインライン化 p121もよい

インサイダー取引

  • モジュール同士が意図せず密結合になっている状態
  • 解消法
    • 単純に分離できるなら関数の移動 p206,フィールドの移動 p215
    • 共通の興味を持つなら
      • 第三のモジュールを共通データの管理役とするか
      • 委譲の隠蔽 p196
    • 継承が原因の場合は
      • 委譲によるサブクラスの置き換え p388
      • 委譲によるスーパークラスの置き換え p407

巨大なクラス

  • 多すぎるインスタンス変数
    • クラスの抽出 p189により細分化する
      • 同じ接頭詞で始まる変数はまとめやすいかも
  • 継承でまとめられる部分があるなら:
    • スーパークラスの抽出 p382
    • サブクラスによるタイプコードの置き換え p369
  • クラスのクライアントがサブセットだけ使っている場合は分割の良いサイン

クラスのインターフェース不一致

  • インターフェースを揃えましょう
    • 関数宣言の変更 p130
    • 関数の移動 p206

データクラス

  • getter と setter しか持たないクラス
    • 関数型プログラミングでいうと、型はあるけど型に適用できる関数群がない状態
  • 間違った箇所に振る舞いが記述されていることの兆候
  • 解消法
    • レコードのカプセル化 p168でなるべく中身を隠す
      • immutability が担保されているなら不要
    • 変更されて困る属性にはsetterの削除 p339
    • 使用箇所を調べて関数の移動 p206関数の抽出 p112によりクラスに振る舞いを移す

相続拒否

  • 継承階層が間違っていることの兆候
  • 解消法
    • 振る舞いが再利用されていない場合には、兄弟クラスを新たに作成して、再利用している連中だけをそちらに移す
      • メソッドの押し下げ p367
      • フィールドの押し下げ p368
      • (問題が出ていないならほっといても OK)
    • インターフェースが再利用されていない場合には
      • 委譲によるサブクラスの置き換え p388
      • 委譲によるスーパークラスの置き換え p407

コメント

  • リファクタリングすることでコメントがなくても分かるコードを目指せ
    • 関数の抽出 p112で細かい単位に分ける
    • 関数宣言の変更 p130でわかりやすい名前にする
    • アサーションの導入 p309でルールを強制する
  • コメントは以下の目的で使うとよい
    • 疑問点を書き留めておく
    • 処理自体の説明ではなく分かりづらい事項の説明をする
    • その処理を選択した理由を書く

テストの構築

以下、重要部分のみ抜粋

  • 自己テスト
    • 完全に自動化された、テスト自身が結果をチェックするテストのこと
  • TDD のメリット
    • すべきことは何かを事前に整理せざるを得ないこと
    • 実装よりもインターフェースに集中せざるを得ないこと
    • コーディングの完了時点が明白であること(テストに通過したとき)
  • フィクスチャ
    • テストに必要となるデータやオブジェクトのこと
  • test関数に与える説明文の粒度
    • 失敗したときにどのテストか識別できる程度に書くとよい
    • 説明しすぎは冗長なコメントのようなもの
  • どこに、どれくらいテストを書くべきか
    • 全てのコードにテストを書こうと思わないこと
    • 以下を集中的にテストすること
      • 怪しいと思うところ
      • 複雑なところ
      • エラーが起こりそうなところ
    • 無意味なテストをするな(例えば単なるセッターのテスト)
  • fixture をテスト間で共有するな
    • 大変なトラブルのもとになる
    • beforeEach()を使って分離せよ
// Bad
describe('', () => {
const asia = new Province();
// many test cases go here..
});

// Good
describe('', () => {
let asia;
beforeEach('', () => {
const asia = new Province();
});
// many test cases go here..
});
  • テストの構成
    • よくあるのは以下のパターン
        1. fixture を共通した方法でセットアップし
        1. テスト文で少し改変したうえで
        1. 検証する
    • これには色々な呼び名がついている
      • Set up - Exercise - Verify
      • Given - When - Then
      • Arrange - Act - Assert
    • Tear down というフェーズもあるがこれを使うのはレアケース
      • e.g. 生成にあまりに時間がかかるため、テスト間で共有せざるを得ない fixture を扱っている場合など
  • 境界値の検査
    • 当たり前のケースのテストは「ハッピーパス」という
    • 通常想定されないケースや境界値でのテストも大事。境界値とは:
      • 集合なら空の時
      • 数値なら 0 や負の値の時
      • 文字列なら空文字の時
  • 通常想定されないケースでランタイムエラー(e.g. *** is not a functionなど)が出たときの選択肢
    • より適切なエラーを返す
    • ログを書いてから親切に値を初期化する
    • 何もしない
      • モジュール間でたくさんのチェックを設けると冗長化して逆にトラブルのもとになることもある
      • 外からくる信頼できない値に対してはちゃんとチェックは必要だけどね
  • アサーションのテスト
    • 書かない。アサーション自体がテストだから。
  • リファクタを始めるときにどれくらいテストを書くか
    • ある程度テストを書く
    • あとは作業の進度に合わせてテストを追加する
  • バグレポートを受け取ったら、まず、そのバグの存在を明確に示す単体テストを書くことから始めよ
  • カバレージによる分析
    • コードの中のまだテストされていない箇所を突き止めるために使う
    • テスト自体の品質を保証するものではない
  • テストの品質は品質は主観で決めるのが良い
    • 誰かが変な変更をしたときにテストは失敗すると確信がもてるか?という視点が大事
  • テストの書きすぎ
    • テストコード自体の修正に時間がかかり開発が遅れているなら、書き過ぎの兆候かも
    • ただしテスト過多になるケースはほとんどない

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

関数の抽出 p112

  • 目的
    • 意図と実装を分離するため
      • 意図 - 「何」をしているか。関数名でわかりやすく表す
      • 実装 - 「どのように」目的を達成するか
  • きわめて頻繁に行われるリファクタリング
  • 意図がわかりやすくなるなら、コードが例え数行でも躊躇なく
  • 関数内に関数が入れ子になっても OK

関数のインライン化 p121

  • 目的
    • 不要な間接化を取り除くため
  • いつ使うか
    • うまく分割できていない関数群を一旦リセットするとき
    • 別の関数を呼び出すだけのバケツリレー関数を一旦リセットするとき

変数の抽出 p125

  • 目的
    • 意図をわかりやすくするため
  • 何をするか
    • 複雑なロジックの式の一部に名前をつける

変数のインライン化

  • 目的
    • 無意味な変数をなくすため

関数宣言の変更

  • 目的
    • 関数名を改善することで、わかりやすく意図を伝える
    • パラメータを改善することで、外部と疎結合にする
  • 方法
    • 全部を一度に済ませる方法と
    • 順を追って行う「移行的手順」がある
      • 仮の新関数で旧関数をラップし、徐々に仮の新関数を呼び出すように移行したうえ、移行が完了したら最後に名前をもとどおりにする方法

変数のカプセル化

  • 関数を介して変数を操作することで、変更を容易にしたり監視を可能にしたりする
  • データは関数よりも操作しにくい
  • データの再構成という困難な作業を、関数の再構成というより簡単な作業に変える
  • 方法
    • getter や setter を介して値を操作するようにする
    • react のuseStateが限りなく近い
    • もう少し突っ込んでやるなら「レコードのカプセル化」までやる

変数名の変更

  • 読んで字のごとく、変数の名前を変えること
  • 以下を満たす場合には、変数のカプセル化も合わせて実施するとよい
    • 変数のスコープが 1 つの関数より広い
    • 変数が更新されている

パラメータオブジェクトの導入

  • 複数のパラメータをまとめてオブジェクト(クラス)にする
  • メリット
    • パラメータがスッキリする
    • パラメータ間の関係を表現できる
    • 一貫性が向上する
    • パラメータに振る舞いをもたせることができる

関数群のクラスへの集約

  • 以下を一つのクラスにまとめること
    • データ
    • データをもとに派生値を算出する関数
  • 関数型プログラミングにおいては「入れ子関数」で同様のことができる
    • (しかし、結局のところシリアライズ不可能なオブジェクトになってしまうので、関数型プログラミングとは言えない?)
    • (関連する関数を単一のオブジェクトにまとめておく、だけでも良いのではないか?)

関数群の変換への集約

  • データ変換関数を用いて、もとのデータに派生値をトッピングしたデータを作る方法
    • データ変換関数 = enrichSomething()transformSomething()など
  • 扱い方を間違うと、もとのデータが変更されたときに不整合が起こりがちなので注意
  • (こっちのほうが関数型プログラミングと親和性が高そう)

フェーズの分離

  • 以下を満たすコードを含む関数を分離する
    • 異なる 2 つ以上のトピックが一つの関数に含まれている
    • 異なる 2 つ以上のトピックが時系列として前後に分離できる
  • 中間データ構造を使いながら移行を進める。詳細は本書 p161 参照。

カプセル化

レコードのカプセル化

  • プレーンなオブジェクトをクラスインスタンスにしたうえ、各レコード(プロパティ)を Setter、Getter 介さないと操作できないようにする
  • (型、関数型プログラミング、Immutability 全盛の時代には少々時代遅れ感がある)

コレクション(配列等)のカプセル化

  • getter でコレクションそのものを返してしまうと、コレクションを保持するクラスを介さずに、その中身を変更できてしまう
  • これを防ぐために、get, add, remove の 3 つの関数を用意する
  • get ではslice()等を使って複製してから返すことで変更されるのを防ぐ
    • あとでハードなデバッグをすることと比べたらパフォーマンス問題なんて微々たるもの
  • (型、関数型プログラミング、Immutability 全盛の時代には少々時代遅れ感がある)

オブジェクトによるプリミティブの置き換え

  • 最初は単純なデータに見えるものでも、あっという間に関連処理が増える
    • e.g. 電話番号であれば、整形や市外局番の抽出処理など
  • なのでプリミティブな値やその処理のまとまりを見つけたら、速やかに値オブジェクト or 参照オブジェクトにしちゃいましょう

問い合わせによる一時変数の置き換え

  • 一時変数を問い合わせ関数にすると:
    • 関数の抽出がやりやすくなる
    • 計算ロジックの重複を避けられる
  • クラスの中で行うのが一番最適
  • 副作用がある場合は事前に分離しておく

クラスの抽出

  • クラスの中から、共通性のある一部のプロパティ・メソッドを別のクラスに抽出すること
  • 常にこれを心がけておかないと、あっという間にクラスは肥大化していく

クラスのインライン化

  • クラスの抽出の逆
  • もはや不要になったクラスを捨てるときや、リファクタ前に一旦全部フラットにしたいときに使う

移譲の隠匿

  • ある関数が他の関数等に処理を委譲していたとしても、利用者にはそれを見えないようにすること
  • 余計なことを利用者に知らせないという、カプセル化の手法の一つ

仲介人の除去

  • 「移譲の隠匿」の逆
  • 隠匿が単なる仲介に成り下がっている場合は、いっそ利用者に委譲先を見せてしまったほうが具合がいい場合もある
  • 要はバランス

アルゴリズムの置き換え

  • よりよい方法が見つかったらまるっと置き換える
  • 置き換え前後で結果に差異がないことを確認すること

特性の移動

関数の移動

  • 前提
    • 全ての関数は何らかのコンテキストに置かれる
    • コンテキスト ≒ ドメイン ≒ データとふるまいのまとまり ≒ クラス
  • 手法
    • 関数の呼び出し元、呼び出し先を確認し、最も適したコンテキストに移動する
    • この際、新たにコンテキストを作成したほうがよいこともある

フィールドの移動

  • データ構造は超重要
  • データ構造が正しくないとわかったらすぐに変更せよ
  • やり方
    • 移動元のデータをカプセル化して全ての参照・更新処理を事前に明らかにしておく
    • 新しい場所にデータを移す
    • DB の変更を伴う場合は段階的移行が必要

ステートメントの関数への移動

  • ある関数の呼び出しと必ずペアで呼ばれているコード(重複しているコード)があるなら、関数に入れ込む
  • やり方
    • 呼び出し元で重複しているコードを一時関数として抽出する
    • 全ての重複コードを抽出した一時関数に置き換えていく
    • 全ての置き換えが完了したら、抽出した一時関数の中に元の関数をインライン化する
    • 一時関数の名前を元通りにする(ワーオ完成〜すごい〜)

ステートメントの呼び出し側への移動

  • ステートメントの関数への移動の逆
  • ここでも一時関数を使った手法が有効

ステートメントのスライド

  • コードの場所を前後に移動すること
  • スライドできるかどうかは注意深く確認する必要がある

ループの分離

  • ループの中でコンテキストの異なる処理を行っている場合は、ループを分離する
  • 分離自体は目的ではなく、その後において分離したそれぞれの処理を関数に抽出するなどしてリファクタするまでがセット

パイプラインによるループの置き換え

  • ループ処理には for 文よりも map/filter/reduce などのパイプライン処理の方がよりシンプルに書けることが多い
  • 方法
    • 別の変数を前段に用意して、そちらに徐々に処理を移していく

デッドコードの削除

  • 未利用のコードを消すこと

データの再編成

変数の分離

  • 変更可能な変数を各所で上書きしながら使い回すな
  • 変更不可能な変数を別途定義せよ(Immutablility)

フィールドの変更

  • データ構造は最も大事、よってフィールドの名前も大事
    • フローチャートなんぞ見ても何もわからんが DB 設計見れば一発で分かる
  • 方法
    • リネームするだけ
    • 規模が大きいならクラスを使ってカプセル化して段階的に移行する方法もある

問い合わせによる導出変数の置き換え

  • 計算で算出できるなら状態として持つな

参照から値への変更

  • 入れ子のオブジェクトは、可能な限り参照ではなく値として持て
    • 参照 - オブジェクトのプロパティを更新する
    • 値 - オブジェクトをまるごと置き換える

条件記述の単純化

条件記述の分解

  • 長い関数は読みにくい
  • 条件分岐に関する長い関数は超読みにくい
  • 方法
    • 条件文と分岐後の処理を、それぞれ関数として独立させる
    • 三項演算子で仕上げるとワンダフルよ

条件記述の統合

  • 分岐は複数定義されているが、結果が同じものがある場合は、統合せよ
  • 方法
    • 条件文を関数化して集約したうえで処理文を一つにまとめる

ガード節による入れ子の条件記述の置き換え

  • then/else は、どちらも正常系で等しく起こりうる場合に使う
  • そうでない場合は、例外的な条件においてはガード節で早々に離脱させる
  • 条件文を反対にすることでガード節を作りやすくなることがある(詳細は本文参照)

ポリモーフィズムによる条件記述の置き換え

  • if や switch では複雑になりすぎる場合に利用を検討する
    • オブジェクトの種類によってコードのいたるところで条件分岐が散在している場合など
  • (読んだけどやはり時代遅れ感あり。fp ではどうやるんだろう)

特殊ケースの導入

  • Special case pattern, もしくは Null object pattern ともいう
  • やり方
    • 「変換」を使って予め例外なパターンのデータを必要に応じて埋め込んでおくことで、利用側を楽にさせてあげる
    • 詳細は本文参照。
    • (クラスを使った例も書いてあるが今の時代に使えるものはなさそう)

アサーションの導入

  • メリット
    • エラーの早期発見につながる
    • コミュニケーションのツールになる
    • デバッグが楽になる
  • 使いすぎは危険
    • 真である必要がある箇所にのみ使う
    • プログラマのエラーに対してのみ使う
    • バリデーションなど、値のチェックはアサーションではなくプログラムに書くこと

API のリファクタリング

問い合わせと更新の分離

  • 問い合わせは蓋然性が高く、どこにでも移動でき、テストも用意
  • 値を返すにもかかわらず、観察可能な副作用があるメソッドがあったら、必ず分離する
    • 観察可能 = 例えばデータ取得時にキャッシュ層で値が変わるのは観察不可能なので許容される

パラメータによる関数の統合

  • ほとんど重複コードな関数群があって、違いは使われている定数がちょっと違うだけの場合は、定数をパラメータで与えることで関数群を統合せよ

フラグパラメータの削除

  • フラグパラメータ(フラグ引数)は使うな
  • 理解しにくいから
  • 特に bool リテラル値は最悪
    • ただし bool がデータ型として与えられている場合はフラグ値とはみなさない場合もある
  • フラグ引数が多すぎる場合は問題を整理できていない可能性あり

オブジェクトそのものの受け渡し

  • オブジェクトから数個の値を抜き出して関数に渡しているなら、オブジェクトそのものを渡す
  • ただしこのリファクタをしたくないこともある
    • 例えば依存関係をもたせたくない場合など
    • そういうときは「特性の横恋慕」が疑われるので、ロジックの場所を移すべきではないか、考えてみるとよい

問い合わせによるパラメータの置き換え(またはその逆)

  • (順)パラメータで渡すまでもなく問い合わせで計算可能なものは、削除する
  • (逆)関数内部で行われている問い合わせを、パラメータに置き換える
  • どちらがいいかは状況による
  • 一つ言えるのは、参照透過性(パラメータが同じなら結果が同じ)は大事だということ
  • よくあるパターンは、参照透過性を持つ純粋関数群を作っていき、それらをラップして副作用のある関数と組み合わせて使うというもの

setter の削除

  • 不要な setter は消して可能な限りイミュータブルにしようね

ファクトリ関数によるコンストラクタの置き換え

  • コンストラクタはいろいろな制約があるからラップして使え、とのこと
  • (時代遅れ感)

コマンドによる関数の置き換え

  • クラスを使うことで、入れ子関数と同じようなことを実現する
  • (入れ子関数でよくないか?)
  • 「関数によるコマンドの置き換え」という逆パターンもある

継承の取り扱い

(省略)