DEV Community

Cover image for Fat controllers, fat models, and the layer MVC forgot
Hassan Farooq
Hassan Farooq

Posted on

Fat controllers, fat models, and the layer MVC forgot

MVC is the first thing anyone learns about Rails and the last thing people actually get right. The pattern itself is simple. Three layers, each with one job. Where it falls apart is the stuff that doesn't fit cleanly into any of the three, and that's exactly what interviewers poke at.

I'll walk through the three layers fast, then spend the real time on the question that separates a junior answer from a mid-level one: where do you put the logic for creating an order, charging Stripe, and sending a confirmation email?

The three layers

The model owns your data and the rules attached to it. With Active Record that means associations, validations, scopes, and small bits of behavior that belong to the record itself, like order.total or user.full_name. If the question is "what is true about this record," the answer lives in the model.

The controller handles one request. It reads params, checks who you are and what you're allowed to do, calls a model or a service, and picks a response. That's it. A controller should read like a list of instructions a manager gives, not like the work itself. People call a bloated one a "fat controller," and it's a smell.

The view renders what the user sees. An ERB template, a Turbo Stream, a JSON response from a serializer. Presentation only. The view should never decide business rules. It just shows whatever the controller handed it.

A request walks through them in order. The router matches the URL to a controller action, the controller calls a model or service, the result goes to a view, and the response heads back to the browser.

The part nobody teaches

Here's the trap. Rails makes it so easy to put code in the model or the controller that people put everything there. Both rot the same way.

Stuff everything in the model and you get a 600-line User class that sends emails, talks to Stripe, and syncs a CRM, all triggered by callbacks you forgot existed. Stuff everything in the controller and you get a create action that's forty lines of business logic with rendering buried at the bottom.

The fix is to notice when an operation isn't really model data and isn't really request handling. When something coordinates several models, calls an external service, or kicks off side effects, it's a workflow. Workflows go in a service object. That's the missing letter MVC never gave you.

The order example

So you need to create an order, charge the customer through Stripe, and email them a confirmation. Three things, and they each belong in a different place.

Start with the controller. It should stay boring:

class OrdersController < ApplicationController
  def create
    result = OrderCreator.call(user: current_user, params: order_params)

    if result.success?
      render json: result.order, status: :created
    else
      render json: { errors: result.errors }, status: :unprocessable_entity
    end
  end

  private

  def order_params
    params.require(:order).permit(:product_id, :quantity)
  end
end
Enter fullscreen mode Exit fullscreen mode

Notice what's missing. No Stripe, no mailer, no transaction. The controller checks the user (through current_user), permits params, hands off to a service, and turns the result into a response. If you came back in a year and the only change was the JSON shape, this is the only file you'd touch.

Now the service, where the actual workflow lives:

class OrderCreator
  def self.call(user:, params:)
    new(user, params).call
  end

  def initialize(user, params)
    @user = user
    @params = params
  end

  def call
    order = nil

    Order.transaction do
      order = @user.orders.create!(@params)
      order.build_line_items_from_cart!
    end

    charge_customer(order)
    OrderMailer.confirmation(order.id).deliver_later

    Result.success(order)
  rescue ActiveRecord::RecordInvalid => e
    Result.failure(e.record.errors.full_messages)
  rescue Stripe::CardError => e
    order&.update(status: "payment_failed")
    Result.failure([e.message])
  end

  private

  def charge_customer(order)
    Stripe::Charge.create(
      amount: order.total_cents,
      currency: "usd",
      customer: @user.stripe_customer_id,
      idempotency_key: "order-#{order.id}"
    )
  end
end
Enter fullscreen mode Exit fullscreen mode

Three pieces, three homes:

The order and its line items are database writes that have to succeed or fail together. You don't want an order with no line items, or line items pointing at an order that never saved. So they go inside a transaction. I use create! with the bang so a failure raises and the transaction rolls back instead of silently saving half the data. The order's own rules, like validates :quantity, numericality: { greater_than: 0 }, still live on the Order model where they belong.

The Stripe charge sits in the service but outside the transaction, and that placement is deliberate. A transaction holds a database connection and often locks rows. If you wrap a network call to Stripe inside it, you're holding that lock for as long as Stripe takes to answer, which could be seconds. Worse, a charge can't be rolled back. If the DB transaction fails after the card was charged, your database and Stripe now disagree and the customer is out real money. So I charge after the transaction commits, with an idempotency key so a retry never double-charges.

The email goes to a background job with deliver_later. The customer shouldn't sit and wait for an SMTP server, and a flaky mail provider shouldn't blow up an otherwise good order. Note I pass order.id, not the order object, because the job runs later and should reload fresh data.

The one-liner

If an interviewer asks and you've got ten seconds: controllers coordinate the request, models hold data and record-level rules, views render, and any multi-step workflow with side effects goes in a service object. The order example is the proof. Creation in a transaction, the external charge outside it, the email in a job, and a controller thin enough to read in one breath.

Top comments (0)