Rubyのクラスメソッドがリファクタリングに抵抗する理由(翻訳)

概要

原著者の許諾を得て翻訳・公開いたします。

Rubyのクラスメソッドがリファクタリングに抵抗する理由(翻訳)

私の記事『肥大化したActiveRecordモデルをリファクタリングする7つの方法』に対して、「クラスメソッドでできることをなぜわざわざインスタンスでやるんですか?」という質問をよくいただきました。お答えしましょう。要するに以下が理由です。

私がクラスメソッドよりオブジェクトインスタンスを好む理由は、クラスメソッドはリファクタリングに抵抗するからです

詳しく説明するために、データを外部アナリティクスサービスと同期するバックグラウンドジョブを例に取ることにします。次をご覧ください。

class SyncToAnalyticsService
  ConnectionFailure = Class.new(StandardError)

  def self.perform(data)
    data              = data.symbolize_keys
    account           = Account.find(data[:account_id])
    analytics_client  = Analytics::Client.new(CC.config[:analytics_api_key])

    account_attributes = {
      account_id:         account.id,
      account_name:       account.name,
      account_user_count: account.users.count
    }

    account.users.each do |user|
      analytics_client.create_or_update({
        id:             user.id,
        email:          user.email,
        account_admin:  account.administered_by?(user)
      }.merge(account_attributes))
    end
  rescue SocketError => ex
    raise ConnectionFailure.new(ex.message)
  end
end

このジョブは、ユーザーごとに属性のハッシュをHTTP POSTとして繰り返し送信します。SocketErrorraiseされるとSyncToAnalyticsService::ConnectionFailureでラップし、こちら側のエラートラッキングシステムで正しく分類されるようにします。

このSyncToAnalyticsService.performメソッドはなかなか複雑で、責務をいくつも抱えています。単一責任原則(SRP)はちょうどフラクタルのように、より細かいレベルでアプリ全体/モジュール/クラス/メソッドに渡って適用されると考えることができます。SyncToAnalyticsService.performは、そのメソッドのさまざまな動作が必ずしも同じ抽象化レベルにないため、Composed Methodパターンには該当しません。

訳注: Composed Methodパターンについては以下もどうぞ。
「文章のように読めるメソッドを作る」Composed Method パターン

解決法のひとつは、Extract Methodパターンを何回か適用してメソッドを切り出すことです。結果は以下のような感じになります。

class SyncToAnalyticsService
  ConnectionFailure = Class.new(StandardError)

  def self.perform(data)
    data                = data.symbolize_keys
    account             = Account.find(data[:account_id])
    analytics_client    = Analytics::Client.new(CC.config[:analytics_api_key])

    sync_users(analytics_client, account)
  end

  def self.sync_users(analytics_client, account)
    account_attributes  = account_attributes(account)

    account.users.each do |user|
      sync_user(analytics_client, account_attributes, user)
    end
  end

  def self.sync_user(analytics_client, account_attributes, user)
    create_or_update_user(analytics_client, account_attributes, user)
  rescue SocketError => ex
    raise ConnectionFailure.new(ex.message)
  end

  def self.create_or_update_user(analytics_client, account_attributes, user)
    attributes = user_attributes(user, account).merge(account_attributes)
    analytics_client.create_or_update(attributes)
  end

  def self.user_attributes(user, account)
    {
      id:             user.id,
      email:          user.email,
      account_admin:  account.administered_by?(user)
    }
  end

  def self.account_attributes(account)
    {
      account_id:         account.id,
      account_name:       account.name,
      account_user_count: account.users.count
    }
  end
end

元のコードと比べれば少しはマシになりましたが、どうも今ひとつです。これではオブジェクト指向にならず、手続き型プログラミングと関数型プログラミングの不気味な折衷案がオブジェクトベースの世界で立ち往生しているような感じです。さらに、切り出したメソッドはどれもクラスレベルなので、private宣言も簡単にはできそうにありません(おそらくclass << selfのような見苦しい方法に切り替えざるを得ないでしょう)。

私だったら、SyncToAnalyticsServiceの元の実装を目にしたとしてもこんな形でリファクタリングを完了する気にはとてもならないでしょう。代わりに、次のようにリファクタリングを始めるでしょう。

class SyncToAnalyticsService
  ConnectionFailure = Class.new(StandardError)

  def self.perform(data)
    new(data).perform
  end

  def initialize(data)
    @data = data.symbolize_keys
  end

  def perform
    account           = Account.find(@data[:account_id])
    analytics_client  = Analytics::Client.new(CC.config[:analytics_api_key])

    account_attributes = {
      account_id:         account.id,
      account_name:       account.name,
      account_user_count: account.users.count
    }

    account.users.each do |user|
      analytics_client.create_or_update({
        id:             user.id,
        email:          user.email,
        account_admin:  account.administered_by?(user)
      }.merge(account_attributes))
    end
  rescue SocketError => ex
    raise ConnectionFailure.new(ex.message)
  end
end

元のコードとほとんど変わらないように見えますが、今度は機能をクラスメソッドではなくインスタンスメソッドにしている点が異なります。ここに再びExtract Methodパターンを適用すると次のような感じになります。

class SyncToAnalyticsService
  ConnectionFailure = Class.new(StandardError)

  def self.perform(data)
    new(data).perform
  end

  def initialize(data)
    @data = data.symbolize_keys
  end

  def perform
    account.users.each do |user|
      create_or_update_user(user)
    end
  rescue SocketError => ex
    raise ConnectionFailure.new(ex.message)
  end

private

  def create_or_update_user(user)
    attributes = user_attributes(user).merge(account_attributes)
    analytics_client.create_or_update(attributes)
  end

  def user_attributes(user)
    {
      id:             user.id,
      email:          user.email,
      account_admin:  account.administered_by?(user)
    }
  end

  def account_attributes
    @account_attributes ||= {
      account_id:         account.id,
      account_name:       account.name,
      account_user_count: account.users.count
    }
  end

  def analytics_client
    @analytics_client ||= Analytics::Client.new(CC.config[:analytics_api_key])
  end

  def account
    @account ||= Account.find(@data[:account_id])
  end
end

操作を完了するために直接変数(immediate variable)を引き回さなければならないクラスメソッドを追加するのではなく、結果をメモ化する#account_attributesのようなメソッドを追加しました。これは私のお気に入りの手法です。メソッドを分割するときに、メモ化したアクセサとして直接変数を切り出す方法は、私の大好きなリファクタリングです。このクラスは開始時にいかなるステートも持ちませんが、分割されているおかげでそこに何か追加するのも簡単でした

今度の結果は私にとってずっと明確になりました。こういうリファクタリングは完全勝利の気分になれます。ステートとロジックが1つのオブジェクトにきっちりカプセル化されていますし、(与えられた)オブジェクトの作成が操作の呼び出し(のタイミング)と分離されているので、テストも簡単です。しかも、AccountAnalytics::Clientといった変数をどこにも引き回していません。

さらに、このロジックを用いるどのコード片も(グローバルな)クラス名と結合していません。これらを新しいクラスで差し替えるのは大変ですが、新しいインスタンスでなら簡単に差し替えられます。このおかげで、追加の動作をコンポジションで組み立てやすくなります。変更のたびにクラスを再オープンして拡張する必要はありません。

リファクタリングメモ: 私なら、上の最終的なソースの状態でクラスの実装をやめておくでしょう。しかしロジックがさらに複雑になった場合、このジョブは単一ユーザーを同期するための別のクラスを欲しがるでしょう。

さて、このことは本記事で最初に述べた前提とどんな関係があるのでしょうか?クラスメソッドを分割するとコードが見苦しくなるので、私がクラスメソッドをリファクタリングする機会はあまりないでしょう。最初からインスタンス形式で書いておけばリファクタリングの選択肢も明確になりますし、必要な対策を取るときの抵抗も減らせます。この効果は私自身のコーディングでも何度となく体験していますし、ここ数年のさまざまなRubyチームを外から眺めていてもやはりそうです。

想定反論

YAGNIではないか?

YAGNI: You Aren’t Going To Need It(後で必要になるかもしれないという理由でやるべきではない)

YAGNIが重要な法則であることはもちろんですが、ここで適用するのは筋違いです。これらのクラスをエディタで開いてみれば、そうでないクラスと比べて複雑さはさほど変わりません。「このオブジェクトはYAGNIだ」という指摘は、「タブ文字1つではなくスペース2文字にするのはYAGNIだ」という指摘と大して変わりません。違いはスタイル上のものでしかありません。オブジェクト指向設計にYAGNIを適用する意味があるのは、わかりやすさに違いが生じる場合だけです(使うクラスが1つなのか2つなのか、など)。

オブジェクトが1つ余分になる

インスタンス形式だとオブジェクトが1つ余分に作成されるという根拠で反対する人もいます。オブジェクトが作成されることについてはそのとおりですが、実用上は何の影響もありません。Railsのリクエストやバックグラウンドジョブではおびただしい数のRubyオブジェクトが作成されます。オブジェクト作成を最適化してRubyのガベージコレクタの負担を軽減するのは正統な手法ですが、それが意味を持つ場合に行うべきです。そしてそれは測定によってのみ確認できます。そのインスタンスの変種がたったひとつの追加オブジェクトを作成しただけでシステムのパフォーマンスに大きく影響するとは考えられません(データ付きの反例をお持ちの方がいたらぜひ拝見したいと思います)。

呼び出しが面倒

最後の想定反論は、クラスメソッドの方が入力文字数が少なくて済むというものです。

Job.perform(args)
#  どっちがいいか?
Job.new(args).perform

入力文字数が少ないのはそのとおりです。私なら、オブジェクトをビルドする手頃なクラスメソッドを1つこしらえ、それに委譲して済ませるでしょう。実際、これは私が認める数少ないクラスメソッドの使い方の1つです。こうやって自分で作って自分でおいしくいただく分には構いません。

まとめ

常にオブジェクトのインスタンスから出発しましょう。ステートや複数のメソッドが当分使われないとしても、そうすべきです。いずれ変更のときが来れば、あなたや同僚がリファクタリングしたくなります。コードが今後も決して変更されないのであれば、クラスメソッド方式かインスタンスかという違いは無視して構わない性質のものであり、今後そのコードが改悪されることもまずありません。

私がコードでクラスメソッドを使って幸せになれたケースはほぼ皆無です。あるとすれば、インスタンスの初期化手順をラップして一発で呼び出せるお便利メソッドか、連携する他のオブジェクトからオブジェクトをより簡単に構成できる徹底的にシンプルなファクトリーメソッド(多くとも2行以内)ぐらいです。

皆さまはどう思われますか?どちらがお好みですか?そしてその理由は?私が見落としたメリットやデメリットはありますでしょうか?ぜひコメント欄でお知らせください。

追伸: こうした問題に興味をお持ちで、他の記事を読んでみたい方は、元記事末尾のフォームでCodeClimateニュースレターをぜひご購読ください。Rubyに特化したリファクタリングやオブジェクト設計に関する話題を月に一度メール配信いたします。

詳しく知りたい方へ

本記事をレビューしてくれたDoug Cole、Don Morrison、Josh Susserに感謝いたします。

関連記事

肥大化したActiveRecordモデルをリファクタリングする7つの方法(翻訳)

Rails: モデルのクエリをカプセル化する2つの方法(翻訳)

Rubyのクラスメソッドをclass << selfで定義している理由(翻訳)

Ruby on RailsによるWEBシステム開発、Android/iPhoneアプリ開発、電子書籍配信のことならお任せください この記事を書いた人と働こう! Ruby on Rails の開発なら実績豊富なBPS

この記事の著者

hachi8833

Twitter: @hachi8833、GitHub: @hachi8833 コボラー、ITコンサル、ローカライズ業界、Rails開発を経てTechRachoの編集・記事作成を担当。 これまでにRuby on Rails チュートリアル第2版の半分ほど、Railsガイドの初期翻訳ではほぼすべてを翻訳。その後も折に触れてそれぞれ一部を翻訳。 かと思うと、正規表現の粋を尽くした日本語エラーチェックサービス enno.jpを運営。 実は最近Go言語が好き。 仕事に関係ないすっとこブログ「あけてくれ」は2000年頃から多少の中断をはさんで継続、現在はnote.muに移転。

hachi8833の書いた記事

週刊Railsウォッチ

インフラ

BigBinary記事より

ActiveSupport探訪シリーズ