👏

リファクタリング

2020/10/10に公開

この記事はプログラミングの世界をほんの少しよくするための記事です。

リファクタリングという単語を聞いたときに、「いっぱい時間をとってやる作業」「気合いを入れてやる改善作業」「大変なもの」「やりたいんだけど、上司が許してくれない」というイメージを持っている人が多いかもしれません。そのとき想像しているものは決してリファクタリングではありません。

リファクタリングとは「外から見た振る舞い(GUIなら画面、関数なら入出力、副作用)を変更せずに、コードをほんの少し改善する行為」のことです。

ほとんどの人が想像するものは「リデザイン」とか「リアーキテクティング」と呼ぶべきもの、つまり「再設計」になっています。

この記事を読んで、覚えておいてほしいことは「リファクタリングは、安全に少しずつ書き換える」ための技術なので、「いっぱい時間をとってやる作業」ではありませんし、「気合いを入れてやる改善作業」でも「大変なもの」でもありませんし、「やりたいんだけど、上司が許してくれない」ものでもありません(本来のリファクタリングすら上司の許可がないとできないかもしれませんが、その場合はもはや組織論なので、この記事の範疇ではありません)。

ボーイスカウトルールというものがあります。ほんの少しずつ綺麗にする活動のことです。あなたが何かをコミットする時に、ほんの少しのリファクタリングを混ぜていけば、そのプロジェクトは技術的負債を貯めづらい強靱なプロジェクトになっていきます。

ちなみに、JavaScript/TypeScript を使ってる人は、素直に、リファクタリング 既存のコードを安全に改善する(第2版) | Ohmsha を買ってマスターするまで読んで、実践してを繰り返してください。それが一番早いです。こんな記事を読んでる場合ではありません。

Javaの人は新装版 リファクタリング 既存のコードを安全に改善する | Ohmshaでいいかもしれませんが、最新のJavaだと事情が変わってるかもしれません。

Rubyの場合は、リファクタリング Ruby エディションがあったんですが絶版です。一応復刊ドットコムで復刊してるようですが、めちゃくちゃ高いのと、さすがに内容が古いと思います。

読むべき本は、Martin Fowler のリファクタリング本です[1]

リファクタリングのやり方

再度書きますが、リファクタリングは外から見た振る舞いを変えずに中身を変更するものです。加えて、リファクタリングで覚えておくべき一番重要な原則として「小さく変更する」というものがあります。

リファクタリングが大変になるのは、一度に複数の変更をしてしまっているからです。

まず間違いがない、というくらい確実でかつ簡単な変更を繰り返すことで、簡単・確実にリファクタリングができるようになります。

const someFunction = (hoge: string) => {
  apiCall(`magic-${hoge}`)
}

たとえば、このようなコードがあったとします。apiCall といういかにも API 呼び出しな関数を叩くコードです。引数は hoge 変数の中身によって magic-hoge だったり magic-fuga かもしれません。

  • magic- という文字列は、言ってみればマジックナンバー(文字列ですが)です
  • API呼び出しの引数を別の場所で組み立てたいかもしれません
  • API呼び出しの引数をリテラルで書きたくないかもしれません

色々な考え方により、このコードのリファクタリングが可能です。

const API_MAGIC = 'magic'
const getApiName = (arg: string) => `${API_MAGIC}-${arg}`

const someFunction = (hoge: string) => {
  const apiName = getApiName(hoge)
  apiCall(apiName)
}

このような形にしたいとします。最初のコードを、一気にここまで変更するのは望ましくありません。

もちろん、この程度のコードなら、皆さんの実力であれば、ミスなく書き換えられるかもしれませんが、それでもより小さく確実に変更すべきです。

const someFunction = (hoge: string) => {
  const apiName = `magic-${hoge}`
  apiCall(apiName)
}

まず第一歩として、意味がわかりやすくなるように apiName という変数に入れることで apiCall の引数の意味を明確化してみます。これくらいの変更であれば間違えることもないでしょうし、ユニットテストで確認しやすいでしょう。

好みによりますが git squash することを前提として、マイクロコミットしておくと色々捗ります。

次に

const getApiName = (arg: string) => `magic-${hoge}`

const someFunction = (hoge: string) => {
  const apiName = getApiName(hoge)
  apiCall(apiName)
}

apiName の生成を getApiName という関数に外だしします。

最後に、マジックナンバーを定数にしてしまいます。

const API_MAGIC = 'magic'
const getApiName = (arg: string) => `${API_MAGIC}-${arg}`

const someFunction = (hoge: string) => {
  const apiName = getApiName(hoge)
  apiCall(apiName)
}

もっとも apiName はやりすぎな気がします。getApiName をしているんだから、それは apiName でしょうし、apiCall の JSDoc に十分な説明があればなおさらです。

気が変わったので、

const API_MAGIC = 'magic'
const getApiName = (arg: string) => `${API_MAGIC}-${arg}`

const someFunction = (hoge: string) => {
  apiCall(getApiName(hoge))
}

にしてしまいましょう。

さらに

const API_MAGIC = 'magic'

/**
 * API名を取得する
 * arg - API名を決めるための○○
 */
const getApiName = (arg: string) => `${API_MAGIC}-${arg}`

const someFunction = (hoge: string) => {
  apiCall(getApiName(hoge))
}

JSDoc も追加しておきましょうか。

もしかしたら API_MAGIC は別ファイルにあるべきかもしれません。

import { API_MAGIC } from './constants'

/**
 * API名を取得する
 * arg - API名を決めるための○○
 */
const getApiName = (arg: string) => `${API_MAGIC}-${arg}`

const someFunction = (hoge: string) => {
  apiCall(getApiName(hoge))
}

というような改修をしているかもしれません。

最初に提示したリファクタリングの予定からは色々とズレています。これも小さくリファクタリングをしながら、考えを巡らせた結果、よりよいコードを追求できたということです。

今回のリファクタリングは、説明用にでっちあげたコードなので、あまりいいサンプルではないかもしれませんが、ポイントとしては、これくらいの小さな修正を繰り返すことです。マイクロコミットであれば間違いを犯しても途中からやり直すこともできますし、過程を調べることもできます。push するときには squash で、コミット履歴をまとめておくべきですが。

リファクタリングテクニックには様々なものがあります。

  • 関数に外だしする
  • 関数をインライン化する
  • 変数化する
  • 変数をインライン化する
  • ファイル分割する
  • ファイル統合する
  • JSDoc を追加する
  • 使われていないものを削る

etcetc...

こういった色々なテクニックを細かく積み重ねることで、最終的にメンテナンスしやすく、読みやすいコードにしていくのです。

鉄則

リファクタリングをしてるときは、動作の変わる改修はしないようにしましょう。

  • リファクタリング
  • 動作が変わる改修

は別のものです。必ず分けて行いましょう。同時にやるとそれはリファクタリングではありません。

マイクロコミットのススメ

git を使っているならマイクロコミット、つまりリファクタリングの工程1つで1コミットをしてみましょう。

git ならほぼノーコストで、ブランチもコミットも作り放題です。リファクタリングでは、しっくりこないとき、やり方にミスをしたときに以前のバージョンに戻したいということもありますが、マイクロコミットをしておけばそれらも簡単です。

ただ、マイクロコミットは過程としてやるべきなのであって、プロジェクトの一履歴で残したいものでもありません。

そのためリファクタリングが一段落したら git squash で、マイクロコミットを1つのコミットにまとめあげましょう。

注意すべきこととして、git squash は歴史改ざんと同じなので、リファクタリング中のブランチを他の人には触らせないようにしましょう。混乱を招きます。

リファクタリングで難しい点

リファクタリングが難しいとすれば、リファクタリングにはゴールはありませんし、決して正解というものがない点でしょう。

そもそもコードのリーダビリティなんてものは、全人類の共通認識ではありません。あるコードを書き換えたとき、読みやすくなったという人もいれば読みづらくなったという人もいるでしょう。

実際、Martin Fowler もリファクタリングで必要なものは「嗅覚」であるといっていて、とても感覚的なものです。経験と勘に裏打ちされたようなものです。

リファクタリングテクニックの数々には、正反対のテクニックが存在します。たとえば、「関数を外だしにする」と「関数をインライン化する」です。これらは正反対の操作です。

それは時と場合によっては、どちらの操作も正解になりうるからです。

でも大きな目で見たときに、より良い設計や、より良いコードがあるのも事実です。これらはリファクタリングだけでは身につけることはできません。

プログラミング上達したい人に繰り返し読んで欲しい4冊改訂版|erukiti|noteという以前Noteで書いた記事でも取り上げたClean Architecture - アスキードワンゴは、設計のエッセンスについてコンパクトに学べる良著です。

安全にリファクタリングをする

IDEやエディタによっては、リファクタリングは半自動で行えるものです。

たとえば、VSCode で IntelliSense に対応している環境なら、ファイル名を書き換えたり、ディレクトリを移動すれば、勝手に、インポートパスを書き換えてくれます。

キーワードにフォーカスを当てている状態で、デフォルトキーバインドなら F2 を押せばそのキーワードの名前を、文脈を見て正確に書き換える機能もあります。

CMD+. (WindowsならCTRL+.)で、幾つかのリファクタリングメニューから選んでリファクタリングも可能です。たとえば、関数をファイルに外だしするような機能です。

VSCode はまだまだリファクタリング機能が少ないですが、Visual Studio 本家や InteliJ IDEA などの本格的な IDE なら、もっと多彩なリファクタリング機能を持っています。

これらを使えば、基本的には壊すことなく安全にリファクタリング可能です[2]

人力のみでリファクタリングする場合でも、ユニットテストなど、自動テスティングがあれば、リファクタリングはより安全に可能です。またウェブアプリや GUI のようなものなら、見た目を人力で判定することもできるでしょう。もちろんその見た目も画像回帰テストのような方法で安全性を担保できるはずです。

また、組織によっては自動テスティングが不十分なケースもあるでしょう。場合によっては QA 頼み、つまり人力による確認を頼りにする事例もあるかもしれません。

リファクタリングをどれだけ安全に行えるかは、そのプロジェクトの置かれた状況にも依存します。

脚注
  1. Ruby edition は例外的に著者が違いますが、Martin Fowler の本をベースに、Ruby に合わせた本なので問題ありません。内容が古くなってるとは思いますが、良い本なのは間違いありません。 ↩︎

  2. 基本的には、というのは場合によってはバグとか操作ミスとかで壊すこともあるかもしれないからです。エディタ・IDEが書き換えてくれる変更点の確認はした方がいいです。 ↩︎

Discussion