500行超のスパゲッティコードなメソッドを10行に整理したお話

こんにちは。toyocです。READYFORでエンジニアをやっています。
最近までバックエンドフロントエンド問わず不具合を直したり、古くなって手がつけられないコードやモジュールを掃除したり、もくもく会で料理したり、用務員のおじさん的な働きをしておりました。
この記事ではお掃除の一環で、500行超のスパゲッティコードなメソッドを理解・分解・再構築して10行に整理してまとめた話をします。

対象となるメソッドについて
メソッドを理解する
メソッドを分解する
メソッドを再構築する

1.対象となるメソッドについて

READYFORには実行者が開設するプロジェクトページがあるのですが、今回対象となるのはその更新を担うメソッドになります。
プロジェクトページは大きく分けると下記の4つの情報に大別されます。
　①タイトルなどの基本情報
　②プロジェクトの説明
　③支援者が選択するリターン
　④検索などに使われるタグ
実際に実行者がプロジェクトページを編集する際は、それぞれについて1画面ずつ計4画面を遷移しながら編集していくことになります。（執筆時点）
それぞれの画面からフォームを送信していくわけですが、送信先は1つのエンドポイントに集約されています。
さらに、①や③には画像情報も含まれるのですが、それをAjaxで非同期に更新できるようにもなっています。これの受け口も同じエンドポイントです。

ここからさらに話が複雑になるのですが、READYFORでは現在実行者に「シンプルプラン」「フルサポートプラン」の2つのプランでサービスを提供しています。（執筆時点）
（ご興味ある方はこちらをご覧ください）
それぞれのプランで提供機能に差異があり、更新の挙動が異なります。特に「シンプルプラン」は提供サービスの仕様上、プロジェクトの公開前と公開後でも挙動が変わってきます。

つまり、まとめると1つのエンドポイントで下記のパターンを全部引き受けています。

（①〜④のフォーム送信 + ①③の画像Ajax非同期通信）×（フルサポート + シンプルプラン公開前&公開後）

そしてその処理内容はほぼ一つのメソッドの中にたくさんのif文と共に記述されています。これが今回対象となるメソッドです。
とても複雑なコードになっているのは想像に難くないと思います。
（ちなみに、循環的複雑度を測ってみたら64でした。一般的には50以上は複雑すぎてテスト不可能、バグの温床になるとか。やばいですね。）

2.メソッドを理解する

このメソッドを整理するにあたり、中でどんな機能を担っているのか理解する必要がありました。
ということで、まずは各パターンの条件と入力でどんな結果が期待できるか、テストコードに書くことから始めました。いわゆるブラックボックステストですね。
しかし、前述したとおり、とても複雑なので、正常系のテストだけに絞ることにします。それでも、パターン数が多いこともあり、書き上げてみたらテストコードだけで900行ぐらいの量になりました。

書き上げることで、おおよそ機能は理解できました。次に中の処理を詳しくみながら分解していくことにします。

3.メソッドを分解する

テストを書くことはできたので、えいやで色々いじってもテストさえ通れば少なくとも正常系については問題ないはずです。
というわけで、前述した下記のパターン毎にメソッドを分割し、可読性をあげようと思います。

（①〜④のフォーム送信 + ①③の画像Ajax非同期通信）× （フルサポート + シンプルプラン公開前&公開後）

ここではif文だけを見ながらなるべく機械的に分解してくことにします。

1.まずはフルサポートorシンプルプラン公開前orシンプルプラン公開後、の3つのパターンごとにメソッドを分割します。

# before
def update
  # 500行の複雑なコード
end

# after
def update
  if シンプルプラン && 公開後
    simple_plan_after
  elsif シンプルプラン
    simple_plan_before
  else
    full_support
  end
end

2.分割メソッドにはそれぞれ元々の分割前のメソッドの内容をコピペします。

def simple_plan_after
  # 500行の複雑なコード
end

def simple_plan_before
  # 500行の複雑なコード
end

def full_support
  # 500行の複雑なコード
end

3.それぞれの分割メソッドのなかで、プランの条件にそぐわないコードを削除します。

def simple_plan_after
  # フルサポートプランや公開前の場合分けを削除したコード
end

def simple_plan_before
  # フルサポートプランや公開後の場合分けを削除したコード
end

def full_support
  # シンプルプランの場合分けを削除したコード
end

4.同じやり方で、フォームの種類や、Ajaxリクエストかどうか、といったパターン毎にさらにメソッドを分割していきます。

def simple_plan_before_json_basic
  # 読む気になれるシンプルなコード
end
def simple_plan_before_json_reward
  # 読む気になれる短めなコード
end
def simple_plan_before_html_basic
  # 読む気になれる（以下略
end
def simple_plan_before_html_story; end
...
def simple_plan_after_json_basic; end
...
def full_support_html_basic; end
...

分割することで500行1メソッドだったコードが、平均24行程度のメソッドになり、だいぶ可読性が上がりました。

4.メソッドを再構築する

分割したコードを一つ一つ見ていくと、このパターンとこのパターンはまったく同じ処理だった、このパターンは今では想定されてないパターンだった、など色々整理できるところが見えてきました。
あとは適宜テストが通るか確認しながら、同じ処理内容を切り出したり、不要な処理を削除したりしていくだけです。ただし、先ほど書いたテストは正常系だけなので、異常系については別途テストを行う必要があります。