リファクタリング専門チームによるお金周りリファクタリング

こんにちは、エンジニアの @MinoDriven です。 今年2019年4月にリファクタリング専門チームを発足しました。 crowdworks.jp の最重要機能であるお金周りの機能に関して、どのような技術アプローチでリファクタしているかを紹介致します。特に、Railsには適用困難と言われているドメイン駆動設計の考え方を取り入れた手法を解説致します。

---

目次

---

※本記事は下記スライドと関連します。ご参考にどうぞ。

クラウドワークス プロダクトの持続的開発のためのリファクタリング実践アプローチ from MinoDriven

www.slideshare.net

背景

過去記事 にあるように、crowdworks.jpはサービスインから7年経過し、30万行を超えるモノリシックRailsアプリになっています。

コード増加量やファイル変更数が低下し、開発生産性が低下傾向にあります。 技術的負債の蓄積により、事業継続性や成長性を考える上での課題となっております。

---

リファクタリング専門チーム発足

技術的負債に対しては、外から見た挙動を変えずにコードを整理する「リファクタリング」で解決する必要があります。 しかし新機能追加案件に比べると優先度が落ちてしまうなど、力学上どうしても後手後手になりがちです。

そこでプロダクト開発と同じプライオリティでリファクタリングを遂行できるように、2019年4月にリファクタリング専門チーム「バグハンター」を発足しました。 チーム名は、私が制作した、リファクタリング技術によりバグを退治するゲーム「 バグハンター 」に由来しています。

バグハンターは一般的には「セキュリティ脆弱性を発見し賞金を得る専門家」という意味で使われますが、ここでは「バグやバグになりやすい設計を退治する専門家」という意味合いで使っています。 「バグが生まれてからバグ退治するのは退治にあらず。始めからバグが生み出されない、バグを根絶やしにする構造に作り変えるのが真のバグ退治」という想いも込めています。

私を含めチーム人数は2名。 両名ともエレガントなコードや設計が大好きで、なにより私自身は リファクタリング専門要員として採用され 、2019年2月にクラウドワークスにジョインしました。

---

技術的負債

様々な技術的負債を抱える中、目下最大の課題はFat Modelです。

crowdworks.jpではActiveRecordの多くがFat Modelとなっています。 Railsのお作法ではFat Modelがあるべき姿なのかも知れません。しかし実際には巨大化複雑化しすぎていて、下記にあるような技術的負債を抱え、開発生産性に悪影響を及ぼしています。

技術的負債	説明
関連コードの散在(低凝集)	概念的に本来関連し合うコードが、様々なクラスに散在してしまっている。あるモデルに対する処理が別のモデルに書かれているなど責務外の実装が多い。コード追跡が困難。
コピペコード	同じロジックがいたる所に実装されている。仕様変更に弱く、修正漏れがあるとバグ化。
意図不明なコード	何をやらせたいのか意図不明。理解や修正を妨げる。
うねるコード	条件分岐(if, case-when)やループ制御(each do)などが多重にネストしており、遠くから見るとうねっているように見えるコード。複雑で理解困難。
異なるコンテキストの混在	ActiveRecordにあらゆるコンテキストの実装が入り込んでいて、巨大化の主要因になっている。
共通化に反するConcern	共通処理の定義に便利なConcernに、respond_to?やis_a?など特定クラスの場合の条件分岐が大量に入り込んでいて、共通化に反している。複雑な上、Concernをincludeしているクラス全てに変更影響があり、触ると危険。

今回は、現在チームで取り組んでいる「共通化に反するConcern」に関してのリファクタリングを説明します。

---

リファクタリング対象選定

まず、「共通化に反するConcern」をリファクタリング対象として選定した理由を説明します。

crowdworks.jpのコードは30万行以上。限られたリソースの中、全てリファクタするのは不可能です。闇雲にやれば良いというものではなく、例えば大して変更が入らないコードをリファクタしても開発生産性への寄与率は低いものとなります。開発生産性向上に高い効果を見込める箇所を選定することが肝要です。選定で用いた、２つの方針を紹介します。

方針①：パレートの法則（80:20の法則）

パレートの法則とは「全体の少数の要因が全体の多数の結果に影響を与えている」ことを示した経験則です。例えば「売上の8割は全商品の内2割で生み出している」などです。ソフトウェアにも同じことが言えて、「ソースコード全体の内2割が、そのソフトウェアの価値を決定付けるコアロジック」と経験則的に考えられます。この2割のコアロジックを、リファクタリング価値の高い対象として選定します。

方針②：リファクタリング選定基準3軸

書籍「レガシーソフトウェア改善ガイド」記載の考え方で、リファクタリング候補選定には、「価値」「難度」「リスク」の3軸で評価せよとあります。

レガシーソフトウェア改善ガイド (Object Oriented Selection)

価値は「コアロジックの変更容易性が向上した、生産性向上した」などの有益性。難度はリファクタリングの技術的難度。リスクはリファクタリングに失敗した場合どれほどの損害が生じるかの指標です。候補を3軸で評価し、状況的に相応しいものを選定します。

これらの方針に基づいた結果、「仕事周りのモデル」と「お金周りのモデル」が俎上に上がりました。

「仕事周り」か「お金周り」か

crowdworks.jpの仕事周りのモデルとお金周りのモデルは、群を抜いてFatで複雑です。また、リファクタしてほしい箇所をエンジニアの皆さんにアンケートしたところ、やはりこの2つが多く挙がりました。crowdworks.jpはクラウドソーシングサービスでありますから、仕事とお金がコアであることは明白です。パレートの法則にも当てはまりそうです。

では仕事とお金、どちらのモデルをリファクタリング対象とするか比較しました。仕事周りは確かに巨大でしたが、難度やリスクの高いロジックが多いわけではなく、相対的に旨味は少なめでした。一方お金周りは消費税10％対応の修正や、各種電子決済サービスとのやり取りをより堅牢な設計に改善するなど、高価値の変更が多く要望されているのに対し、関連モデルが巨大化複雑化していて高価値の変更の妨げになっているので、お金周りのモデルをリファクタリング対象として選定しました。

お金周りモデルのリファクタリングを妨げるConcern

Railsには共通処理の記述に便利なActiveSupport::Concernがあります。しかしcrowdworks.jpでは正しく使われていません。共通化に反する使われ方がされていて、保守性と理解容易性を損ねています。お金周りのモデルは多くのConcernをincludeしており、更に混乱しています。それぞれ課題を説明します。

課題①：ActiveRecord側の構造に依存したコード

Concernのメソッド内で、include先のActiveRecordのカラムを操作したり、メソッドコールをしています。

module Hogeable
  extend ActiveSupport::Concern

  def complete
    self.completed_at = Time.now  # ActiveRecord側のカラム
    self.foo  # ActiveRecord側のメソッド
  end

ActiveRecordの構造を熟知したロジックとなっているのです。Concern側がActiveRecordの構造に依存していることが妨げとなり、ActiveRecord側をリファクタしようにもできない実装になっています。

課題②：型や構造のチェック(リスコフの置換原則違反)

Concern側の多くのメソッドに、include先のActiveRecordのメソッドの有無を調べるrespond_to?や型を調べるis_a?の判定ロジックが実装されています。

module Hogeable
  extend ActiveSupport::Concern

  def execute
    if respond_to?(:foo)
      self.foo
      if is_a?(ModelA)
        self.fuga
      end
    end
  end

ロジックの共通化が果たされてない上に、この手のチェック分岐が非常に複雑化していて、どのActiveRecordで何のロジックが実行されるのか、理解を極めて難しくしています。リスコフの置換原則に違反しています。

課題③：重要業務概念の埋没

Concernに限らずFat Model全般に言えることなのですが、crowdworks.jp上で扱われる重要な概念が、長大なロジックに数多く紛れ込んでいます。

例えばConcernのとあるメソッド、ズラーッと何十行にも書き連ねられたロジック眺めて、どうもお金関係の演算をしているらしいことは読めるものの、それが具体的に何であるか分からない。関連コードを調べてみたり有識者に尋ねてみたりしたところ、なんと割引金額や請求金額などの重要な演算ロジック。そんな重要なロジックが、長大なメソッドの中に雑多に紛れ込んで埋没している、というのが本当に多く見受けられます。ソースコード上で業務概念が見える化されておらず、仕様理解を妨げます。

……以上の課題により、お金周りのモデルをリファクタするには、モデルに取り巻き複雑化させているConcernをまずリファクタする必要があったのです。

---

どのようにリファクタしたか

Concernをリファクタした手法を以下に説明します。

手法①：Concern側メソッドのinterface化

上記課題①、②に対応する解決手段です。Concernは既に共通化の役割を果たしておらず、Concernをincludeした型ごとに処理が異なっています。こうした型ごとに異なる実装は、オブジェクト指向でいうinterfaceにより解決可能です。詳細ロジックを型ごとに実装するのです。具体的な手順を示します。

まずConcern側のメソッドをinclude先の、全てのActiveRecordへ丸ごとコピーします。このときテストが不足している場合、予めテストコードを書いておきます。

class ModelA < ActiveRecord::Base
  include Hogeable

  def foo
    # 省略
  end

  def fuga
    # 省略
  end

  # コピーしたConcernのメソッド
  def execute
    if respond_to?(:foo)
      self.foo
      if is_a?(ModelA)
        self.fuga
      end
    end
  end

これでテストが通ったならば、次にロジックの最適化をします。上記ModelAにはfooメソッドがあり、また自身がModelAであることからrespond_to?とis_a?は不要です。

class ModelA < ActiveRecord::Base
  include Hogeable

  def foo
    # 省略
  end

  def fuga
    # 省略
  end

  def execute
    foo
    fuga
  end

これでロジックの見通しが良くなりました。ModelAのexecuteメソッドではfooとfugaがコールされることが一目瞭然です。他のActiveRecordに関しても同様に最適化します。

全てのActiveRecordで最適化が済んだら、Concern側のロジックは不要になります。executeメソッド自体削除してしまっても良いのですが、NotImplementedErrorをraiseするロジックに差し替えます。

module Hogeable
  extend ActiveSupport::Concern

  def execute
    raise NotImplementedError
  end

これでinclude先でのinterfaceの実装を強制させることができます。もし今後新たにHogeableをincludeしたときに、そのモデルでexecuteメソッドの実装が不要と判断された場合、interface分離原則違反を疑うことできるので、更に構造のブラッシュアップが見込めるという追加効果もあります。

手法②：ドメインオブジェクト

課題③に対応する解決手段です。ドメイン駆動設計に登場するドメインオブジェクトを利用します。

エリック・エヴァンスのドメイン駆動設計 (IT Architects’Archive ソフトウェア開発の実践)

ドメインオブジェクトとは、業務概念の名前で名付けられ、その概念に関連するロジックがカプセル化されたクラスです。例えばアプリで税込金額を取り扱うならば「税込金額」というドメインオブジェクトを作り、原価と消費税率で税込金額を演算するロジックをその税込金額クラスにカプセル化する、というような設計をします。

# 税込金額
class AmountIncludingTax
  attr_reader :value

  # [Int] amount_excluding_tax 税抜金額
  # [Float] sales_tax_rate 消費税率
  def initialize(amount_excluding_tax:, sales_tax_rate:)
    @value = (amount_excluding_tax * (1 + sales_tax_rate)).floor
  end
end

# 最も簡単なValueObjectの例。
# より堅牢にするならば税抜金額や消費税率もValueObjectとして設計する。

これにより業務概念：クラス：ロジック＝１：１：１となります。このように設計すると、「税込金額に関して仕様変更したい」と将来考えた場合、税込金額クラスだけを見に行き変更すれば良くなります。つまりコード追跡性と理解容易性が格段に向上します。

課題③に登場する例でも同様に「割引金額」「請求金額」それぞれをドメインオブジェクト化します。Fat Modelに埋没していた関連ロジックを抽出して、割引金額クラス、請求金額クラスに委譲するのです。なお、このような値を表現する設計パターンをValueObjectといいます。更に、不完全な初期化を避けるため、ValueObjectには必ず完全コンストラクタパターンを適用します。

Concernのロジックに埋没していた業務概念も、上記のように掘り起こして抽出し、ドメインオブジェクトとして切り出してやります。特に、前項のinterface化リファクタリングではコピーコードが量産されてしまいます。重要演算ロジックを予めドメインオブジェクト化しておけば、処理の共通化を果たすことにもなります。

手法③：コンテキスト分解

課題③に関連する解決手段です。ドメイン駆動設計に登場する「境界付けられたコンテキスト」の考えを取り入れています。これはソフトウェア設計原則「関心の分離」の延長上にある考えで、疎結合高凝集に貢献したり、Fat Modelを抑止しクラスを小さく維持することに貢献します。コンテキストとは文脈、状況であり、状況が異なるとモデリングの仕方や設計がまるで違ってきます。どういうことか説明します。

自動車のモデリングを例に考えてみます。

カーレースゲームのコンテキストにおいては、プレイヤーの操作に合わせて画面上の自動車を動かす必要がありますから、画面上に表示する3Dデータや、操作に合わせて自動車を加速したり向きを変える概念が自動車に付いて回ります。

一方、自動車販売のコンテキストにおいては、価格や車種名、車体番号など、販売に必要な概念が自動車に付いて回ります。そこにはカーレースゲームのような自動車を操作するといった概念は無関係です。

これら全くコンテキストが異なるもの同士を、たったひとつの「自動車クラス」として実装してしまうと、巨大化複雑化し、混乱してしまいます。

従って、同じように見えるモノでも、取り巻くコンテキストを分析し、コンテキストごとにモデリングし、別々にクラス化することが肝要です。 crowdworks.jpの多くのFat Modelは、本来ならばコンテキストごとに別々のクラスとして設計されるべきものが、ActiveRecord制約によってあらゆるコンテキストのロジックがひとつのモデルに実装されてしまっています。

お金周りを例にすると、crowdworks.jpではまず仮払いという形でクライアントさんから弊社がお金をお預かりします。次にワーカーさんが成果物を納品し、検収されて報酬が確定次第、弊社よりワーカーさんへ報酬が支払われる、という仕組みになっております。お金ひとつとっても、クライアントさんから弊社がお預かりするお金と、弊社からワーカーさんへ支払う報酬とではコンテキストがまるで違います。それぞれに取り巻く業務概念やロジックも違ってきます。従ってお金に関してコンテキストを2つ(もしくはそれ以上)に分ける必要があります。

ところで上述の手法②により抽出したドメインオブジェクトはどのフォルダに格納すれば良いのでしょう？ app/models でしょうか？ app/models はActiveRecordの置き場所なので、ここにドメインオブジェクトを格納するとActiveRecordと見分けがつかなくなってしまいます。そこで app/domains を新設し、ここにドメインオブジェクトを格納するようにします。更に、コンテキストごとにフォルダ分けし、抽出したドメインオブジェクトを相応しいコンテキストのフォルダへ格納するようにします。

app/
  └─controllers/
  └─models/
  └─views/
  └─domains/
    └─job_search_context/
    └─payment_context/
      └─domain_object_a.rb
    └─fee_context/
      └─domain_object_b.rb
      └─domain_object_c.rb

---

成果

このリファクタリングは現在進行中ですが、途中成果として以下のようになりました。

構造before→after

beforeではConcernを含めたActiveRecordがFat Modelとしてある構造です。

今回のConcernのリファクタリングを通じて、afterの形に少しずつ近づいています。ConcernやActiveRecordにあるロジックをドメインオブジェクトに次々に委譲していき、ActiveRecordは薄いモデルにしていきます。

技術的負債が低減した

オンラインのメトリクス解析サービス Code Climate Quality で技術的負債を継続的に計測しております。Code ClimateはGithubと連携し、Webサイトで計測結果を閲覧できる他、公開APIで特定のクラスの技術的負債を取得することも可能です。下記グラフは、今回リファクタリングのターゲットとしているお金周りのConcernに関して、公開APIを定期的に叩いてスプレッドシートに記録したものです。

順調に技術的負債が低減していることが分かります。なお、このCode Climateで計測した技術的負債をリファクタリングチームのKPIとしております。

ロジックの見通しが良くなった

上述したように条件分岐が削除されたりドメインオブジェクトとして切り出した結果、ロジックの見通しが良くなってきています。

仕様が見える化された

ドメインオブジェクトとして抽出した結果、どんな業務概念を扱っているかを把握できるようになり、仕様が見える化されてきています。

---

苦労してること

成果は少しずつ出ていますが、様々な困難があります。リソース的には8割近くが下記困難に割かれ、リファクタ設計より大変です。

影響範囲調査が大変

リファクタリングを成功させる上で、影響範囲を正確に知ることは重要です。Rubyは動的型付け言語であるために、静的なコード追跡、解析が大変です。

ところで私は前職で静的型付け言語であるC# を使っていました。VisualStudioのリッチな参照機能やコードジャンプ機能により、正確にコード追跡したり影響範囲を調査することができていました。

現在はRuby用IDEである RubyMine を使用していますが、RubyMineの参照機能は正確とは言えず、漏れがあったり無関係なコードを拾ってきたりするため、結局メソッド名で文字列全検索などしており、コード解析の生産性は高いとは言えません。

一方、ドメインオブジェクト化するにあたり、ユニークな命名をすることで使用箇所追跡の生産性を高めています。

仕様不明瞭

ソースコードから、仕様を読み取れない、仕様と結びつかないことが多々あります。仕様が分からないと、(使われていない)不要なコードかどうか判断がつかないですし、ドメインオブジェクトとして抽出することも困難になります。過去のドキュメントを調査したり、有識者に話を聞くなどして対処しております。

テストが大変

基本的にテストコードが不足しています。上記仕様不明瞭も手伝って、何が正解であるかテストデータを用意するのにも難儀しております。どんな条件が成立したときにどのコードをパスするか、テスト対象のプロダクトコードをつぶさに解析し、それに基づきテストコードを書くなどで対処しております。

---

その他リファクタリングする上で重要な考え方

以上が弊社リファクタリングチームの活動になりますが、リファクタリングする上で欠かせない重要な考え方があります。

理想の設計を追い求めること

そもそもの話として「より良い設計とはどういうものか」を知っていないと、良いリファクタはできません。理想形を描き、現状とのギャップを認識できてこそ、良い設計へリファクタリングの歩みを進めることができます。設計知識が中途半端だと、リファクタリングも中途半端なものになり、極端な話、単にロジックを引っ掻き回すだけで品質が向上しないということがあります。SOLID原則に代表されるソフトウェア設計原則や、近年注目されているドメイン駆動設計などの理解を深め、設計スキルを常に高めています。

既存コードを一切信用しないこと

リファクタリングを中途半端にさせず、本当の役に立つものに昇華させるために重要な考え方です。私は既存コードを一切信用していません。もちろんクラス名やメソッド名ですら信用していません。名前ひとつとっても、仕様と異なる、意味が異なる名前が付与されてるものが多々あるからです。もちろんロジックですら怪しいものがあります。「これは何を解決したいコードなのか」を分析し、あるべき設計をゼロベースで組み上げます。私はこれを「正体を見破る」と呼称しています。正体を見破り、正体を正しく表現した名前をドメインオブジェクトに付与することが肝要です。

---

We are Hiring!!

クラウドワークス では、積極的にリファクタリングを推進していけるエンジニアを募集しております。 話を聞いてみたい、などでも構いませんので、お気軽に遊びにきてください。

www.wantedly.com