こんにちは、MERY のサーバサイド開発をしている末並 @a_suenami です。 TDD、アジャイル、DB 界隈等によく出没しますが、最近では糖質警察としてのほうが広く知られている気がする今日この頃です。糖質制限に興味ある方はぜひウィスキーを片手にケトン体の話でもしながら飲みましょう。

さて、現在、MERY は Ruby on Rails で開発されていますが、最初にリリースされたのはもう 3 年近く前であり、その頃とはサービスを取り巻く状況が大きく変わってきています。これまで多くのユーザの「かわいい」を支え、よい体験を提供し続けてきた現在の MERY とそのコードベースを否定することは決してできませんが、日々変わるユーザの「かわいい」ニーズと我々のビジネス状況の変化にスピード感を持って追随していくためにはいわゆる「技術的負債」を返済していく必要があることも否定できない事実です。

そこで今回は実際の Rails アプリケーションにありそうなサンプルコードとともにそのリファクタリングについて紹介しようかと思います。普段、私が MERY のコードをリファクタリングする際にも意識しているテクニックになります。

技術的負債とは？

ソフトウェアエンジニアの人で「技術的負債」という言葉を聞いたことがないという人はあまりいないと思いますが、巷では単なるクソコードを揶揄するためだけにその言葉が使われている印象を受けることも少なくありません。もちろんクソコードを書かないに越したことはありませんが、それは技術的負債以前の話であり採用や教育によって解決するべき課題でしょう。 本来の金融的意味での負債がそうであるように技術的負債もそれ自体は悪いことではありません。資金がないと事業に推進力が生まれないのと同様、今この瞬間にこの機能をリリースしないと貴重なビジネスチャンスを失うということはありえます。重要なのは通常の負債と同様、きちんと返済計画を立てることであり、そのためにはリファクタリングのパターンを知り使いこなせるようになる必要があります。

リファクタリングはいつやるべきか

リファクタリングは「時間があればやる」という優先度になりがちな作業ですが、私の知る限り、その感覚でうまくいく事例はほとんどありません。 外部からの振る舞いが変わらない以上、非エンジニアからの価値はゼロに等しく、そのためのまとまった時間を確保することは実際にはかなり厳しいでしょう。（実際には開発スピードの改善やチームの士気向上という効果はあるのですが、それはエンジニア同士でしか計測ができません。）

したがってリファクタリングを継続的におこなえるチームを作るためにはそれを「特別なタスク」としてとらえるのではなく「定常業務のひとつ」ととらえなければなりません。朝出社したらタイムカードを押すように、数時間おきにメールチェックをするように、帰る前には日報を書くように、リファクタリングも日々の業務の中に紛れ込ませて進めていくべき業務なのです。 リファクタリングといえばそれがズバリそのまま書籍名になっているマーチン・ファウラー氏の名著がありますが、そこでは「いつリファクタリングをすべきか？」という問いに対していくつかの回答があります。

• 同じコードを 3 回書いてしまったとき
• 既存機能に変更をするために該当のコードを読むついでに
• 新機能を実装してバージョン管理システムにコミットする前

同じコードを書いてしまったときというのはわかりやすいでしょう。いわゆる DRY 原則にしたがって重複を排除しようということです。ここで重要だと思うのは「3 回」と回数を指定しているところです。2 回目だとたまたまコードが同じになった可能性を否定できず、早すぎる最適化になりうるることを示唆しています。文脈を無視してコードの見た目が似てるからまとめたという過度な抽象化は DRY であるどころか害悪ですので気をつけましょう。

2 つ目のシチュエーションは私が最も多く遭遇するケースです。既存機能に変更をおこなうケースではまずその部分のコードを読み、どこをどのように変更すれば要件を満たせるかを検討しますが、コードを読む時点で読みにくいと感じたり意味がわからなかったりするケースがあります。この場合、後述する仕様化テストを書き、自分が理解しやすく、かつ、これから自分がやろうとしている変更をしやすいだろうと思うように書きなおしてみます。これが機能変更前のリファクタリングになります。

3 つ目はテスト駆動開発（ TDD ）をちゃんと実践できている人にとっては当たり前だと思いますが、機能を実装したあとに自分以外の誰かに見てもらう前にリファクタリングをおこないます。TDD に関してここで言及すると紙面が足りなくなるので割愛しますが、テストファーストを意識していれば今書かれているプロダクトコードに対しては確実にテストコードが存在することになりリファクタリング可能ですし、このシチュエーションは主に新機能の開発時に遭遇すると思うので既存のコードベースは関係なく、もっともリファクタリングを実践しやすいケースかもしれません。

レガシーコードのリファクタリングを行うための重要なテクニック

リファクタリングはソフトウェアそのものの振る舞いを変えずに内部の実装方法を変えることであり、通常は振る舞いを変えてないことを保証するためにテストコードが必要です。しかし、世の中にはテストコードが存在していないコードベースも存在しており、そういったソフトウェアのリファクタリングをおこなうためのテクニックとして「レガシーコード改善ガイド」という書籍ではいくつかの手法を紹介しています。

これらのテクニックは非常に多くの現場で活用可能なのでぜひ積極的に活用していきましょう。

シグネチャの維持

テストコードがない状態でも可能なリファクタリング手法として「レガシーコード改善ガイド」では 2 つの手法が紹介されています。

1 つは「コンパイラまかせ」で、テストが本来担う役割をコンパイラに任せる方法です。コンパイラの静的型検査を利用することによって修正必要箇所を教えてもらうという方法で、その言語の型システムや静的検査時の厳密さ次第ではかなり有効な方法ですが、残念ながら動的型付き言語である Ruby でこの手法は適用できません。

もう 1 つの方法が「シグネチャの維持」です。これはある場所に定義されたメソッドを"そのまま"別のもっとテストしやすいクラスやモジュールに移動させることです。当たり前だと思う人もいるかもしれませんが、人間の欲望というのはおもしろいものでリファクタリングをするつもりになったらつい"ついでに"他のこともやりたくなってしまうものです。しかし、テストコードのないプロダクトコードに対して、それがどんなに簡単に見えて自分自身は修正することに自信があったとしても、シグネチャを維持しない状態で変更をするのはリスクをともないます。

リファクタリングの第一歩は何よりもまずテスト可能な状態にすることであり、それ以上のことを最初にやるべきではないのです。

メソッドの抽出

ある場所にあるメソッドをそのまま別のクラスやモジュールに委譲する場合は「シグネチャの維持」が使えますが、巨大なモンスターメソッドのごく一部が非常に複雑になっていてその部分だけ別に切り出したいケースもあるでしょう。むしろ実際の現場ではそのケースのほうが多いと思います。

この場合には「メソッドの抽出」という手法が使えます。その名の通り、あるメソッド内のある処理を別のクラスやモジュールのメソッドとする手法で、本来はもともとのメソッドにテストコードがある場合か IDE に自動リファクタリング機能ががある場合に利用するのが好ましいですが、そうでない場合にも以下の手順で慎重に行えばリスクを最小化して変更を行うことが可能です。

1. 抽出したいコードを特定してコメントアウトする。
2. 新しいメソッドを作成し、抽出したいコードをコピーする。
3. 新たに作成したメソッドをただ実行するだけのテストコードを書き、引数・戻り値を調整する。
4. コメントアウトした箇所を新たに作成したメソッドの呼び出しに変更する。
5. コメントアウトした部分を削除する。

「レガシーコード改善ガイド」の中では 3 の手順が「コンパイラまかせ」を使うように紹介されていますが先述したように Ruby ではこの手法は使えないため、テストコードで代用します。この手順を経ることでグローバル変数やもともとのクラスのインスタンス変数へ依存していたことがわかることがあり、それらも引数で渡してあげるようにしましょう。「シグネチャの維持」でも述べたようにこの状態におけるリファクタリングではコードそのものはコピー以外のことをしてはいけません。

仕様化テスト

TDD では何を実装するべきかをテストコードで表現し、それをパスするようにプロダクトコードを実装します。しかし、すでに存在しているコードにテストコードがない場合はこの方法は使えません。そこで用いる有効な手段が「仕様化テスト」です。

仕様化テストとは現在の実装が正しく仕様を満たしていると仮定して書かれるテストコードのことです。仕様に基づいて実装をするという本来のソフトウェア開発の真逆の考え方で、現在の実装を仕様として再定義するということになります。

シグネチャの維持やメソッドの抽出もそうですが、リファクタリングの最中というのはついついいろいろなところに手を出したくなるものです。リファクタリングの仮定で今まで気づかなかったバグや好ましくない挙動に気づくことも少なくなく、そのときに"ついでに"なおしてしまおうとすることはよくあります。しかし、テストコードがない状況においてはこれは危険な発想で、自分にとって不具合だと思ったことがある状況において重要な振る舞いであることもあるので、まずは現在の振る舞いを正しく表現するテストコード、つまり仕様化テストを準備することが大切なのです。

Rails アプリケーションにおけるリファクタリングの実践

ここからは実際の Rails アプリケーションでどのようなリファクタリングを行っていくかをサンプルコードを交えながらご紹介していこうと思います。MERY にもまだファットコントローラーやファットモデルとなっている部分があるため、今回はその改善について 2 つのアンチパターンとその対処法という形で紹介します。

アンチパターン: とりあえずインスタンス変数

私はこれは明確にアンチパターンだと思っているのですが、ビューテンプレートで利用する変数をすべてインスタンス変数としてアクションメソッド内で宣言してしまうケースがよくあります。MERY 内でも最近はあまりありませんが以前に実装された部分だとまだ残っていることがあります。これは以下のような点で扱いが難しいです。

• アクションメソッドが長くなってしまい可読性が下がる。
• 一般にコントローラーはテストしづらく、やろうと思うと実際にもしくは擬似的に HTTP リクエストを該当のエンドポイントに送る必要がある。
• コントローラーのインスタンス変数とはいっても実際はそのリクエスト内においてはグローバル変数のようなものであり、どこでどういう風に参照・更新されているかを把握するのが難しい。

特に重要なのは 3 つ目で、MERY だと PC とモバイルで異なるテンプレートを使っていたり、パーシャルテンプレートを多用していたりするため、影響範囲が広く小さな修正でも確認箇所が増えてしまうということになりがちです。

対処法

不幸中の幸いはこれらがメタプログラミングによって動的に生成されたものでなく、ソースコード内の検索で宣言されている箇所・参照されている箇所をほぼ見つけられることです。Ruby のよいところでもあり悪いところとしてメタプログラミングのやりやすさがあり、例えば define_method や class_eval、instance_eval などで動的にメソッド定義可能ですし、逆に send で動的に呼び出し可能です。今回問題にしているインスタンス変数も instance_variable_set や instance_variable_get で同様の問題は起こり、実際にそのメソッドや変数が定義されている箇所や参照されている箇所を特定しにくいことがしばしば問題になります。

アクションメソッド内で宣言・代入されビューテンプレートで参照されるインスタンス変数において、現状の MERY ではほぼこういった問題はないため、きちんとしたテクニックさえ知っていればリファクタリングをおこなうことが可能です。

1. コントローラーのインスタンス変数に代入されている部分を"そのまま"モデルやヘルパーに移動する。（メソッドの抽出・シグネチャの維持）
2. views/ 以下の全ファイルを対象にコード内検索をかけ、そのインスタンス変数を使っているところを新たに作成したメソッドを呼ぶようにする。
3. 作成メソッドの仕様化テストを書く。
4. ブラウザで手動動作確認をする。

もちろん 4 も理想的には feature spec や Cucumber ライクなフレームワークによる自動シナリオテストで行われるのが理想ですが、まずはファットになっているコントローラーから他のコンポーネントへ処理を移譲していき、モデルやヘルパーのテストで品質を安定させていくことが目的であり、シナリオテスト自動化はこれらの積み重ねによってようやく達成可能になるので、これが第一歩と言えます。

例

以下のように条件分岐やループ処理を含むコードがアクションメソッド内にあるとします。

class UserController < ApplicationController
  def mypage
    @user = User.find(params[:id])
    # 制御構造を含む複雑な処理
    @some_array = []
    if @user.some_condition?
      @user.some_collection.each do |element|
        @some_array << do_something_to(element)
      end
    else
      @user.another_collection.each do |element|
        @some_array << do_otherthing_to(element)
      end
    end
  end
end

これを次のようにほぼそのまま User クラスのメソッドとして抽出します。

class User
  def some_array
    some_array = []
    if self.some_condition?
      self.some_collection.each do |element|
        @some_array << do_something_to(element)
      end
    else
      self.another_collection.each do |element|
        @some_array << do_otherthing_to(element)
      end
    end
    some_array
  end
end

これによりビュー側では

<%= @some_array.each do |element| %>

としていたところが

<%= @user.some_array.each do |element| %>

のようになります。

ここのポイントは some_array の処理は @user から self への置換こそあるもののそれ以外に一切手が加えられてないことです。これ以上手を加えるのはこのメソッドのテストコードが十分に整備されるまで決してやってはいけません。

アンチパターン: 賢すぎるビュー

ドメイン駆動設計（DDD）をご存知の方はご存知の人も多いかと思いますが、本来ドメイン層（ビジネスロジックと言い換えてもよい）にあるべきはずの振る舞いが画面に漏れ出してしまい、画面の変更がしにくくなるという状態を「スマートUI（利口なUI）」と呼びます。

これは Rails のアプリケーションではモデルやヘルパーではなくビューテンプレートに複雑な処理が出現するという形で現れます。UI というのはニーズの変化によって変更されやすい箇所であり、それがテストしにくいビューテンプレートに直接書かれていることは大規模なサービスになると非常に問題になってくるので改善が必要になります。

対処法

このアンチパターンも「とりあえずインスタンス変数」アンチパターンと同様、モデルやヘルパーに振る舞いを移動するということで対処するのが基本ですが、ERB や Haml 等のテンプレートエンジンから通常のクラスやモジュールへコードを移動することになるので本来の意味での「メソッドの抽出」は適用できず単なるコピー以上のコード変更をすることになります。そのため、先に紹介したリファクタリングよりさらに慎重に行う必要があります。

1. ビューテンプレートに直接記述されている複雑な処理を"制御構造はそのままの状態で"モデルやヘルパーに移動する。（「メソッドの抽出」の応用）
2. 対象のビューテンプレートでは新たに作成したメソッドを呼ぶようにする。
3. 作成メソッドの仕様化テストを書く。
4. ブラウザで手動動作確認をする。

ポイントはメソッドの制御構造を変えないことです。

例

以下はパンくずリストの表示をビューテンプレートで実装している例になります。

<ul class="breadcrumb">
  <li itemscope itemtype="http://data-vocabulary.org/Breadcrumb">
    <a href="<%= root_url %>" itemprop="url">
      <span itemprop="title">MERYトップ</span>
    </a>
  </li>

  <% if @user.group.present? %>
    <li itemscope itemtype="http://data-vocabulary.org/Breadcrumb">
      <a href="<%= group_url(@user.group) %>" itemprop="url">
        <span itemprop="title"><%= @user.group.name %></span>
      </a>
    </li>
  <% end %>

  <%# 複雑な処理 %>

  <li itemscope itemtype="http://data-vocabulary.org/Breadcrumb">
    <a href="<%= user_url(@user) %>" itemprop="url">
      <span itemprop="title"><%= @user.name %></span>
    </a>
  </li>
</ul>

このコードは制御構造は複雑ですがレンダリング処理そのものは単純なループ処理で実現でき、簡単なバリューオブジェクトの導入で以下のように整理できます。

class BreadcrumbItem
  attr_reader :title
  attr_reader :url

  def initialize(title, url)
    @title, @url = title, url
  end
end

<ul class="breadcrumb">
  <% user_breadcrumb_items(@user).each do |item| %>
    <li itemscope itemtype="http://data-vocabulary.org/Breadcrumb">
      <a href="<%= item.url %>" itemprop="url">
        <span itemprop="title"><%= item.title %></span>
      </a>
    </li>
  <% end %>
</ul>

この変更によって複雑なコードはすべてヘルパーへ移動することができます。

module UsersHelper
  def user_breadcrump_items(user)
    items = []
    items << BreadcrumbItem.new('MERYトップ', root_url)

    if user.group.present?
      items << BreadcrumbItem.new(user.group.name, group_url(user.group))
    end

    # 複雑な処理

    items << BreadcrumbItem.new(user.name, user_url(user))
  end
end

この変更はコードのコピー以上のことをやっているため厳密には「メソッドの抽出」とは言えませんし、それをテストコードなしの状態で行っているため「リファクタリング」と呼んでいい変更とも言えませんが、十分に小さい範囲を慎重に行う限りにおいては通常の「メソッドの抽出」や「シグネチャの維持」と同様にリスクを最小化して実施することができます。「レガシーコード改善ガイド」で紹介されている手法のある種の応用と言えるでしょう。

これ以降は抽出したヘルパーメソッドに仕様化テストを記述し、通常のリファクタリングを行っていくことになります。

まとめ

リファクタリングには多くのテクニックがあり、正しい手順でリスクを最小限にしながらおこなうためにはそれなりの知識と習熟が必要になります。しかし、リファクタリングの結果を保証するためのテストケースが十分に存在している限り、それを必要以上に恐れる必要はありません。

それよりも難しいのは何よりもまず「リファクタリング可能な状態にする」ことであり、世の中に多く存在しているであろうテストコードがないソフトウェアにとってはむしろこちらのほうが重要なスキルであり、単にリファクタリングをおこなうことよりもさらに多くの知識と高い技術が必要になります。

今回は実際の Rails アプリケーションで陥りがちな 2 つのアンチパターンを例にとってその具体的な改善方法を紹介しましたが、技術的負債の返済方法は他にも多くのテクニックやアプローチがあります。これを読んだみなさんがこれをきっかけにそれらを学び、技術的負債を適切にマネジメントしてサービス価値の最大化につながれば幸いです。

推薦書籍

テストコードがないコードをリファクタリング可能にするテクニックについては先に紹介した「レガシーコード改善ガイド」に数多く紹介されていますし、その後のリファクタリングについてはマーチン・ファウラー氏の「リファクタリング」を参考にすれば多くのコードを改善していくことができるでしょう。

ここではさらに役に立つであるであろう 2 冊の書籍をご紹介します。

リファクタリング: Ruby エディション

その名の通り、「リファクタリング」の Ruby 版になります。もともとの書籍は多くのサンプルコードが Java や C++ で記述されており、コンパイラによる静的検査を前提とするテクニックも多いため、Ruby ではそのまま適用できない手法も存在します。この書籍ではそういった部分を Ruby ではどうするかという観点から紹介されているため、Ruby を利用しているエンジニアは必見の書籍になるかと思います。

ソフトウェアテスト技法ドリル

リファクタリングが可能になればその後はテストケースの追加とプロダクトコードの修正を繰り返しながら設計を改善していけばよいのですが、それを続けていくと、どの程度・どういう観点でテストを書けばいいかという問題に直面することがあります。 もちろんこれは TDD 的な開発者視点でのテスト、ユーザ視点での QA テスト等によっても違いますし、テストレベル（ユニットテスト、結合テスト、システムテスト etc. ）によっても違うのですが、そういったテスト観点の洗い出しや分析、適切なテスト設計を学ぶためにはこの書籍が最良かと思います。