島までは遠い 〜サークルアラウンド株式会社代表佐藤のブログ〜

佐藤正志@サークルアラウンド株式会社のことが少しわかる場所。プログラマーを育てるトレーナーとして、現役のソフトウェア技術者として、経営者の端くれとして、想うことをつづる。

「モデルの中からPOROを呼び出すのはどの程度認められるのか」気になります

はじめに

tech.medpeer.co.jp

この記事読んで思ったことを誰にともなくコミュニケーションしてみたくなったので書いてみます。当該の記事はリファクタリングの考え方としてとても好きです。その一方で過去私がとても苦しんだ事と同じ課題(もはやトラウマみたいな)を感じたのでアウトプットしてみたくなりました。

私の感覚

長くなるので後の方に色々細々した気持ちを書きますが、今の私はモデルのリファクタリングについて下記の記事をかなり重要視しています。その観点で当該のサンプルをリファクタリングするとどうなるかを書いてみると良いと思いました。

techracho.bpsinc.jp

上記記事に従うと、今回の場合「処理が複雑になった(という前提ですよね?)」と考えてServiceを採用するか、「あるユースケースにおける保存処理に介入する」と考えてDecoratorを採用するのが妥当な気がします。というわけで二つ書きました。そして私ならこれらの二つのどちらかがシックリくると考えて、元記事でのリファクタリング結果には意見を提示すると思います*1

Serviceを採用した場合

class Message < ApplicationRecord
  has_many :mentions
  belongs_to :creator, class_name: 'User'
  belongs_to :channel

  def here?; end # 省略
  def channel?; end #省略
end

class MentionNotifier
  def self.call(message)
    self.new(message).call
  end

  def initialize(message)
    @message = message
  end

  def call
    ActiveRecord::Base.transaction do
      return false unless @message.valid?
      @message.save!
      create_here_menthin if @message.here?
      create_channel_mention if @message.channel?
    end
    @message
  end

  private

  def create_here_mention
    members = @message.channel.members.active - [@message.creator] # これはMessageのメソッド化しそうですが、本題とはずれるのでこのまま
    create_mentions(members)
  end

  def create_channel_mention
    members = @message.channel.members - [@message.creator] 
    create_mentions(members)
  end

  def create_mentions(members)
    members.each do |member|
      @message.mentions.create!(to: member, chennel: @message.channel)
    end
  end
end

# 使い方(create アクションの中の想定)
message = Message.new(params)
MentionNotifier.call(message)

Decoratorとしての切り出し

class Message < ApplicationRecord
  has_many :mentions
  belongs_to :creator, class_name: 'User'
  belongs_to :channel

  def here?; end # 省略
  def channel?; end #省略
end

class MentionNotifier
  attr_accessor :message

  def initialize(message)
    @message = message
  end

  def save
    ActiveRecord::Base.transaction do
      return false unless @message.valid?
      @message.save!
      create_here_menthin if @message.here?
      create_channel_mention if @message.channel?
    end
    @message
  end

  private

  def create_here_mention
    members = @message.channel.members.active - [@message.creator] # これはMessageのメソッド化しそうですが、本題とはずれるのでこのまま
    create_mentions(members)
  end

  def create_channel_mention
    members = @message.channel.members - [@message.creator] 
    create_mentions(members)
  end

  def create_mentions(members)
    members.each do |member|
      @message.mentions.create!(to: member, chennel: @message.channel)
    end
  end
end

# 使い方(create アクションの中の想定)
message = MentionNotifier.new(Message.new(params))
message.save

モデルの中からPOROを呼び出すのはどの程度認められるのか

「今回選ばれた例が通知をする処理だから上記のような形の方がシックリくるのではないか」

と思われてしまうと本題と外れてしまうので章立てしました。私がRailsでコードを書き始めて当初悩んだことの一つがこの「モデルの中からPOROをバンバン呼び出すのはアリなのか」ということなんです。

私にとってJavaの業務系でのコードのスタイルがキャリアの初期にあった為か、POJO を中心とする書き方が身についており、肥大化するモデルの処理や、通知などの内容は元記事のコードのようにモデルの中からPOROを呼びたくなっていたんです。ただ、ロジック層に入ってPOROから(ActiveRecordに強く縛られた)モデルを呼び、さらその中で業務に関連したPOROを...と呼ばれるのが不自然に感じられていたんですね。

そういう中で出会ったのが先に紹介した肥大化したActiveRecordモデルをリファクタリングする7つの方法(翻訳)の記事です。この記事は悩んでいた私に一つの考え方を指し示してくれました。「基本的にModelを渡す形で良い」という事です。この記事に出会ってからは私が抱えていた悩みはスッと晴れました。手本とするのに適切であると思っています*2

ただ物事には例外がつき物なので「〇〇なケースでは認めて良いのでは?」というのもありそうです。私だとSTIしている複数のクラスからそれぞれの型に関連したPOROを生成させるメソッドを作りたいと思うことはよくあります。これは認めても良いかもと感じています。

そんな私が今回の記事を読んだ時に「あれ?私の過去の悩みって結構無駄だった?」なんて変な汗が出てしまったのでした(笑) というわけで、この話題について多くの方々がどのように考えていらっしゃるのか大変興味があります。もしも何か良い指針があれば私もそれを知りたいのです。

おしまいに

特にRailsでは慣習や大勢のパターンに合わせた書き方を強く推奨することで、多くのプログラマーがプロジェクトを移動しても活躍しやすいという土壌があると思っています。少し前にも「Serviceクラスをどのように書くべきなのか」などの話題があったりしましたね。こうやって「xxxなケースはyyyな解法が概ね良い*3」という事が積み重なってより良い開発をお互いにできる気がしています。ということもあり「私はこうかなー」というのを書いてみました。

また、スルーしてしまいましたが元記事の主題であるサービスクラスの書き方については、下記に倣っている様子なので「ですよね〜」と思って拝見していました*4techracho.bpsinc.jp

*1:開発の仕事の中でレビューする際に、私個人の哲学を押し付けて良い場合は限られていると考えています。だからこういうシーンでは大抵自分の意見を提示してみてコミュニケーションを取ります

*2:自分ところのトレーニングでもこの内容はよく紹介しています

*3:最高で無くても良く、一般的に皆がある程度納得できるものであれば良いというイメージです

*4:私だとメソッド名固定に強いこだわりは無かったりしますが