0

SRP violations to recognize — fat models, fat controllers

Intermediate5 min read·eng-01-002
interviewsolid

Concept

Recognizing SRP violations before they become entrenched is a core senior developer skill. The two most notorious hosts for violations in PHP/Laravel codebases are the fat model and the fat controller — and both feel completely natural as they accumulate responsibilities, which is exactly what makes them dangerous.

A fat model grows because Eloquent's ActiveRecord style encourages adding behavior directly onto the model. The User class in a typical Laravel app ends up doing: authentication checks, password reset logic, permission/role lookups, billing status checks, notification sending, API token generation, and profile image handling. Every one of those is a separate stakeholder concern. The model class has dozens of reasons to change.

A fat controller grows because controllers are the "obvious place" to put logic when you are just trying to get something working. The OrderController::store() method ends up validating input, applying discount rules, charging a payment gateway, saving to the database, dispatching emails, and logging to an audit trail — all inline. This is SRP (and many other principles) violated in one method.

The warning signs to look for:

Warning signLikely violation
Controller action method > 30 linesLogic that belongs in a service/action
Model with send*, generate*, check* methodsNon-persistence concerns on a model
Class name ends in Manager, Helper, UtilUsually a dumping ground for unrelated logic
Method comments that say "Step 1... Step 2... Step 3..."The method is doing multiple things
Test class needs 10+ different mock dependenciesThe tested class has too many responsibilities

Code Example

php
<?php
declare(strict_types=1);

// VIOLATION: Fat controller — one action does everything
class OrderController extends Controller
{
    public function store(Request $request): JsonResponse
    {
        // Validation concern
        $validated = $request->validate([
            'items'       => 'required|array',
            'coupon_code' => 'nullable|string',
        ]);

        // Discount concern
        $discount = 0;
        if ($validated['coupon_code']) {
            $coupon = Coupon::where('code', $validated['coupon_code'])->first();
            if ($coupon && $coupon->expires_at > now()) {
                $discount = $coupon->discount_percent;
            }
        }

        // Pricing concern
        $subtotal = collect($validated['items'])->sum(
            fn($item) => $item['price'] * $item['quantity']
        );
        $total = $subtotal * (1 - $discount / 100);

        // Persistence concern
        $order = Order::create([
            'user_id' => auth()->id(),
            'total'   => $total,
            'status'  => 'pending',
        ]);

        // Payment concern
        $charge = Stripe::charge(['amount' => $total * 100, 'currency' => 'usd']);
        $order->update(['stripe_charge_id' => $charge->id, 'status' => 'paid']);

        // Notification concern
        Mail::to(auth()->user())->send(new OrderConfirmation($order));

        // Audit concern
        AuditLog::record('order.placed', ['order_id' => $order->id]);

        return response()->json($order);
    }
}

// CORRECT: Thin controller — delegates to focused classes

// Validation FormRequest
class PlaceOrderRequest extends FormRequest
{
    public function rules(): array
    {
        return [
            'items'       => 'required|array',
            'coupon_code' => 'nullable|string|exists:coupons,code',
        ];
    }
}

// Single-responsibility action
final class PlaceOrderAction
{
    public function __construct(
        private readonly CouponResolver   $coupons,
        private readonly OrderPricer      $pricer,
        private readonly PaymentGateway   $payments,
        private readonly OrderRepository  $orders,
        private readonly AuditLogger      $audit,
    ) {}

    public function execute(int $userId, array $items, ?string $couponCode): Order
    {
        $discount = $couponCode ? $this->coupons->resolveDiscount($couponCode) : 0;
        $total    = $this->pricer->calculate($items, $discount);
        $order    = $this->orders->create($userId, $total);

        $this->payments->charge($order);
        $this->audit->record('order.placed', $order->id);

        return $order;
    }
}

// Thin controller
class OrderController extends Controller
{
    public function __construct(private readonly PlaceOrderAction $placeOrder) {}

    public function store(PlaceOrderRequest $request): JsonResponse
    {
        $order = $this->placeOrder->execute(
            userId:     auth()->id(),
            items:      $request->validated('items'),
            couponCode: $request->validated('coupon_code'),
        );

        return response()->json($order, 201);
    }
}

Interview Q&A

Q: What are the symptoms of a fat model and how would you refactor one?

Fat model symptoms: the model class imports multiple unrelated facades or services at the top; you find methods for sending emails, generating tokens, checking subscriptions, and handling payments all on one Eloquent model. The refactoring approach is extraction: move payment logic into a BillingService, move email-related methods into a Notifier or use Laravel's notification system, extract permission checks into a PermissionResolver or a Policy, and keep the model responsible only for defining the Eloquent relationships and casting rules. Do this incrementally — extract one responsibility at a time, writing tests before and after each extraction.


Q: How do you distinguish "thin controller" from "anemic controller"?

A thin controller correctly delegates business logic but still handles HTTP-specific concerns: extracting validated input, calling an action or service, and mapping the result to an HTTP response (status codes, JSON structure, redirect targets). An anemic controller would be so thin it adds no value at all — just pass-through code that calls a service with no transformation. The controller should own the HTTP layer: content negotiation, error-to-HTTP-status mapping, authorization checks via $this->authorize(). It should not own domain logic (discount calculations, payment processing, email content).


Q: Is a class with 500 lines always a SRP violation?

Not necessarily — but it is a red flag worth investigating. A RecurringBillingCalculator that handles every edge case of billing periods, proration, trial periods, and tax calculations could be 500 lines and still serve one single stakeholder (the billing team). The question is always: how many independent reasons could this class change? If 500 lines all answer to the same concern, it is fine. If even 100 lines contain logic that two different teams would modify independently of each other, it is a violation. Use the "git blame and stakeholder" heuristic: if the last 20 commits to this class come from three different feature branches owned by three different product teams, it is doing too much.