リファクタリングの奨め

はじめに

こんにちは〜！フィヨルドブートキャンプ現役生のwataと申します。

これは、「フィヨルドブートキャンプ Advent Calendar 2021」の18日目の記事です。

フィヨルドブートキャンプ Part 1 Advent Calendar 2021 - Adventar

フィヨルドブートキャンプ Part 2 Advent Calendar 2021 - Adventar

軽い自己紹介ですが、私は現在、新卒で入社したSESの企業でプログラマーとして働いており、入社3年目になります。

フィヨルドブートキャンプに入会して約半年、全くブログを公開してませんでした(汗)。Advent Calendarをきっかけに重い腰を上げてエントリーしてみました！

初投稿なので、温かい目で見守ってください笑🙇‍♂️

本記事では、リファクタリングを知らない人、リファクタリングに抵抗を感じる人を対象に、自分が日頃実践しているマーティン・ファウラー著のリファクタリング本の手法を紹介しつつ、リファクタリングの魅力を伝えられればと思います！

※以降の章では、「リファクタリング本」のことを「リファクタ本」と省略することにします。

※本記事で説明するコードは、フィヨルドブートキャンプのプラクティスであるFizzBuzz問題、カレンダープログラムを取り扱います。

リファクタリングとは

まずはリファクタリングの定義の説明をします。リファクタリング本で次のように述べています。

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

ＭａｒｔｉｎＦｏｗｌｅｒ. リファクタリング既存のコードを安全に改善する（第2版） (p.45). Kindle 版.

外部の振る舞いとは、アプリケーションの動きのことです。内部機能とは、外部の振る舞いを満たすためのロジック、処理の詳細になります。

リファクタリングの肝は改善することにあります。例としてFizzBuzz問題を取り扱います。最初に、下記のような内部機能となるコードを書いて、FizzBuzz問題の振る舞いを実現したとします。

def FizzBuzzProblem(number)
  if (number % 15).zero?
     puts 'FizzBuzz'
  elseif (number % 3).zero?
     puts 'Fizz'
  elseif (number % 5).zero?
     puts 'Buzz'
  else
     puts number
  end
end

次に、リファクタリングによって改善したコードは、下記のようになります。

def FizzBuzzProblem(number)
  if FizzBuzz?(number)
     puts 'FizzBuzz'
  elseif Fizz?(number)
     puts 'Fizz'
  elseif Buzz?(number)
     puts 'Buzz'
  else
     puts number
  end
end

def FizzBuzz?(number)
  Fizz?(number) && Buzz?(number)
end

def Fizz?(number)
  (number % 3).zero?
end

def Buzz?(number)
  (number % 5).zero?
end

改善前後を比較してみます。改善後のコードは、メソッドを増やしたため全体のコード量がふえました。しかし、条件のロジックをメソッドで切り分けることで、条件に名前を付けることができました。名前を付けることで、処理の詳細を見なくても概要を把握が容易になります。

また、FizzBuzz問題の要件が変更されても、すぐに該当箇所を把握できる、変更内容が他の処理に影響しない構造になっています。

しかし、改善後のコードに振る舞いの変化はありません。

理解しづらい、変更しづらいコードは色々と辛い

FizzBuzz問題では、リファクタリングの旨味を薄く感じるかも知れないです。少し長めのコードを例にしたいと思います。リファクタリングすべきコードは以下のパターンがあります。

メソッドの中身が膨大
変数名、関数名とコードの表現が不一致

メソッドの中身が膨大である特徴の1つとして、「1つのメソッドの中に、別のメソッドを呼び出さず、そのまま処理を書く」ことがあります。

カレンダープラクティスのコードを例にします。2021年の12月のカレンダーを表示したプログラムで、外部の振る舞いは下記の3つです。

コンソールの1行目に年と月を表示する
コンソールの2行目に曜日を表示する
コンソールの3行目以降に各週ごとの日を表示する

def cal
  year = 2021
  month = 12
  # 12月の週を配列の要素とする。
  # さらに各週ごとの日を要素とする。曜日に紐づく日がない場合はnil。
  days = [
    [nil, nil, nil, 1, 2, 3, 4],
    [5,  6, 7, 8, 9, 10, 11],
    [12, 13, 14, 15, 16, 17, 18],
    [19, 20, 21, 22, 23, 24, 25],
    [26, 27, 28, 29, 30, 31, nil]
  ]

  # 外部の振る舞い1。カレンダーを描画
  puts "#{month}月 #{year}".center(20)
  # 外部の振る舞い2。曜日を描画
  displayed_day_of_week = %w[日 月 火 水 木 金 土]
  sep = ' '
  puts displayed_day_of_week.join(sep)

  # 外部の振る舞い3。各週の日を描画
  days.each do |week|
    displayed_week = week.map do |day|
      # 曜日列に日付がない場合は空白表示
      next '  ' if day.nil?

      # 日付の文字数が1文字の場合は空白を追加して幅を調整
      padding_width = day < 10 ? 2 : 0
      day.to_s.rjust(padding_width)
    end
    sep = ' '
    puts displayed_week.join(sep)
  end
end

cal

各外部の振る舞いの開始位置に、振る舞いのコメントを書いてます。コメントがあるため、上から読めば、各外部の振る舞いが理解可能です。しかし、コードの理解、機能の追加、不具合の修正を行う度に、以下のことを意識する必要があります。

各処理の範囲がどこまでか。
- 明確なくくりがないので、視覚的にわかりづらい。
各処理の振る舞いが何を表すのか。
- 概要を表現したコメントがない場合、コードの詳細を追わなければならない。
- 経験的に、コメントはメンテされにくいので、概要を記載したコメントと処理の意味に乖離が起きやすい。
各処理で使用した変数が、他の処理で変更されてないか。
- 変数のスコープが1つの処理の前後まで及ぼすので、影響している可能性がある。

これらの労力は、1つのメソッドの中にあるコード量に比例します。カレンダーのコードは、先程のFizzBuzz問題のコードと比較すると、注意する労力をより一層要します。

また、2つ目のパターンの変数名、メソッド名の名前が不一致だと、せっかく付けた名前も無駄になります。結局、処理の詳細を見ないと、誤解なく正確に理解することが難しいからです。

私が今まで職場で見たコード(私自身含めて)も、このパターンは多かったです。バグの調査、コードの修正や、コードで仕様把握等の作業過程で、コードを理解するのに時間がかかり、苦労した思い出があります😭

リファクタリングの紹介

カレンダープログラムのサンプルコードを例にして、リファクタ本にある2つの手法を紹介していきたいと思います。

テストコードを書く

リファクタリングをする準備として、テストコードを書きます。テストコードは、メソッドの振る舞いが期待通りに動くか確認するための用途になります。リファクタリングをする上で、テストコードを書くメリットは以下が挙げられます。

修正中にバグを素早く検知することができる
少しずつリファクタリングを進めることができる

テストコードがない場合、プログラムが正常に動作する検証方法は、プログラムの実行が必要です。プログラムの実行は全体を通した確認ができても、部分的に正しいかを詳細に見ることはできません。

私が経験した現場では、テストコードがなかったので、修正してもデグレが多発しました。そのため、今までテストした項目も最初からテストすることがありました...

片やテストコードは、修正したコードが原因で不具合が発生すれば、ピンポイントでかつ素早く通知します。安心してプログラムの修正に取り掛かることが可能です。

先程のカレンダープログラムを構成する外部の振る舞い3を対象に、テストコードを書いてみます。

テストコードの対象を見つける手順は、メソッドの抽出で説明します。

各週ごとの日付を表示する処理がありますが、この処理の入力と出力を考えます。

入力：1週間の日を要素とした1次元配列。
- その週の日がない場合、要素はnil。
出力：1週間の日を空白区切りの文字列で出力。
- 日付表示の最大幅は2。
- 日付が1文字の場合は右詰め。
- 要素がnilの場合は空白表示。

上記の入力と出力を元に、以下のようなテストコードを書きます。 1例として入力は、最初の週にします。

テストコード

require 'minitest/autorun'
require_relative './cal'

class CalTest < Minitest::Test
  def test_get_week
    # 入力
    input = [nil, nil, nil, 1, 2, 3, 4]
    # 期待する出力
    expected = '          1  2  3  4'
    
    # 検証。
    assert expected, get_week(input)
  end
end

テストコードにMinitestを使用しています。テストコードのディレクトリは、cal.rbと同じです。

リファクタリングの最低限の準備はこれだけです。

次はリファクタリングに入ります。

メソッドの抽出

長い処理の中には、細分化できる塊が多く存在します。この塊には、それぞれの役割があり独立できる可能性があります。意味のある塊をコメントで表現するのではなく、メソッドとして抽出することは以下のメリットがあります。

塊の範囲が視覚的にわかりやすい。
リファクタリングが容易にできる。
- 関数名の変更、内部機能の変更など。
処理を閉じることで、他の関数からの影響を受けにくい。

メソッドの抽出は、大まかに以下の方法で実現できます。リファクタ本では、グローバルなパラメータを参照している場合など、複雑な状況に対応した手順が紹介されています。

意味のある塊（処理）を探す。
抽出用のメソッドを用意する。
手順1で探した処理をコピーし、抽出用のメソッドにペーストする。
コピー元の処理を削除して抽出メソッドを呼び出す。

上記の手順を見るとそこまで複雑なことはしてないです。

外部の振る舞い3を対象にメソッドの抽出を行います。
まずは手順1の「意味のある塊」を探します。

この振る舞いを実現するために、内部機能では、1週間ごとの日を表示する繰り返し処理を行っています。1週間ごとの日を表示する処理は意味のある塊なので、抽出対象となります。

該当箇所は下記になります。

    # 日付をカレンダーのフォーマットに変更
    displayed_week = week.map do |day|
      # 曜日列に日付がない場合は空白表示
      next '  ' if day.nil?

      # 日付の文字数が1文字の場合は空白を追加して幅を調整
      width = 2
      day.to_s.rjust(width)
    end
    sep = ' '
    puts displayed_week.join(sep)

putsは、引数を標準出力に出力するメソッドなので、これを塊の範囲内にすると、返り値がないメソッドになってしまいます。テストコードでは返り値を期待結果と比較するので、putsは除外します。

次に手順2です。
抽出用のメソッドを用意します。メソッド名の検討は後ほど考えて、暫定的な名前を付けます。

def get_week
end

手順3です。コピーした処理を抽出用メソッドにコピーします。putsは除外します。

def get_week
    # 日付をカレンダーのフォーマットに変更
    displayed_week = week.map do |day|
      # 曜日列に日付がない場合は空白表示
      next '  ' if day.nil?

      # 日付の文字数が1文字の場合は空白を追加して幅を調整
      width = 2
      day.to_s.rjust(width)
    end
    sep = ' '
    displayed_week.join(sep)
end

この状態でテストコードを実行してみると失敗します。メソッドの引数がないからです。コピー元のコードでは、変数weekをパラメータとして使用しているので、weekを引数として追加します。

def get_week(week)
    # 日付をカレンダーのフォーマットに変更
    displayed_week = week.map do |day|
      # 曜日列に日付がない場合は空白表示
      next '  ' if day.nil?

      # 日付の文字数が1文字の場合は空白を追加して幅を調整
      width = 2
      day.to_s.rjust(width)
    end
    sep = ' '
    displayed_week.join(sep)
end

最後にコピー元のメソッドを削除してメソッドを呼び出します。メソッドの引数があることに注意します。

def cal
  # ... 省略
  # 各週の日にちを描画
  days.each do |week|
    displayed_week = get_week(week)
    puts displayed_week
  end
end

メソッドの抽出が完了しました。
リファクタリング前後で比べていかがでしょうか。リファクタリング後のコードは、処理の概要を把握しやすくなったと思います。

このコードを見るとさらに塊があります。繰り返し処理をしている処理です。これもメソッドの抽出ができます。
※テストコードは省略します。

def cal
  # ... 省略
  print_days(days)
end

def print_days(days)
  days.each do |week|
    displayed_week = get_week(week)
    puts displayed_week
  end
end

次に、抽出したメソッド名をリファクタリングしていきます。

メソッド名の修正

メソッドの抽出で作られたメソッドにより、長い処理を意味のある塊として切り分けをしました。しかし、意味のある塊を表現する名前が不適切な場合もあります。名前と処理の表現に乖離があると以下のデメリットがあります。

処理の内容を誤解してしまう
理解するのにコストがかかる

名前が不適切だと、結局は処理の詳細を追わなければなりません。

名前の重要性についての詳細と、良い名前の付け方は、WEB+DB PRESS Vol.110の名前付け大全記事が参考になります。

名前を修正するリファクタリング手順について、リファクタ本では移行的手順が紹介されています。移行的手順は主に以下のステップで行います。

リネームの対象となるメソッドの処理全体に、メソッドの抽出を施す。
古いメソッド名で呼び出している箇所を、新しいメソッド名にリネームする。

メソッドの抽出でリファクタリングした外部の振る舞い3を対象に、メソッド名の修正を行います。
この塊の処理は、表示するカレンダー用に1週間の日付を文字列で返しています。
そこで、修正後の名前をdisplayed_weekとします。Rubyでは命名にget、setをつけない慣習なので、getは省略します。

手順1です。
メソッドの抽出を行います。抽出する範囲は、メソッドの処理全体になります。

def get_week(week)
  displayed_week(week)
end

def displayed_week(week)
    # 日付をカレンダーのフォーマットに変更
    displayed_week = week.map do |day|
      # 曜日列に日付がない場合は空白表示
      next '  ' if day.nil?

      # 日付の文字数が1文字の場合は空白を追加して幅を調整
      width = 2
      day.to_s.rjust(width)
    end
    sep = ' '
    displayed_week.join(sep)
end

次に手順2です。
テストコード、呼び出し元のコードを1つずつ新しいメソッドで置き換えます。

def cal
  # ... 省略
  print_days(days)
end

def print_days(days)
  days.each do |week|    
    puts displayed_week(week)
  end
end

この方法だと、単純にメソッド名を一括置換する方法と比較して、安全に変更することができます。(IDEのリネーム機能は例外かもしれません)。途中でテストコードを実行して、正常動作するか確認ができます。また、途中でリネームの作業を止めてもコードは動くので安心です。

print_daysのメソッド名も不適切なので、同様にリファクタリングしていきます。 print_daysは各週の日付を1行ごとにputsメソッドでコンソールに表示しているので display_weeksとします。

リファクタリングの紹介まとめ

テストコードを作成し、メソッドの抽出とメソッド名のリファクタリングを適用しました。
他の箇所で上記の方法を適用できたり、変数名の修正ができるので、最終的に修正したコードが下記になります。

def cal
  year = 2021
  month = 12
  # 12月の週を配列の要素とする。
  # さらに各週ごとの日を要素とする。曜日に紐づく日がない場合はnil。
  # 曜日のはじめは日曜。
  weeks_of_one_month = [
    [nil, nil, nil, 1, 2, 3, 4],
    [5,  6, 7, 8, 9, 10, 11],
    [12, 13, 14, 15, 16, 17, 18],
    [19, 20, 21, 22, 23, 24, 25],
    [26, 27, 28, 29, 30, 31, nil]
  ]

  # 外部の振る舞い1。
  puts displayed_year_and_month(year, month)

  # 外部の振る舞い2。
  puts displayed_day_of_week(%w[日 月 火 水 木 金 土])

  # 外部の振る舞い3。
  display_weeks(weeks_of_one_month)
end

def displayed_year_and_month(year, month)
  "#{month}月 #{year}".center(20)
end

def displayed_day_of_week(day_of_week)
  day_of_week = %w[日 月 火 水 木 金 土]
  sep = ' '
  day_of_week.join(sep)
end

def display_weeks(weeks)
  weeks.each do |week|
    puts displayed_week(week)
  end
end

def displayed_week(week)
  # 日付をカレンダーのフォーマットに変更
  displayed_days = week.map do |day|
    # 曜日列に日付がない場合は空白表示
    next '  ' if day.nil?

    # 日付の文字数が1文字の場合は空白を追加して幅を調整
    width = 2
    day.to_s.rjust(width)
  end
  sep = ' '
  displayed_days.join(sep)
end

cal

今回紹介した方法は、意外にも簡単にできると思います。実際、私がリファクタ本を実践する前は難しいものだと感じてましたが、プラクティスでもすぐに実践できました。

冒頭のリファクタリングとはで、改善することがリファクタリングの肝だと言いましたが、もう1つ肝があります。 リファクタ本では、リファクタリングについて以下のことも述べています。

リファクタリングは振る舞いを保ちつつ小さなステップを適用していくことであり、ステップを積み重ねていくことで大きな変化をもたらしていくものなのです。ここのリファクタリングでは非常に小さいステップ、またはそれらの組み合わせでできています。

ＭａｒｔｉｎＦｏｗｌｅｒ. リファクタリング既存のコードを安全に改善する（第2版） (p.46). Kindle 版.

つまり、リファクタリングは、いきなり大規模にコードを修正することではなく、小さな範囲から始めることになります。リファクタリングは、熟練者だけができる方法論ではななく、誰でも簡単に始められるものです。

プラクティスで実践してみての所感

私は、lsコマンドを作るまでリファクタ本を実践してきました。実践を通して気づいたことは、「リファクタリングは良い循環を与えてくれる」ことです。具体例として、下記のような、1度改善をするとさらに改善できそうなポイントを自然と見つけられる機会が多くなりました。

メソッドを抽出したら、他にもメソッドを抽出できないかを探る。
メソッドを作るのでメソッド名を考えるようになる。
名前がしっくり来なければ、切り出す範囲を変えてメソッドの抽出を再度行う。
名前を考える際に正しい表現なのか調べる。
- ドメイン内ではどのように使われているか。
- 意味の似た単語の違いは何か調べる。
複数の変数名に同じ形容詞がつくと、データのまとまりにできないか考える。その際に、まとまりを表現する名前を考える

いきなり良い設計が思い浮かばずとも、リファクタリングを繰り返すことで、良い設計にすることも可能なのかなと感じてます。まさに、小さなステップで大きな変化を体感できるようになりつつあります。

リファクタ本では、今回紹介した方法以外にも多くの手法が紹介されています。また、手法だけでなく、なぜその改善をすべきなのか、改善していくと得すること等も説明があるので、良いコードと悪いコードの特徴を知ることもできます。

まとめ

本記事では、リファクタリングのメリットの説明と、リファクタ本にある手軽に始められる手法の紹介をしました。

私は全くの駆け出しですが、今回の記事を通して、「ちょっとリファクタリングしてみようかな」と思って貰えると幸いです。

かなり長いかつ拙い文章ですが、最後まで読んでいただきありがとうございました。🙏

wataのアウトプット