読書メモ：リファクタリング（第２版）

読書メモ：リファクタリング（第２版）

July 29, 2023

年単位で長いこと積読してしまっていた上記を読了。

著者は著名なソフトウェア技術者であるマーティン・ファウラー。一部の章については、こちらも同様に著名なケント・ベックと共著になっている。昨今のアジャイル開発の先駆け（＊）となったエクストリーム・プログラミング（XP）を提唱したのがケント・ベックで、そのなかでリファクタリングの重要性も語られていた記憶（XP の書籍は読んだことがあるのだが、手元に残っていないため確認できず）。本書で語られていたが、マーティン・ファウラーはケント・ベックにリファクタリングを教わったらしい。マーティン・ファウラーはリファクタリングの可能性と重要性を認識し、それを広めるべきと考えたが、当時のケント・ベックは XP 本の執筆に忙しかったため、マーティン・ファウラーがリファクタリングの本を書くことにしたとのことだった。第１版は 2000 年に発売されていて、改訂第２版である本書 2019 年に発売された。第１版では Java を用いて説明していたものが、第２版では JavaScript へと変わっている。

＊ここの前後関係は詳しくないが、XP の本が出版されたのが 1999 年で、アジャイルソフトウェア開発宣言が採択されたのが 2001 年であり、現在のように「アジャイル」という用語が認知される前には XP はすでに存在していたはず。

書籍内で説明されている内容は「長いコードをベタ書きするのではなく、意味ある単位で関数に切り出してそれらを呼び出す形にした方が良い」であったり「関数の引数が膨大になったときは、意味ある単位でオブジェクトにまとめて渡したほうが良い」といった具合である。もう少し発展的な内容もあるが、読者がある程度の開発経験を有する場合、半ば当たり前と思える内容だろう。そういう意味では、経験の浅いときにこの本を購入した私は賢く、そしてそのとき読まずに積読した私は愚かだった。

関連項目

内容の一部を以下にメモしておく。

リファクタリングの原則 #

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

リファクタリングがプログラムの速度性能に与える影響は、一般に懸念されるところです。ソフトウェアを理解しやすくするための変更は、しばしばプログラムの実行速度を落としてしまいます。このことについては、深刻に受け止める必要があります。リファクタリングは、ソフトウェアの実行速度を遅くする一方で、よりパフォーマンスチューニングをしやすい形にします。早いソフトウェアを作る秘訣は、最初にチューニングしやすく作って、十分な速度がでるまで、段階的にチューニングすることです。

パフォーマンスで興味深いのは、プログラムを解析してみると、ほとんどの時間がごく一部の処理で集中的に消費されているという事実です。そのため、コード全体にわたって均等に最適化を行ったとしても、その活動の 90% は無駄になるのです。まず、該当プログラムが、実際に時間とメモリをどこで消費しているかを計測しましょう。パフォーマンス上のホットスポットを見つけたらそこの部分をチューニングしましょう。

データベース #

通常のリファクタリングとの違いは、データベースの変更は、正式に至るまでに複数のリリースに分割するのがベストということです。こうすることで、本番運用で問題が起きても変更を取り消すことが容易になります。フィールド名を変えるときは、最初のコミットは、新しいフィールドをデータベースに追加するものの、まったく使用しないということになるでしょう。次に更新処理のコードを変更しますが、新旧両方のフィールドに同時に書き込むようにします。それから読み出しコードを、徐々に新しいフィールドを使うように書き換えていきます。こうしてすべての箇所が新しいフィールドを使うようになって、バグがしばらく出てこないようであれば、今や使われなくなった古いフィールドを削除します。

リファクタリングのカタログ #

関数のインライン化 #

あまりに細かく関数化しすぎた結果、かえって分かりづらく道に迷ってしまうような場合があります。このような関数群は、一度、それらをすべて元の１つの大きな関数にインライン化しましょう。必要ならば、より望ましい関数群としてそこから再抽出すればよいのです。

仲介人の除去 #

「委譲の隠蔽」では、委譲先のオブジェクトをカプセル化することの有効性について述べました。これはただではありません。対価として、クライアントが委譲先のオブジェクトの新たな特性を使おうとするたびに、単純な委譲メソッドをサーバオブジェクトに追加しなければなりません。特性を追加していると、やがてあまりにも多くの転送にいらいらしてきます。そうなると、もはやサーバオブジェクトはただの仲介人に過ぎません。そろそろ委譲先のオブジェクトを直接呼ばせてはどうでしょう。

この不吉な臭いは、開発メンバが「デメテルの法則（＊）」にあまりにものめり込んでいると漂ってきます。これは、法則というよりも、「ときには役立つデメテルの提案」と呼んだほうが良いと強く思います。

システムが変化すれば、どの程度隠蔽すべきかの基準も変化します。以前は妥当だったカプセル化も、半年後には扱いにくいものになります。謝る必要はありません。ひたすら直すのみです。それがリファクタリングというものです。

＊あるオブジェクトが呼び出せるメソッドを、直接参照しているオブジェクトや渡されたオブジェクトのものだけに限定する設計原則。

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

// BEFORE
let result;
if (availableResources.length === 0) {
  result = createResource();
  allocatedResource.push(result);
} else {
  result = availableResources.pop();
  allocatedResource.push(result);
}
return result;

// AFTER
let result;
if (availableResources.length === 0) {
  result = createResource();
} else {
  result = availableResources.pop();
}
allocatedResource.push(result);
return result;

ループの分離 #

１回のループで処理したいというだけの理由で、２つの異なる処理を同時に行なっているループをよく見かけます。しかし、同じループの中で２つの異なる処理をしていると、ループを修正する際には、必ず両方の処理内容を理解しなければならなくなります。ループを分離することで、変更すべき処理だけを理解すればよくなります。

// BEFORE
let averageAge = 0;
let totalSalary = 0;
for (const p of people) {
  averageAge += p.age;
  totalSalary += p.salary;
}
averageAge = averageAge / people.length;
totalSalary = totalSalary / people.length;

// AFTER
let averageAge = 0;
for (const p of people) {
  averageAge += p.age;
}
averageAge = averageAge / people.length;

let totalSalary = 0;
for (const p of people) {
  totalSalary += p.salary;
}
totalSalary = totalSalary / people.length;

結果としてループを２回処理することになるため、多くのプログラマはこのリファクタリングを不快に思うでしょう。私が常に心がけていることは、リファクタリングと最適化の分離です。最適化はコードをきれいにした後で行えばよく、ループの走査が性能のボトルネックになっているなら、簡単に元に戻せます。実際には大きなリストに対するループであっても、ボトルネックになることはめったにありません。ループを分離することで、より強力な他の最適化が可能になることもよくあります。

デッドコードの削除 #

コードが使用されなくなったら削除すべきです。そのコードが将来必要になるかもしれないなどという心配はしません。必要になったらいつでも、バージョン管理システムから再び掘り起こせるからです。本当に将来必要になるかもしれないと思うならば、削除するコードと、削除したリビジョンに関してコードにコメントを残すかもしれません。しかし正直なところ、それを最後にやったのがいつなのか思い出せませんし、そうしなかったことを後悔した記憶もありません。

デッドコードのコメントアウトは、かつては一般的な習慣でした。それは、バージョン管理システムが広く使用される以前の時代や、使いづらかった時代には有用でした。現在では、とても小さなコードベースでもバージョン管理システムにおけるため、もはや必要のない習慣です。

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

記述条件には二つのスタイルがあると考えています。一つ目は、then 節と else 節の両方が正常動作の場合で、二つ目は、どちらか一方の節が正常動作で他方が例外的な動作の場合です。この２種類の条件記述は意図が異なるため、その違いをコードにきちんと表現すべきです。もし、両方の節が正常動作ならば、then 節と else 節を明示して条件を記述します。例外的な動作に対しては、その条件を判定し、成立する場合にはリターンします。この種の判定をよくガード節と呼びます。私が if-then-else 構文を使うときは、then 節にも else 節にも同じウェイトを置きます。これにより読み手に対して、両方が等しく起こり得ること、および等しく重要であることを伝えます。逆にガード節は「主要な処理ではないため、起きたときには何がしかのことをやって脱出すること」を伝えます。

メソッドの入口と出口を一つずつするように教わったプログラマと仕事をするときは、このリファクタリングを行うことがよくあります。入口一つについては、現在の言語ではそうするしかありませんが、出口が一つだけというルールは、まったく有益ではありません。大原則は明快さです。出口を一つにすることでメソッドが明快になるなら出口は一つでかまいませんが、そうでなければ出口の数にこだわるべきではありません。

// BEFORE
function getPayAmount() {
  let result;
  if (isDead) {
    result = deadAmount();
  } else {
    if (isSeparated) {
      result = separatedAmount();
    } else {
      if (isRetired) {
        result = retiredAmount();
      } else {
        result = normalPayAmount();
      }
    }
  }
  return result;
}

// AFTER
function getPayAmount() {
  if (isDead) return deadAmount();
  if (isSeparated) return separatedAmount();
  if (isRetired) return retiredAmount();
  return normalPayAmount();
}

フラグパラメータの削除 #

フラグ引数は、呼び出し元が呼び出し先に対してどのロジックを実行してほしいかを指示するためのパラメータとして使われます。フラグ引数は好ましくありません。どの関数呼び出しが使えるか、それをどう呼び出せば良いかを理解するプロセスが煩雑になるからです。

// BEFORE
function setDimension(name, value) {
  if (name === 'height') {
    this._height = value;
    return;
  }
  if (name === 'width') {
    this._width = value;
    return;
  }
}

// AFTER
function setHeight(value) {
  this._height = height;
}

function setWidth(value) {
  this._width = width;
}