🧱

経年的な既存 Rails コードを前に、何からやっていくか

2022/12/03に公開

バックエンドに Rails をどっかり据えた開発へ携わって 1 年ちょっと経ちました。今年 9 月ころまでは Rails を使っての新規開発に参加していましたが、最近は rails new されたのが 2016 年で最初のコミットの Gemfile には gem 'rails', '4.1.4' と刻まれたプロジェクトで開発をしています(もちろん現在はアップデートが進んでいます)。

この記事では Ruby/Rails の経験がそこまで長くない開発者が経年的な既存コードを前にどう理解したか、どう改善・手入れをしているか、その一部について書いていきます。

また、この記事は Classi developers Advent Calendar 2022 の 3 日目の記事です。

経年的既存 Rails コード x 薄いドメイン知識

Rails をバックエンド API に据えた新規開発では、知識の豊かなメンバーがいたことやモブプロという開発スタイルだったことも相まって、いわゆる Rails Way から道を踏み外すことはなく当初の設計意図も把握しているので迷うということはありませんでした。

こういった自分の習熟状態で、経年的な Rails プロジェクトに関わるとこれまでの Rails の知識だけでは当然太刀打ちできません。

コードにはいくつか課題があることは一見してわかりました。たとえばインスタンス変数の状態に左右され、コンテキスト依存が発生しうるような Helper が存在しなおかつビジネスロジックが植え付けられている、などは特に迫力がある課題があると感じました。

また、一部のコードの臭いを嗅ぎ取ることはできたとしても、ドメイン知識が薄いと攻略の手立てはありません。すでに長く関わっているメンバーの手助けは強力なものですが、自らコードを改善し安心して開発を進めるためには、ドメイン知識をコードから汲み取っていく必要があります。

何から理解していくか

ドメイン知識含めてゼロからすべてを把握することは難しいですよね。直近で改修にあたる作業のためテストコードを追加する必要があり、私は既存のテストコードから読むことにしました。

Request Spec

まずは内部の細かい実装よりもワンパスで入力・出力がはっきりしている Request Spec から着目します。すべてのリクエストが網羅されているわけではありませんでしたが、任意のエンドポイントへの Request Spec を読むと以下のようなことが見えてきます。

  1. リクエストを受け付けるために前提としているコンテキストがある
  2. 入力となるパラメータやユーザーのコンテキストがわかる
  3. レスポンスに対しての期待値がわかる

1 は特に入り口として重要で、具体的にはユーザーの認証・認可がある・特定のロールしか閲覧できない・ユーザー種別に対して利用制限がある、などの前提条件をつかむことができるでしょう。RSpec と FactoryBot で記述されていることがほとんどでしょうから、it do;...end 以前でのセットアップ、テストで言うところ Arrange といった部分に着目します。(以下はサンプルコードです)

describe '/api/lectures/:id' type: :request do
  # ここでユーザーの前提条件を作っている
  let(:tenant) { create(:tenant) }
  let(:user) do
    create(:user, tenant: tenant) do |u|
      create(:usage_type, :basic, user: u, year: Time.current.year)
      create(:usage_type, :extra, user: u, year: Time.current.year)
    end
  end
  let(:consumer) { create(consumer:, user: user) }
  # エンドポイントのリソースについての下準備が続いている
  let(:program_category) { create(:program_category) }
  let(:program_course) do
    create(:program_course, program_category_id: program_category.id) do |pc|
      create(:program_usage_type, parent: pc, usage_type: :basic)
      create(:lecture, course: pc)
    end
  end

  before do
    login(consumer)
    # ...ログインの他にもいくつかリクエストの前段で必要な準備をしていたりする
  end

  it do
    # some expectation
  end
end

見えてくる前提条件・ユーザー種別

最初からすべてを網羅するのは難しいので、ユーザーのセットアップがいくつか必要そうだ、といったところをまずとらえます。つまりユーザーの利用条件がドメイン知識になりえるということです。ここでは :basic, :extra といった利用種別がありそうで、ユーザーごと・年ごとにレコードが作られるのでしょうか。そしてユーザー作成のあとに利用者 consumer が作成されていることもわかります。consumer ではないユーザーも存在しそうですね。

このエンドポイントの利用者をベースにしていくつかドメイン知識を得ることができそうです。

何から改善していくか

ドメイン知識を少しずつ知識を増やしつつテストコードを書き本筋である改修作業を進めていると、テストコードの前提条件作成がダルくなってきました。

ボーイスカウトの規則を思い出せ

ボーイスカウトの父であるロバート・ベーデン=パウエルは、スカウトに向けて「世界を見つけたときよりもいい場所にしてほしい」という言葉を遺した。私の「ボーイスカウトの規則」もこの言葉が元になっている。これは「チェックインするコードはチェックアウトしたときよりも美しく」というものだ。どうするのか? コードをチェックインするたびに「親切な行為」をすればいい。
Clean Craftsmanship 規律、基準、倫理

プロダクトコードのリファクタリングを本筋の改修作業と一緒にしてしまうと観点がぶれますが、テストコードの改善は容易なものです。テストを書き続けていると出てくる違和感はそのままにせず、アンクルボブの言葉を胸に親切な行為をテストコードに施していきます。

たとえば先ほどのユーザーのセットアップ、少しだけ冗長ですよね。trait を使って利用種別も合わせて作ってしまえばよいのです。

# Spec ファイル
let(:user) { create(:user, :with_usage_types, usage_types:[:basic, :extra]) }

# ユーザー作成 FactoryBot の別ファイル
FactoryBot.define do
  factory :user do
    # ...
  end

  trait :with_usage_types do
    transient do
      usage_types { [:basic] }
    end
    after(:create) do |user, e|
      e.usage_types.each do |ut|
        create(:usage_type, user: user, usage_type_code: ut, year: Time.current.year)
      end
    end
  end
end

踏み外しすぎた「親切心」はやっかいですが、テストコードの冗長なセットアップから説明十分な知識の入り口を整理しておくということは重要に感じます。

違和感を発散せよ

また、ある程度テストコードを書き続けプロダクトコード自体に理解力がついてくると「なぜこうなっているのか分からないが Rails としては正しいのかもしれない」のような疑念が生まれます。経験則的に嗅ぎ取れる感覚とは別に、Rails のプラクティスとして既存コードがベターか自分で判断がつかないといったことがあるわけです。

そういうときは Slack などで投稿する場合が多いです。識者が集まるかどうかは所属するチームによるかもしれませんが、「そこは斯々然々でな?」と教えてくれるメンバーがいるかもしれません。

たとえば Active Record の scope メソッドについて知識が薄く任意のユースケースで有用か分からないということがありましたが、リアクションをいくつかもらって判断材料にしたということもありました。

まとめ

経年的な既存 Rails コードやドメイン知識がない状況での個人的な取り組み方について書かせてもらいました。

正直どこから理解するかは人それぞれではあるものの、歴史のあるコードは多くがテストコードを完備していないことが多く、テストを補強するなどといったところを取っ掛かりとすると取り組みやすいかもしれません。

「レガシーコード改善ガイド」ではテストがないコードをレガシーコードと定義しています。テストがあればレガシーコードじゃないのかという反論はあるでしょうが、少なくとも開発者の知識が薄く見積もりの難しい場面では強力な知識の外部化になりえるはずです。

レガシーなんじゃない、ただテストがなく、取りかかれないのだ(取りかかるのに不安がある)。さあ明日からも少しずつテストをしっかり書きましょう。テストを書かなければいつまで経っても改善やリファクタリングができないのです。

最初から正しくやろうとするのは大きなプレッシャーになる。これはすべてのことに当てはまる。いつでも元に戻れること、編集できること、クリーンアップできること、いつでもリファクタリングできること。これらを理解していれば、非常に大きな自由が得られる。
レガシーコードからの脱却 ―ソフトウェアの寿命を延ばし価値を高める9つのプラクティス


Classi developers Advent Calendar 12/4 は onigra_ さんです。どうぞお楽しみに。

Discussion