Tech Racho エンジニアの「?」を「!」に。
  • Ruby / Rails関連

Railsアンチパターン: Decoratorの肥大化(翻訳)

概要

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

パターン名は英語表記としています。

  • 2018/03/06: 初版公開
  • 2023/05/25: 更新

Railsアンチパターン: Decoratorの肥大化(翻訳)

RailsでDecoratorを用いるとさまざまなメリットが得られます。モデルはスリムになり、ビューもすっきりし、手続き臭い従来のビューヘルパーが過去のものになります。

RailsプロジェクトにDecoratorパターンを適用するとき、ともするとモデルとDecoratorを1対1で対応させたい誘惑にかられます。たとえばArticleのプレゼンテーションロジックはすべてArticleDecoratorに置く、という具合です。これはDecoratorが小さいうちは正当なアプローチでしょう。

しかしこのDecoratorに要件が時とともに追加されていくと、さらに多くのメソッドが集結し、責務は増加し、いつしかDecoratorはゆっくりと肥大化していきます。ぎっしり詰まったメソッド群の中からいずれバグが顔を出すでしょう。

単一のDecoratorに何もかも詰め込んでいては、Railsのビューヘルパーを使っていたときと大差ありません。Decoratorを最大限に活用したいのであれば、オブジェクト指向プログラミングの原則をDecoratorにも適用することが重要です。

本記事では、これまで私がDecoratorでよく目にするコードの臭いをいくつかご紹介します。

🔗 変更の分散(divergent changes)

大規模なDecoratorでは機能が広範囲に渡っていることがよくあります。要件が変更されると、1つのDecoratorがいくつもの異なる要件に合わせようとしてたびたび自らを変更していたりします。これが「変更の分散」と呼ばれるコードの臭いです。

この臭いは、単一責任の原則に違反していることを示しています。ただし、プレゼンテーションロジックの面倒を見ることは単一責任にはカウントしません;-)。機能の1つの部分について責任を持つメソッド群が互いに強く関連しているのであれば、おそらくそれらを独自のクラスに切り出すべきでしょう。

🔗 機能の羨望(feature envy)

不要なロジックをビューから切り出すとき、目についた最寄りのDecoratorに置いて済ませていることがよくあります。これをやると、Decoratorが他のオブジェクトに越境し始めるようになり、しまいには自分自身ではなく他のオブジェクトに関心を持つようになってしまいます。

そうなると、オブジェクト間に多数の癒着が発生し、リファクタリングが困難になります。この種の機能は、もっと適切なDecoratorに移す必要があります。

🔗 臭いを放つDecorator

以下は、Git commitのプレゼンテーションロジックを扱うDecoratorです。簡単にするため、コードのかなりの部分を省略してあります。私はDraper gemを用いていますが、他のgemや自作のDecoratorを使うことも可能です。

class CommitDecorator < Draper::Decorator
  delegate_all

  def author_link
    h.link_to(author.name, h.profile_path(author.username))
  end

  def parent_link
    h.link_to(parent.truncated_sha, h.project_commit_path(project, parent.sha))
  end

  def diff_stats
    h.t('commits.show.diff_stats_html',
        changed: diffs.count,
        additions: diffs.sum(&:additions),
        deletions: diffs.sum(&:deletions))
  end

  def file_changes
    diffs.map do |diff|
      DiffLine.new(
        status_class_for(diff), diff.path, diff.additions, diff.deletions)
    end
  end

  private

  DiffLine = Struct.new(:status_class, :path, :additions, :deletions)

  def status_class_for(diff)
    if diff.deleted?
      'deletion'
    elsif diff.added?
      'addition'
    else
      'change'
    end
  end
end

commitのコントローラは次のような感じだとします。

class CommitsController < ApplicationController
  decorates_assigned :commits, :commit

  def index
    @commits = find_project.commits
  end

  def show
    @commit = find_project.find_commit_by_sha(params[:sha])
  end

  private

  def find_project
    Project.find(params[:project_id])
  end
end

以下の2つのシナリオが想定されています。

  • プロジェクトのcommitリストが表示される
  • 変更されたファイルのcommitが1件表示される

🔗 問題点

このDecoratorには、先にご紹介した2種類の「コードの臭い」が両方とも出現しています。

わずかな情報しかないcommitサマリーを表示するだけのために、機能を満載したDecoratorを使うのは、いかにもイケてない感じがします。

このDecoratorは他のオブジェクトの詳細部分にかなりちょっかいを出しています。特に、diff内部の詳細と非常に強く癒着しています。

🔗 ちょっかいを出さないようにする

特にヤバイのは、ファイル変更情報を集めるためにdiffの詳細に立ち入っている点です。取り急ぎこの部分をDiffDecoratorに切り出すリファクタリングを行いましょう。

class DiffDecorator < Draper::Decorator
  delegate_all

  def status_class
    if deleted?
      'deletion'
    elsif added?
      'addition'
    else
      'change'
    end
  end
end

これだけで、以下のCommitDecoratorがスッキリと変わったことにご注目ください。file_changesはデコレーションされたdiffと同義になりました。厳密には必要ではありませんが、ここではalias_methodを用いてインターフェイスを揃えています。

class CommitDecorator < Draper::Decorator
  delegate_all
  decorates_association :diffs

  alias_method :file_changes, :diffs

  def author_link
    h.link_to(author.name, h.profile_path(author.username))
  end

  def parent_link
    h.link_to(parent.truncated_sha, h.project_commit_path(project, parent.sha))
  end

  def diff_stats
    h.t('commits.show.diff_stats_html',
        changed: diffs.count,
        additions: diffs.sum(&:additions),
        deletions: diffs.sum(&:deletions))
  end
end

authorへのリンクと親commitは、このDecorator内でもうまく切り離されました。親自身もcommitオブジェクトなので、CommitDecoratorへのlinkメソッドが暗に追加されます。

class AuthorDecorator < Draper::Decorator
  delegate_all

  def link
    h.link_to(name, h.profile_path(username))
  end
end

class CommitDecorator < Draper::Decorator
  delegate_all
  decorates_association :diffs
  decorates_association :author
  decorates_association :parent

  alias_method :file_changes, :diffs

  delegate :link, to: :author, prefix: true
  delegate :link, to: :parent, prefix: true

  def link
    h.link_to(truncated_sha, h.project_commit_path(project, sha))
  end

  def diff_stats
    h.t('commits.show.diff_stats_html',
        changed: diffs.count,
        additions: diffs.sum(&:additions),
        deletions: diffs.sum(&:deletions))
  end
end

これでCommitDecoratorは、author_linkparent_linkをいくつか委譲するだけとなりました。

🔗 「変更の分散」に対処する

私は、あるモデルのデフォルト(ベース)Decorator的なものを用意する手法を常に好んでいます。このDecoratorでは、別のコンテキストでよく用いられるメソッドを定義します。これはCommitDecoratorと呼ぶのに相応しいでしょう。commit.decorateが呼び出されれば、デフォルトDecoratorによって常に自動でデコレーションされます。

デコレーションされる機能によっては、デコレーションを必要とするコンテキストが1つしかないこともあります。私は、その種の機能を独自のDecoratorクラスに配置して明示的に利用し、共通機能の中に埋もれないようにしておくのが好みです。

たとえば、commitサマリーをデフォルトDecoratorでデコレーションできれば十分だとしましょう。しかし詳細なcommitは明らかに独自のデコレータを必要としています。

module Commits
  class DetailedCommitDecorator < Draper::Decorator
    delegate_all

    def initialize(*args)
      super(CommitDecorator.new(*args))
    end

    def diff_stats
      h.t('commits.show.diff_stats_html',
          changed: diffs.count,
          additions: diffs.sum(&:additions),
          deletions: diffs.sum(&:deletions))
    end
  end
end

私はいつも、この種のコンテキスト固有なDecoratorを、デコレーションするクラスで名前空間化するようにしています。これならDecoratorの数が増えても概要を十分把握できます。

継承でDecoratorを用いる方法は、Decoratorの本質に逆らっているように感じられますし、親クラスのメソッドをオーバーライドするときに混乱しがちなので好きではありません。継承されたメソッドをオーバーライドするのであれば、モデルで定義済みのメソッドをオーバーライドする方法ではない、別のアプローチが必要になるかもしれません。

そういうわけで、私は最初に元のcommitオブジェクトをCommitDecoratorでラップしています。これはCommits::DetailedCommitDecorator.new(CommitDecorator.new(commit))みたいな方法と同等とも言えますが、両方のDecoratorにその都度適用しなければならないことを気にしたくありません。私のDecoratorはどれもdelegate_allを使っていることにご注意ください。このdelegate_allは、Decoratorが知らないメソッドから、デコレーションされるオブジェクトへ委譲するためのものです。

詳細なcommitの機能を切り出したことで、CommitDecoratorがこんなにシンプルになりました。

class CommitDecorator < Draper::Decorator
  delegate_all
  decorates_association :diffs
  decorates_association :author
  decorates_association :parent

  alias_method :file_changes, :diffs

  delegate :link, to: :author, prefix: true
  delegate :link, to: :parent, prefix: true

  def link
    h.link_to(truncated_sha, h.project_commit_path(project, sha))
  end
end

最後の仕上げに、コントローラで適切なDecoratorを適用します。

class CommitsController < ApplicationController
  decorates_assigned :commits
  decorates_assigned :commit, with: Commits::DetailedCommitDecorator

  def index
    @commits = find_project.commits
  end

  def show
    @commit = find_project.find_commit_by_sha(params[:sha])
  end

  private

  def find_project
    Project.find(params[:project_id])
  end
end

🔗 まとめ

Decoratorを従来のビューヘルパー代わりに用いることは避けましょう。Decoratorからコードの臭いが立ち昇ったら適切に処理し、メソッドや責務が複雑にならないようにしましょう。Decoratorクラスを新しく導入することを恐れてはいけませんが、Decoratorが存在してもよい正当な理由がある場合にのみ行いましょう。

皆さんのDecoratorからはどんな臭いがしますか?ぜひ原文末尾のコメント欄までお寄せください。

関連記事

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

Rails: テストのリファクタリングでアプリ設計を改良する(翻訳)

[保存版]人間が読んで理解できるデザインパターン解説#2: 構造系(翻訳)


CONTACT

TechRachoでは、パートナーシップをご検討いただける方からの
ご連絡をお待ちしております。ぜひお気軽にご意見・ご相談ください。