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

概要

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

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

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の概要を表示したいだけであれば、1つの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: 構造系(翻訳)

デザインも頼めるシステム開発会社をお探しならBPS株式会社までどうぞ 開発エンジニア積極採用中です! Ruby on Rails の開発なら実績豊富なBPS

この記事の著者

hachi8833

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

hachi8833の書いた記事

BPSアドベントカレンダー

週刊Railsウォッチ

インフラ

ActiveSupport探訪シリーズ