LoginSignup
0
0

More than 1 year has passed since last update.

@Jazuma

リファクタリング入門

Posted at

リファクタリング 既存のコードを安全に改善する を読んだので備忘録としてまとめます。

目的

基本的なリファクタリングの基礎知識をまとめて理解しやすく、変更しやすいコードを書けるようになる。

手軽にできるリファクタリング一覧

本書で紹介されているリファクタリング手法の中には使えるタイミングが限られているものもありました。今回は入門ということで、汎用性の高い手法を抜粋します。

  1. 関数の抽出
  2. 条件記述の単純化
  3. 問い合わせと更新の分離

関数の抽出

コードの中の一まとめの処理を独立した関数として切り出すという手法で最もよく使われるリファクタリングです。

関数抽出は次のような目的で実施されることが多いとのことです。

  1. 再利用性を高める
  2. 1つの関数の行数を減らす
  3. 意図と実装を分離する

1と2については本書を読む以前から理解していました。関数化によって同じ処理を抽出できれば同じ処理を何度も書かずに済みます。仕様変更にも容易に対応できるでしょう。

関数1つあたりの行数が減れば(物理的にも論理的にも)読みやすくなります。

しかし、本書は3.を最も重視しています。意図と実装の分離とは「どうやっているか(実装)を知らなくとも何をしているか(意図)を理解でき、目的を達成できるようにすること」と言われています。

また、ある処理が繰り返し出てこない場合や行数が短い場合でも関数として切り出して良い（あるいは切り出すべき）と書かれています。

つまり、長さや使われる回数よりも「意図」が読み手に伝わることを重視すべきということでしょう。

関数抽出の手順

① 新しい関数を作り、その意図に沿って命名する(どうやっているかではなく、何をするかによって命名する)
② 抽出したいコードを元の関数から新しい関数にコピーする
③ 元の関数ではスコープ内だったが抽出先ではスコープ外になる変数を引数として渡す。(抽出先の関数で代入される変数が多い場合、一旦抽出を諦めて変数を整理する)
④ コンパイルする。
⑤ 抽出前のコードを抽出された関数の呼び出しに置き換える。
⑥ テストして振る舞いが変わっていないことを確認する。

サンプルコード

例1. 抽出先でスコープ外になる変数が無い場合

これは最も単純なケースで、単にカット＆ペーストすればokです。

Before1.js

function printOwing(invoice) {
  let outstanding = 0;

//出力処理を関数に抽出
  console.log("******************");
  console.log("***Customer Owes***");
  console.log("******************");

  /// 未払い金の計算
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  // 締め日の記録
  const today = Clock.today;
  invoice.dutDate = new  Date(today.getFullYear() , today.getMonth() , today.getDate() + 30);

  // 明細の印字
  console.log(` name: ${invoice.customer}`);
  console.log(` amount: ${outstanding}`);
  console.log(` due: ${invoice.dutDate.toLocalDateString()}`);
}
After1.js

function printOwing(invoice) {
  let outstanding = 0;

  // 抽出した関数を呼び出し
  printBanner();

  /// 未払い金の計算
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  // 締め日の記録
  const today = Clock.today;
  invoice.dutDate = new  Date(today.getFullYear() , today.getMonth() , today.getDate() + 30);

  // 明細の印字
  console.log(` name: ${invoice.customer}`);
  console.log(` amount: ${outstanding}`);
  console.log(` due: ${invoice.dutDate.toLocalDateString()}`);
}

//出力処理を関数に抽出
function printBanner() {
  console.log("******************");
  console.log("***Customer Owes***");
  console.log("******************");
}
例2. 抽出先でローカル変数を参照する場合

ローカル変数に再代入せず、参照だけする場合は、抽出先の関数に引数として渡します。

Before2.js

function printOwing(invoice) {
  let outstanding = 0;

  // 抽出した関数を呼び出し
  printBanner();

  /// 未払い金の計算
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  // 締め日の記録
  const today = Clock.today;
  invoice.dutDate = new  Date(today.getFullYear() , today.getMonth() , today.getDate() + 30);

  // 明細の印字
  console.log(` name: ${invoice.customer}`);
  console.log(` amount: ${outstanding}`);
  console.log(` due: ${invoice.dutDate.toLocalDateString()}`);
}

//出力処理を関数に抽出
function printBanner() {
  console.log("******************");
  console.log("***Customer Owes***");
  console.log("******************");
}
After2.js
function printOwing(invoice) {
  let outstanding = 0;

  // 抽出した関数を呼び出し
  printBanner();

  /// 未払い金の計算
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  // 締め日の記録
  recordDueDate(invoice);

  // 明細の印字
  printDetails(invoice, outstanding);
}


function printBanner() {
  console.log("******************");
  console.log("***Customer Owes***");
  console.log("******************");
}

//明細印字処理を抽出
function printDetails(invoice, outstanding) {
  console.log(` name: ${invoice.customer}`);
  console.log(` amount: ${outstanding}`);
  console.log(` due: ${invoice.dutDate.toLocalDateString()}`); 
}

// 締め日の記録を抽出
function recordDueDate(invoice) {
  const today = Clock.today;
  invoice.dutDate = new  Date(today.getFullYear() , today.getMonth() , today.getDate() + 30);
}

ローカル変数が構造体(配列やオブジェクトなど)でもパラメータとして抽出した関数に渡すことができます。(引数にオブジェクトを渡すスタンプ結合は避けた方が良い気もしますが...スタンプ結合はベストではないがアンチパターンというほどでもないということでしょうか...)

例3. ローカル変数に再代入する場合

抽出先以外でもローカル変数を使う場合、その変数を戻り値として返す関数を新しく作成します。

Before3.js
function printOwing(invoice) {
  let outstanding = 0;

  printBanner();

  /// 未払い金の計算
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  recordDueDate(invoice);

  printDetails(invoice, outstanding);
}

まずは変数宣言を参照元の直前にスライドさせます。

function printOwing(invoice) {


  printBanner();

  /// 変数宣言をスライド
  let outstanding = 0;
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  recordDueDate(invoice);

  printDetails(invoice, outstanding);
}

続いて、抽出したいコードをコピペして関数化します。

function printOwing(invoice) {

  printBanner();

  /// 変数宣言をスライド
  let outstanding = 0;
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }

  recordDueDate(invoice);
  printDetails(invoice, outstanding);
}

// outstandingを戻り値として返す関数
function calculateOutstanding(invoice) {
  let outstanding = 0;
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }
  return outstanding;
}

outstandingは抽出先で再代入される唯一の変数ですので戻り値として返すことができます。また、outstandingの宣言をcalculateOutstanding関数内部に移しているので、外部からパラメータとして渡す必要がありません。

最後に、元のコードを抽出した関数の呼び出しに置き換えます。
outstanding変数はprintOwing関数内部で再代入されないのでconstにしておきます。

function printOwing(invoice) {

  printBanner();
  // calculateOutstanding関数の戻り値をoutstanding変数に保持する
  const outstanding = calculateOutstanding(invoice);
  recordDueDate(invoice);
  printDetails(invoice, outstanding);
}

function calculateOutstanding(invoice) {
  let outstanding = 0;
  for (const o of invoice.orders) {
    outstanding += o.amount;
  }
  return outstanding;
}

リファクタリングによって20行ほどあったprintOwing関数が6行になり、可読性が上がりました。
また、outstanding変数を計算する責任を別関数に移したため、修正が容易になっています。(outstanding変数の計算式を変える場合、calculateOutstanding関数を変更すれば良いということがすぐに判断できることでしょう。)

条件分岐の単純化

条件記述の分解

条件分岐はプログラムが理解しづらくなる要因になりやすいです。その理由としては

  • 条件が長くなると物理的に読みにくくなる。
  • 「なぜその分岐が必要なのか」(条件分岐が設定されている意図)が明確でない

といったところがあるでしょう。if文を条件部分を関数に抽出することで意図を明確にすることができます。

Before.js

if (!Date.isBefore(plan.summerStart) && !Date.isAfter(plan.summerEnd))
  charge = quantity * plan.summerRate;
else
  charge = quantity * plan.regularRate;

このロジックは条件分岐が横に長いために読みづらく、また「要はどんな処理なのか」ということが一読しただけではわかりません。

次のようにif文の条件を関数にして切り出してやれば「夏季料金とそれ以外の季節で料金が異なる」ということがより明確になります。(関数の役割を小さくしたことでテストもしやすくなりました。)

After.js
if (duringSummer())
  charge = quantity * plan.summerRate;
else
  charge = quantity * plan.regularRate;

function duringSummer() {
return !Date.isBefore(plan.summerStart) && !Date.isAfter(plan.summerEnd);
}

このリファクタリングは曜日や人の年齢など「その値がどういう意味なのか」ということが明確でない場合により有効になるでしょう。(プロパティの状態を数字で管理している場合にも積極的に使いたいです。)

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

条件記述には1. then節とelse節の両方が正常動作の場合(例：ECサイトでユーザの性別によって処理を分ける場合)と、2. どちらかの節が正常動作で一方が例外的な動作の場合(例： 労務管理システムで休職者のみ給与計算処理を変える場合)の2通りの状況があります。

2.の場合、例外的な動作が成立する時にすぐにリターンします。(この類の判定ロジックをガード節と呼ぶことがあります。)

function getPayAmount(employee){
 // 例外動作(休職者に対する処理)
 if (employee.isSepareted){
   return separatedAmount();
 }
 return normalPayAmount();
}
  1. はthen節とelse節が等しく起こり得て、等しく重要であるという意図を持ちます。その一方、2. は「主要な処理ではないので発生した場合は何かしらして脱出します」という意図を持ちます。

問い合わせと更新の分離

単純に値を返す処理と副作用(実行のたびにサーバの状態を変える処理。 例：ファイル作成)を分離する(別々の関数に分ける)のはいい判断です。

Before.js
//未払い賃金の計算とメール送信処理を行う関数
function getTotalOutStandingAndSendBill() {
  const result = customer.invoice.reduce((total, each) => each.amount + total, 0);
  sendBill();
  return result;
}
After.js
//未払い賃金を計算する関数
function getTotalOutStandingAndSendBill() {
  const result = customer.invoice.reduce((total, each) => each.amount + total, 0);
  return result;
}
//メール送信を行う関数
function sendBill() {
  emailGateway.send(formatBill(customer));
}

問い合わせと更新を分離すると以下のようなメリットがあります。

  • 問い合わせ用の関数を呼び出しやすくなる
  • テストしやすくなる
  • 上記により、変更しやすくなる

まとめ

1つの関数に役割が集中しすぎていたり、条件記述が長すぎたりすると可読性が大きく下がるので「関数の抽出」と「条件記述の単純化」は特に使える場面が多そうです。

プログラムを物理的に読みやすい大きさに分割すると論理的な読みやすさも向上する場合が多いというのは新しい発見でした。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
0
0