はじめに
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]
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
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]
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
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」という事が積み重なってより良い開発をお互いにできる気がしています。ということもあり「私はこうかなー」というのを書いてみました。
また、スルーしてしまいましたが元記事の主題であるサービスクラスの書き方については、下記に倣っている様子なので「ですよね〜」と思って拝見していました*4。
techracho.bpsinc.jp