積読解消プロジェクト「リファクタリング（第2版）既存のコードを安全に改善する」Part1

はじめに

株式会社ｉｉｍｏｎアドベントカレンダー7日目担当「たっくー」です！！！

会社ではフロントエンドを主に担当しています。

いやー、あっという間に12月ですね〜

年末といえばすることは一つですよね！！

そう、大掃除！！

大掃除をしていたら積読していた「リファクタリング」(https://www.ohmsha.co.jp/book/9784274224546/)という本を部屋の片隅で見つけました。

本屋でパラッと立ち読みしていい本だな〜と思って購入したのは覚えているのですが、その後の記憶がないまま積んだままになってしまっていました。

このままだとずっと積んだままになってしまうなと思い、このアドベントカレンダー期間を利用して読むことにしました。

日頃からTypeScript をつかう自分にとってこの本はサンプルコードがJavaScript で書かれているというのも気を引く一つの要因です。

この本はリファクタリングのサンプルコードが全体の4分の3を占めていて、他は主にリファクタリングする理由や機会について書いています。ボリュームが多く、1回のブログでは全部まとめきらないので途中までの部分で自分が特に勉強になるなという部分についてまとめました。ほぼ自分用のメモです。

個人的にリファクタリングについて思うこと

本のまとめに入る前に自分が感じているリファクタリングについて思うことを書いていきます。

リファクタリングをすると可読性があがるとか運用しやすくなるとかいろいろ言われてきていると思いますが、個人的には人のコードをレビューするときにリファクタリングの技術が必要だと思うことが多々あります。レビューもらったコードはたいてい、実現したい処理やUIはできていてレビュワーができることの多くは担当者が気付かない思いがけないバグの可能性の指摘とか、リファクタリングの仕方だと個人的に思っています。

担当者が気付かない思いがけないバグの可能性はそのプロダクトのコードを長く書いていれば、指摘できる箇所が多くなっていくと思います。

けど、リファクタリングの技術をつければプロジェクトに参画してすぐでもレビューに参戦できるので即戦力として活躍することも可能なのかなって思います。

リファクタリングの技術はエンジニアとって最強の武器となりえるといえるでしょう。

リファクタリングの大切さがわかったところで本のまとめに入っていきましょう。

リファクタリングの原則

リファクタリングの定義

本書では名詞としてのリファクタリングと動詞としてのリファクタリングがあるとしていて、

名詞としてのリファクタリング

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

動詞としてのリファクタリング

一連のリファクタリング(名詞)を適用して外部から見た振る舞いの変更なしにソフトウェアを構築すること

リファクタリングはコードをきれいにするあらゆる作業を表すものとしてあいまいに使われてきましたが、上記の定義をふまえるとリファクタリングはコードをきれいにするための特定の手法であることがわかります。振る舞いを保ちつつ、小さなステップを適用してステップを積み重ねて大きな変化をもたらすことだと定義できます。

この結果から言えることはリファクタリングではコードが壊れた状態になっている期間は非常に小さくなっているべきであり、未完成でもいつでも中断可能であるべきです。

p.45~46 リファクタリングの定義 一部抜粋

リファクタリングをする理由

開発者が短期的な目的のためにプログラムの内部の設計(アーキテクチャ)を理解せず、変更をするとコードは構造を失います。こうなると設計が把握しずらいコードになり累積的に悪影響がでます。これを防ぐためリファクタリングをしてコードをよい状態を保ちます。

たとえば、リファクタリングの1つに重複したコードを省くという手法があります。

これは設計を改善するための重要事項といえます。コード量を減らしてもシステムが早く動作するわけではないですが、コード量が増えるほど正しく修正するのが難しくなります。

コードのある部分を修正しても他の箇所でも少し違った文脈で同じようなことをしており、その部分の修正を忘れてしまうものです。重複部分を排除するとただの1回のみ書かれ、そこですべてが処理されることだと保証されます。

p.47 リファクタリングはソフトウェア設計を改善する 一部抜粋

→期日にゆとりがある開発はじっくりとリファクタリングしがちですが、ユーザーが困っていてすぐにでも修正しないといけない時はとりあえず動くようにするという短期的な目的を達成するために コードがぐちゃぐちゃになるというのはよくあることだと思います。とりあえず動くコードを書いたときは一旦リリースしたあとにじっくりとリファクタリングをする習慣やチームでルールを定めるのがいいでしょう

リファクタリングはプログラミングを速める

リファクタリングは品質を高めることは明白です。

では開発のスピードはどうでしょうか。

よく当初はすばやく開発することができたけど新たな機能を付け加えるのにずっと時間がかかる既存ベースコードに新機能をどう適合させるか把握するのに費やす時間が、徐々に増えていき追加したとしてもバグが起こり修正にさらに時間がかかるようになっていきます。

これを重ねると読みづらいコードになりどんどん機能追加するスピードが遅くなっていき、いっそ開発当初に戻りたいと思うようになります。自分も非常に身に覚えがあるケースです。

これはだいたいまずい設計で進めた場合になります。

まずい設計のグラフの図(本書参考P.49)

これに対して内部の設計が優れていれば、新たな機能を追加するときにすぐに把握できたり、うまくモジュール化されてれば、変更するコードベースで理解しなければならない箇所は小さく限定できます。また、コードが明確ならバグを埋め込んでしまう可能性も低いし、バグが生じたとしてもデバッグがずっと簡単になります。

まずい設計とすぐれた設計のグラフの図(本書参考P.50)

内部の設計を入念に行えば、ソフトウェア開発のためのスタミナをつけていくことができます。(デザインスタミナ仮説)

昔はコードを書く前に設計が完了していなければならなかったので後は劣化していくばかりでしたが、今では既存のコードの設計も改善していけることがわかっています。

すぐれた設計を前もって完了することは非常に難しいので、すばやく機能を追加し続けるにはリファクタリングが不可欠です。

より詳しくリファクタする理由を考える

1.機能追加を容易にするため

既存のコードに機能を追加する前はリファクタをするのにうってつけのタイミングです。

追加機能と同じような関数があるけど、リテラル値が埋め込まれていてその関数を使えない時、リファクタリングをしないとその関数をコピーして、リテラルを修正することになります。

そして将来、また新機能を追加するときに同種のバリエーションを追加するときにはコピー＆ペーストで対応できなくなります。しかしリファクタリングすれば、変えたい部分を引数にして関数を呼べばいいだけになります。

また、重複したコードを排除することでバグの発生を抑えるとともにバグの修正も容易になります。

p.51 準備のためのリファクタリング 一部抜粋

→タスクをどのくらい期間で修正できるかを測る見積もりの段階で、機能追加または修正する箇所の前後の流れをコードリーディングをしなくてはいけません。 その時に同じような関数があるとあちこち見なくてはいけないので見積もりの段階でも大幅に時間をロスすることになります。

2.理解のため

コードが何をしているかわからないときは一目でわかるようにリファクタリングします。

リファクタリングで変数名を修正したり、長い関数を小さく分割したりすると前は気づかなかった設計上の考慮すべき事項が把握できるようになります。リファクタリングを軽視する人は埋もれた事実を見つけるための絶好の機会を失っていることに気づいていないのです。

p.51~p.52 理解のためのリファクタリング

→要は可読性を高めるリファクタリングのことをこの章では言っていますが、コーディングするときは書くより読む時間が長いと言われており、今はなおさらAIがあるのでコーディングはなんとかなりますが、仕様などの把握やコードの流れを読むのはAIでは一苦労です。ここが変数名などである程度想像できるようになれば、開発がぐっと早くなります。 後述していますが、ループ文をパイプライン操作で置き換えるとデバッグをせずともある程度、処理が一目わかるようになり開発時間の節約になります。

3.ゴミ拾いのため

リファクタリングしたい箇所を見つけても、しかもそのリファクタリングに数時間もかかる場合、今とりかかっているタスクに直接関係のないことに時間を奪われたくないことがほとんどです。

しかし、少しずつでもリファクタリングしていくことには価値があります。これを続けていけばやがて問題はなくなります。リファクタリングの良い点は小さいステップで作業を進めれられるのでコードが壊れないことです。

p.52 ゴミ拾いのためのリファクタリング 一部抜粋

→この本ではキャンプ場のルールである「来たときよりもきれいにして帰ろう」をリファクタリングにおいてのルールとしても定めております。

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

リファクタリングに時間をかけると新機能の実装が遅くなるという認識がいまだ、多くみうけられます。この認識がリファクタリングを行うのを妨げている主要な原因かもしれません。

リファクタリングの目的は少ない労力で多くの価値を生み出すべくプログラミングの速度をあげることになります。

しかし、最も危険なのは罠と思うのは「美しいコード」「すばらしいエンジニアリングのプラクティス」といった道徳的理由により、リファクタリングが正当化される状況です。リファクタリングはコードベースがどれだけ美しいかではなく、純粋に経済的な基準で測られるものです、リファクタリングをするのはあくまで開発スピードをあげるため、新機能の追加やバグの修正を速めるためです。

p.56~57 新機能の実装が遅くなる 一部抜粋

→本書のこのパートではリファクタをする、しないはあくまで主観的な判断に頼るしかないと書いてあり、著者の周りではリファクタリング過多よりもリファクタリング不足のケースをたくさん見かけると書いてあります。本書にも似たようなことが書いてありますが、この主観的な判断はよりよい現場や勉強のなかで良いコードを見ることで健全な判断が養われていくのでしょう。

→可読性のためにチームで扱うコードはある程度、コードルールに沿って開発するべきだと思いますがレビュワーのエゴの部分で修正依頼して時間をかけるというのは、本末転倒ですね。

リファクタリングを行うタイミングについて

本書ではコードの不吉な匂いを感じたらリファクタリングをするべきだとかかれています。

そのコードの不吉な匂いのパターンがいくつも書かれているので、そのうちの何個かを紹介していきます。

ここで紹介するいくつかのサンプルコードは本書に書かれているものではありません。

不可思議な名前

明快なコードにするための最も重要なのは、適切な名前付けです。そのため開発者は関数、モジュール、変数、クラスなどの名前について行っていることや利用方法がはっきりと伝わるように懸命に考えます、しかし名前つけはプログラミングで難しいこととされています。

一度決めた名前を変更することは、開発者はそれほど問題ではないと考え、名前の変更は躊躇してしまいます。しかし、適切に名前を変更してしまえば将来、何時間もかけて思い悩まずに済みます。

名前を決める、変更する際にいい名前が思いつかないということは、設計がまだ固まっていない予兆でもあります。いま一つな名前をどうにかしようと頭をひねることで、コードがずっとシンプルになっていくこともよくあります。

　p.74 不可思議な名前 一部抜粋

→開発で可読性を上げようと変数名で全部つめこんで、長くなってしまい結局何を表したいのかわからないということがあります。これはリファクタリングがうまく行っていない証拠です。 たとえば、ユーザーが現在のセッション中に購入した商品の数を表す変数としてnumberOfItemsPurchasedByUserInCurrentSessionをつかうとします。 これでは変数が長いし、細分化できるよう務めるか適度な抽象化と省略をするべきです。

重複したコード

同じコードが2箇所以上あるときは、1箇所にまとめることができればより良いプログラムになります。

コードに重複があるとコピーされた箇所に出くわすたびに似ているけど違う部分はないか、注意深く読み込まないといけなくなります。重複したコードを修正することになった際には重複部分を漏れなく見つけて、すべて同様の修正を施していく必要があります。

p.74 重複したコード 一部抜粋

本書では「関数の抽出」を適用するのが一番べーシックなパターンで、全く同じではない場合は「ステートメントのスライド」という手法をつかいコードをまとめてから 「関数の抽出」をするという手法が紹介されています。

「関数の抽出」 + 「ステートメントのスライド」(サンプルコード)

会員のステータスごとの値段を決めるメソッド2つ

before

function calcSilverDiscount(price) {
  // Silver：割引率 5%
  const discount = price * 0.05;
  const finalPrice = price - discount;
  return finalPrice;
}

function calcGoldDiscount(price) {
  // Gold：割引率 10%
  const discount = price * 0.10;
  const finalPrice = price - discount;
  return finalPrice;

割引率(リテラル値)だけ違うほぼ重複コードであり、割引率ロジックを変えたいとき2つのメソッドを見なくてはいけない。

after

①「ステートメントのスライド」

function calcSilverDiscount(price) {
  const rate = 0.05;　// ここだけ上にスライド
  const discount = price * rate;
  const finalPrice = price - discount;

  console.log("Silver discount applied:", discount);
  return finalPrice;
}

function calcGoldDiscount(price) {
  const rate = 0.10;　// ここだけ上にスライド
  const discount = price * rate;
  const finalPrice = price - discount;

  console.log("Gold discount applied:", discount);
  return finalPrice;
}

両関数の 同じ部分を揃えて、「関数の抽出」をしやすくする準備をする。

②「関数の抽出」

function calcDiscount(price, rate) {
  const discount = price * rate;
  const finalPrice = price - discount;
  return { discount, finalPrice };
}

割引計算のロジックを抽出して共通化 する。

これで割引計算に関連する機能追加のときも2つのメソッドを見る必要もなくなるし、他のランクにも簡単に対応できるようになります。 このリファクタリング手法は日頃なにも考えずにできている気がしますが、手法として言語化されていると改めて勉強になります。

長い関数

関数が長くなるほど理解しにくくなると言われてきました。古いプログラム言語だとサブルーチンの呼び出しにかかるオーバーヘッドが無視できず、コードを複数の呼び出しに分割すると嫌がられました。最近の言語では呼び出しにかかるコストはプロセス内でほとんど問題になりません。

しかし、それでもコードを読む開発者も次から次へと関数を追っていかねばならず、別のオーバーヘッドがかかるという意見もあります。しかし最近だともっとも使われてるであろう？vscodeみたいな関数の定義と呼び出し部を自由に操れ、複数の関数をブラウズできるような開発環境であれば、このような面倒もなくなります。しかし最もおすすめなのは関数名をわかりやすくすることです。これができていれば、内部の実装を見なくとも先に読み進めていけます。

関数を短くするために行う99%は「関数の抽出」です。関数内で一つにまとめられそうな箇所があったら迷わず抽出しましょう。

パラメータや一時変数が多すぎる関数は抽出を妨げる要因となり「関数の抽出」を行ってもパラメータや一時変数を次々と受け渡していかなければならず、あまり読みやすいコードになりません。「問い合わせによる一時変数の置き換え」を組み合わせて一時変数を減らしていく必要があります。

p.75~p.76 長い関数　一部抜粋

→著者はコメントが必要な関数の命名をするときには内部でどのような処理をしているかではなく、そのコードがなにをしているかの「意図」を示した名前をつけるようにすると言っています。

問い合わせによる一時変数の置き換え(サンプルコード)

ユーザーの健康スコアを計算する関数です。

before

function calculateHealthScore(user) {
  const bmi = user.weight / (user.height / 100) ** 2;
  const sleepScore = user.sleepHours >= 7 ? 10 : 5;
  const activityScore = user.steps >= 10000 ? 10 : 5;
  const agePenalty = user.age > 60 ? -5 : 0;

  const totalScore = bmiScore(bmi) + sleepScore + activityScore + agePenalty;

  return totalScore;
}

function bmiScore(bmi) {
  if (bmi < 18.5) return 5;
  if (bmi < 25) return 10;
  if (bmi < 30) return 7;
  return 4;
}

一時変数 bmi, sleepScore, activityScore, agePenalty が乱立して抽出して関数化しようとすると、必要な値をパラメータで渡し続ける必要がありました（可読性が低い）

after

function calculateHealthScore(user) {
  return bmiScore(user) + sleepScore(user) + activityScore(user) + agePenalty(user);
}

function bmiScore(user) {
  const bmi = user.weight / (user.height / 100) ** 2;

  if (bmi < 18.5) return 5;
  if (bmi < 25) return 10;
  if (bmi < 30) return 7;
  return 4;
}

function sleepScore(user) {
  return user.sleepHours >= 7 ? 10 : 5;
}

function activityScore(user) {
  return user.steps >= 10000 ? 10 : 5;
}

function agePenalty(user) {
  return user.age > 60 ? -5 : 0;
}

これにより

• 一時変数を完全に排除
• それぞれ独立した「問い合わせ関数」を作成
• 抽出後はパラメータuser だけで済む
• 責務が分割され見通しが圧倒的に良くなる

…などのメリットがあります。

条件分岐やループも抽出の対象になります。「条件記述の分解」を条件文に対して行うことができます。

条件記述の分解(サンプルコード)

before

function getShippingFee(order) {
  if (
    order.totalPrice > 10000 || // 高額注文
    (order.member && order.member.rank === "gold") ||  // ゴールド会員
    (order.coupon && order.coupon.type === "free_shipping") // 送料無料クーポン
  ) {
    return 0;
  }

  return 500;
}

条件が長く、何が「送料無料の条件」なのか一目で分からなく、条件が増えるとさらに可読性が落ちていきます。

after

function getShippingFee(order) {
  if (isFreeShipping(order)) {
    return 0;
  }
  return 500;
}

function isFreeShipping(order) {
  return (
    isHighValueOrder(order) ||
    isGoldMember(order) ||
    hasFreeShippingCoupon(order)
  );
}

function isHighValueOrder(order) {
  return order.totalPrice > 10000;
}

function isGoldMember(order) {
  return order.member && order.member.rank === "gold";
}

function hasFreeShippingCoupon(order) {
  return order.coupon && order.coupon.type === "free_shipping";
}

これにより「高額注文」「ゴールド会員」「クーポン」という概念ごとの関数に分割されていて読みやすく、条件変更にも強いコードになります。

同じ条件で分岐しているswitch文が複数あるときには「ポリモーフィズムによる条件記述の置き換え」を適用すべきです。

ポリモーフィズムとは同じメソッド呼び出しに対して、オブジェクトの種類ごとに異なる振る舞いを実現する仕組みです。分岐をなくし、拡張に強いコードを書くための要となる考え方です

「ポリモーフィズムによる条件記述の置き換え」サンプルコード

before

function getDiscountRate(member) {
  switch (member.rank) {
    case "silver":
      return 5;
    case "gold":
      return 10;
    case "platinum":
      return 15;
    default:
      return 0;
  }
}

function getShippingFee(member) {
  switch (member.rank) {
    case "silver":
      return 400;
    case "gold":
      return 200;
    case "platinum":
      return 0;
    default:
      return 500;
  }
}

function getPriority(member) {
  switch (member.rank) {
    case "silver":
      return 2;
    case "gold":
      return 1;
    case "platinum":
      return 0;
    default:
      return 3;
  }
}

「ユーザーの会員ランク」を使って複数の関数がそれぞれswitch 文で分岐しています。

ランクが増えるほどすべての switchを修正しなければならなく、コードのメンテナンス性が悪いです。

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

after

①会員ランクごとにクラスを作成

class Member {
  constructor(rank) {
    this.rank = rank;
  }

  getDiscountRate() {
    return 0;
  }

  getShippingFee() {
    return 500;
  }

  getPriority() {
    return 3;
  }
}

class SilverMember extends Member {
  getDiscountRate() {
    return 5;
  }
  getShippingFee() {
    return 400;
  }
  getPriority() {
    return 2;
  }
}

class GoldMember extends Member {
  getDiscountRate() {
    return 10;
  }
  getShippingFee() {
    return 200;
  }
  getPriority() {
    return 1;
  }
}

class PlatinumMember extends Member {
  getDiscountRate() {
    return 15;
  }
  getShippingFee() {
    return 0;
  }
  getPriority() {
    return 0;
  }
}

②ファクトリー関数で適切なクラスを生成する

ファクトリー関数とは、オブジェクト（インスタンス）を作って返すための関数

function createMember(rank) {
  switch (rank) {
    case "silver":
      return new SilverMember(rank);
    case "gold":
      return new GoldMember(rank);
    case "platinum":
      return new PlatinumMember(rank);
    default:
      return new Member(rank);
  }
}

③使う側は「switchを持たない」コードになる

const member = createMember("gold");

console.log(member.getDiscountRate()); // 10
console.log(member.getShippingFee());  // 200
console.log(member.getPriority());     // 1

これでswitch文は②のファクトリー関数での一回だけになり、分岐の重複がゼロになります

rankが増えてもswitchを増やすことなく追加することができます。

たとえば、diamondランクが増えても下記の追加だけ済みます

DiamondMemberクラスの追加

class DiamondMember extends Member {
  getDiscountRate() { return 20; }
  getShippingFee() { return 0; }
  getPriority() { return 0; }
}

ファクトリー関数に追加

case "diamond":
return new DiamondMember(rank);

また、ループ文もループ文とループ内部部分のコードを抽出して独立した関数にもできます。その抽出したループ文に名前をつけるのが難しければ、ループ以外のことをしている可能性が高いです

ループ

「パイプラインによるループの置き換え」でfilterやmap といったパイプライン操作を使うとループ文を使うより処理対象郡と処理内容をすばやく確認することができます。パイプライン構造で表現すると、ロジックを追うのがかなり簡単になります。

p.82 ループ 一部抜粋

ちなみにパイプラインとは、データが一方向に流れていき、ステップごとに加工される構造のことです。

業務でつかう既存コードにはループ文で書かれている箇所が大量にあるので自分が業務のコードを書くときに使うか微妙なところですが、このような技法があることを知れて良かったです

パイプラインによるループの置き換え(サンプルコード)

before

const result = [];
for (const x of items) {
  if (x.active) {
    result.push(x.value * 2);
  }
}
result.sort();

これではぱっと見、どんな処理かは見抜けない

after

const result = items
  .filter(x => x.active)
  .map(x => x.value * 2)
  .sort();

これだと絞り込み、変換、並び替えをやっていることが縦並びで一瞬でわかる

コメント

コメントを詳しく書くことは悪くないですが、コメントの必要性を感じたときはリファクタを行ってコメントを書かなくても内容がわかるようなコードを目指すといい。

コメントは、不明点を書き留めたりよくわからない事項のメモ、なぜこのような処理を選択したかなどの背景を残しておくこともできます。これらは忘れやすい事項のため以降の修正に役立ちます

p.86 コメント 一部抜粋

→複雑な処理を書いたメソッドやクラスにはコメントをして処理を日本語ですぐわかりやすくしていましたが、本書を読んでコメントをたくさん書きたくなったときこそリファクタリングのタイミングだと学びました。TODOコメントなんかも紛らわしいので書くくらいなら時間をつくって潰したほうが良さそうですね。

この章で書いたリファクタリングの手法の詳しくはこの本の第5章から詳しく書いてあります。

まとめ

コードを書くときはAIに書かせることが最近では多いと思いますが、人間が見やすいコードにリファクタリングをするのはまだAIには難しいんじゃないかなって個人的には思います。これからこのAI時代においてこの本にかかれていることを参考にリファクタリングに重きを置いて、可読性・拡張性の高いコードを書いて開発速度を高めたいと思います。また、この本の第5章からつづくリファクタリングの手法についても機会があればまたブログでまとめようと思います。

最後まで読んで頂きありがとうございます。 弊社ではエンジニアを募集しております。 ご興味がありましたらカジュアル面談も可能ですので、下記リンクより是非ご応募ください！

iimon採用サイト / Wantedly

明日は正体不明の人物 「hoge」さんです。 謎のベールに包まれた彼は一体どんな記事を書いてくれるのでしょう！！ 楽しみですね！！

本のリンク

https://www.ohmsha.co.jp/book/9784274224546/