どういうこと？

railsを例にとるが例えばアカウント登録機能を実装するとする。
アカウント登録が行われたらユーザーに完了メールが飛ぶという仕様。

コントローラーに処理を書くのは良くないので、Modelに実装された。 save()が実行されるとメールが飛ぶように実装された。

class Account < ActiveRecord::Base
 
  after_save do
    # メールを送信する
  end
 
end

コントローラーからはこんな感じで使われる。

account = Account.new(data)
account.save()

アカウント登録が実行されると忘れずにメールが飛ぶので便利。

なにが問題か？

仕様の把握が難解になる

新しくアサインされた開発者は

account = Account.new(data)
account.save()

ここを見てメールが送信されるとは読み取れない。
この関数を使った機能を追加して、意図しないタイミングでユーザーへメールが飛んでしまうバグを仕込んでしまう可能性が大きい。

機能追加の難易度が上がる

例えば、メールアドレスを後から登録することも可能とすることになったとする。
現状Account.save()を実行するとメール送信の処理が実行されるのですんなりと対応できない。

こんな感じのif分岐が内部に作られるかもしれない。

class Account < ActiveRecord::Base
 
  after_save do
    unless mail_address.nil? # mail_addressがnullでなけば
        # メール送信
    end
  end
 
end

とりあえず機能追加はできたが、この対応はコードの複雑さを更に上昇させてしまっているので個人的にはナシ。

コードが仕様を表現していない。
「メールアドレスを保持してなければ、メールを送らない」と書かれているだけで、どういうときにこのパターンが発生するのかがコードから読み取れない。

仕様としては
「1:通常のアカウント登録パターン」と
「2:メールアドレスを後から登録するアカウント登録パターン」があって、
2:のパターンの場合、もちろんメール送信はしないという話。

仮に「(1:)のパターンでのアカウント登録処理の実行中なのにメールアドレスが入力されていない状態」でaccount.save()が実行された場合、本来ならエラーにしたりしないといけないが、上記では登録されてしまうというバグにつながるかもしれない。
その可能性にも気が付けない。

なんでこんなことになってるのか？

「単一責任の原則」が守られていない。

account.save()

は名前どおりaccountが保持するデータの永続化処理のみを提供すべき。そこにメール送信処理が混入しているのが問題。

そもそもAccountモデルはaccoutsテーブルを対象に操作を行うのが責務で、それ以外（メール送信処理）を行うのは責務が混在している。

どうすべきか？

Accountモデルはaccoutsテーブルを対象とした操作のみを行うようにする。

class Account < ActiveRecord::Base
 
  after_save do
    # なにもしない。保存だけを行うように変更した。
  end
 
end

Service層を設けてこんな関数を定義する

class AccountService
    def create_account(data)
        account = account.new(data)
        account.save()
        MailService.send(account.mail_address, '登録完了しました')
    end
end

メールアドレスを後から登録することも可能とすることになったら、こんな感じにメソッドを追加。

class AccountService
    def create_account(data)
        account = account.new(data)
        account.save()
        MailService.send(account.mail_address, '登録完了しました')
    end

    def create_account_without_mail_address(data)
        account = account.new(data)
        account.save()
    end
end

それぞれ適切なバリデーションを設定することもやりやすい。

こんな感じで、
「Modelは担当するDBレコードの操作のみを行い、Serviceがそれらを組み合わせてApplicationが提供する機能を実現する」
というのがいいのではないかと思っている。

ActiveRecordのafter_saveとかは便利と思うけど、 after_saveが他のmodelの保存をおこなって、そのmodelのafter_saveでさらに別のmodelの保存を、、とかなるとかなり辛いのでafter_saveとかあまり使いたくない派。

やる気がストロングZERO

やる気のストロングスタイル

ModelにApplicationが提供する機能を実装するとつらくなる