飲み会アレンジサイトGrouperが、同社のエンジニアブログで、規模の大きなRailsアプリをパフォーマンスよくつくるときの工夫を提案をしてますが、それに対してRailsのクリエーターのDHH (Basecamp / 37 Signals) が厳しいコメントを残しています。

1) Grouperの提案

問題意識

Railsは、コードベースが千行を超えると、テストスイートが遅くなりがちで、フレームワークのロードタイムが増える。

よくあるのは、ビジネスロジックのほとんどがActiveRecord /modelsディレクトリの大きなクラスに置かれていること。この場合、各クラスの役割が多すぎるとか、巨大なAPIとか、関連する複雑なオブジェクト群がうまくワークしないと成り立たないようなメソッドがあるというケースがありえる。

失敗する原因はActiveRecordのコールバックにある。他のオブジェクトのステートを変更するクラスにbefore_saveをセットすると、あつかいたいオブジェクトよりも先にそれらの他のオブジェクトが存在しなくてはいけなくなるので問題。テストの際、それらのオブジェクトをDBから取得しなくてはいけないので、テストスイートの実行がうんざりするほど遅くなるか、もしくは、コールバックと長い一連のメソッドコールをスタブしなくてはいけないプロセスで手間がかかるか、いずれかに陥る。

解決策

解決策としては、Interactor , presenter, policyをうまく利用する。今回はInteractorについて以下で説明。

Interactorでは、ActiveRecordクラスはDBに対してのシンプルなインターフェースになり、アプリケーションのメインのユースケースはPOROs (plain-old-Ruby-objects) にあるかたちになる。

controllerはこのようになる。

1  class GroupersController < ApplicationController::Base
2    …
3    def create
4      interactor = ConfirmGrouper.perform(leader: current_member)
5
6      if interactor.success?
7        redirect_to home_path
8      else
9        flash[:error] = interactor.message
10       render :new
11     end
12   end
13   …
14 end

interactorはこのようになる。

1  # Responsible for creating a Grouper, email
2  #
3  #
4  class ConfirmGrouper
5    include Interactor
6
7    def perform
8      grouper = Grouper.new(leader: member)
9      fail!(grouper.errors.full_messages) unless grouper.save
10     send_emails_for(grouper)
11     assign_bar_for(grouper)
12   end
13
14   private
15
16   def send_emails_for(grouper)
17     LeaderMailer.grouper_confirmed(member: grouper.leader.id).deliver
18     WingMailer.grouper_confirmed(wings: grouper.wings.map(&:id)).deliver
19     AdminMailer.grouper_confirmed(grouper: grouper.admin.id).deliver
20   end
21
22   def assign_bar_for(grouper)
23     # Asynchronous job because it’s a little slow
24     AssignBarForGrouper.enqueue(grouper.id)
25   end
26 end

コードがシンプルになり、ActiveRecordモデルは依存がないかたちなのでテストスピードがあがる。テストの際にRailsを読込む必要がなくなるケースも多い。

controllerのspecはこのようになる。

1  describe GroupersController do
2    …
3    describe “#create” do
4      subject { post :create }
5
6      before { ConfirmGrouper.stub(:perform).and_return(interactor) }
7
8      let(:interactor) { double(“Interactor”, success?: success, message: “foo”) }
9
10     context “when the interactor is a success” do
11       let(:success) { true }
12
13       it { should redirect_to home_path }
14     end
15
16     context “when the interactor fails” do
17       let(:success) { false }
18
19       it { should render :new }
20     end
21   end
22 end

ActiveRecordのDBコールがスタブされたので、テストの実行時間は4〜5秒から0.15秒になった。各クラスは一つの役割だけをもつかたちとなり、複数のInteractorをまとめることで複雑なオペレーションも可能になる。また、コードベースの疎結合化 & 再利用化も実現できる。

2) DHHのコメント on Hacker News

このサンプルコードでは説得力がない。テストケースもよくない。うまく抽出してラップすればコードはもっとすっきりできる。この事例なら全部をcontrollerに入れてしまったほうがよい。Interactorの活用は、例えばSignupのモデルのように複数のモデルが調和してつくられるケース、もしくは何かの理由で振る舞いを再利用しなくてはいけないときに向いている。この事例のように、アクション毎にinteractorモデルをつくるのは間違っている。大きなアプリは普通のRailsで作れる。Basecampしかり。

Railsの基本パターンで書き直したサンプルがこちら 。

contorllerでプライベートメソッドを使わずに、メールのグルーピングをPOROで書き直したサンプルがこちら 。

(書き直したサンプルでメールが不達の場合が考慮されてないとの指摘を受けて。) SMTPサーバがダウンしてメールが届かないことを想定しているのであれば、conditionでなくexceptionで扱えばよい。メアドが無効であれば、それを検知できるところで対応すればよい。この時点では遅すぎる。

（Grouperと似た手法で効果がうまくでた経験があるが、今回のブログのサンプルコードは中規模以上のアプリ向けの事例としては不適切という意見に対して。）既に設計 & 開発完了したシステムを書き直すと、確かにコードベースはクリーンになる。しかし、それを「良いアーキテクチャ」と混乱してはいけない。疎結合という言葉で簡単にかたづけてほしくない。もし今回はサンプルコードが悪いだけだと言うなら、良いと思うサンプルコードを送ってほしい。どんなコードでもやりとりして（論破して）みせる。アプリのコントロールが効かなくなる典型的な事例は、不必要な抽象化にある。それを「上級者テク」とは言わない、うまくいかないのはRailsのせいではなくて、おそらくひどいコードのせい。

POROは優れている。37 Signalsのコードでもよく使っている。問題なのは「上級者テク」という間違った観念をもった人々のほうである。アプリのサイズは、膨張する開発者自身のエゴを除けば、問題になることはほとんどない。

TaskRabbit: 複数のRails 4 Enginesで構成したアプリ

#rails #ruby #コーディング #開発スタイル