⚠️ Common Mistakes & Anti-Patterns

Learn from common mistakes and anti-patterns in Laravel development. Each section shows the problem, explains why it's bad, and provides the correct solution.

🎮 Fat Controllers

❌ Problem

php

<?php

class OrderController extends Controller
{
    public function store(Request $request)
    {
        // Validation in controller
        $request->validate([
            'items' => 'required|array',
            'coupon_code' => 'nullable|string',
        ]);

        // Business logic in controller
        $subtotal = 0;
        foreach ($request->items as $item) {
            $product = Product::find($item['product_id']);
            $subtotal += $product->price * $item['quantity'];
        }

        // Discount calculation in controller
        $discount = 0;
        if ($request->coupon_code) {
            $coupon = Coupon::where('code', $request->coupon_code)->first();
            if ($coupon && $coupon->is_valid) {
                $discount = $subtotal * ($coupon->percentage / 100);
            }
        }

        // Tax calculation in controller
        $tax = ($subtotal - $discount) * 0.15;
        $total = $subtotal - $discount + $tax;

        // Database operations in controller
        $order = Order::create([
            'user_id' => auth()->id(),
            'subtotal' => $subtotal,
            'discount' => $discount,
            'tax' => $tax,
            'total' => $total,
        ]);

        // Attaching relationships in controller
        foreach ($request->items as $item) {
            $order->items()->create([
                'product_id' => $item['product_id'],
                'quantity' => $item['quantity'],
                'price' => Product::find($item['product_id'])->price,
            ]);
        }

        // Email sending in controller
        Mail::to($order->user)->send(new OrderConfirmation($order));

        // Payment processing in controller
        $payment = new PaymentGateway();
        $payment->charge($order->user->payment_method, $total);

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

⚠️ Why It's Bad

Difficult to test - Too many responsibilities
Hard to maintain - Logic scattered everywhere
Not reusable - Can't use logic elsewhere
Violates SRP - Controller does everything
Difficult to debug - Too much happening

✅ Solution

php

<?php

// Controller - thin, handles HTTP only
class OrderController extends Controller
{
    public function __construct(
        private OrderService $orderService
    ) {}

    public function store(CreateOrderRequest $request): JsonResponse
    {
        $order = $this->orderService->createOrder(
            auth()->user(),
            $request->validated()
        );

        return response()->json([
            'data' => new OrderResource($order),
            'message' => 'Order created successfully',
        ], 201);
    }
}

// Service - handles business logic
class OrderService
{
    public function __construct(
        private OrderRepository $orderRepository,
        private OrderCalculator $orderCalculator,
        private PaymentService $paymentService,
        private EmailService $emailService
    ) {}

    public function createOrder(User $user, array $data): Order
    {
        DB::beginTransaction();

        try {
            $order = $this->orderRepository->createOrder($user, $data);
            $this->orderCalculator->calculateTotals($order);
            $this->paymentService->processPayment($order);
            $this->emailService->sendOrderConfirmation($order);

            DB::commit();
            return $order;

        } catch (\Exception $e) {
            DB::rollBack();
            throw $e;
        }
    }
}

🔄 N+1 Query Problem

❌ Problem

php

<?php

// Loads users
$users = User::all(); // 1 query

// N+1 problem - queries posts for each user
foreach ($users as $user) {
    echo $user->name;
    // This triggers a separate query for each user
    foreach ($user->posts as $post) {  // N queries
        echo $post->title;
    }
}

// Results in 1 + N queries (if 100 users = 101 queries!)

⚠️ Why It's Bad

Performance killer - Can generate hundreds of queries
Slow response times - Users experience delays
Database overload - Unnecessary load on database
Scalability issues - Gets worse with more data

✅ Solution

php

<?php

// Eager load relationships - only 2 queries total
$users = User::with('posts')->get();

foreach ($users as $user) {
    echo $user->name;
    foreach ($user->posts as $post) {
        echo $post->title;
    }
}

// Nested relationships
$users = User::with(['posts.comments', 'profile'])->get();

// Conditional eager loading
$users = User::with(['posts' => function ($query) {
    $query->where('is_published', true)->latest();
}])->get();

// Select specific columns
$users = User::with(['posts:id,user_id,title'])->get();

🚫 Missing Type Declarations

❌ Problem

php

<?php

class UserService
{
    private $userRepository;

    public function __construct($userRepository)
    {
        $this->userRepository = $userRepository;
    }

    public function createUser($data)
    {
        $user = $this->userRepository->create($data);
        return $user;
    }

    public function getUserById($id)
    {
        return $this->userRepository->find($id);
    }
}

⚠️ Why It's Bad

Type safety lost - Can pass wrong types
IDE support reduced - No autocompletion
Runtime errors - Fails at runtime, not compile time
Hard to debug - Unclear what types are expected
Poor documentation - Types aren't explicit

✅ Solution

php

<?php

declare(strict_types=1);

namespace App\Services;

use App\Models\User;
use App\Repositories\Interfaces\UserRepositoryInterface;

class UserService
{
    public function __construct(
        private UserRepositoryInterface $userRepository
    ) {}

    public function createUser(array $data): User
    {
        return $this->userRepository->create($data);
    }

    public function getUserById(int $id): ?User
    {
        return $this->userRepository->find($id);
    }

    public function isUserActive(User $user): bool
    {
        return $user->is_active && $user->email_verified_at !== null;
    }
}

👹 God Classes

❌ Problem

php

<?php

// One class doing everything
class UserManager
{
    // User CRUD
    public function createUser(array $data): User { }
    public function updateUser(int $id, array $data): User { }
    public function deleteUser(int $id): bool { }
    
    // Authentication
    public function login(string $email, string $password): bool { }
    public function logout(User $user): void { }
    public function resetPassword(string $email): void { }
    
    // Email operations
    public function sendWelcomeEmail(User $user): void { }
    public function sendPasswordResetEmail(User $user): void { }
    public function sendVerificationEmail(User $user): void { }
    
    // Profile operations
    public function updateProfile(User $user, array $data): void { }
    public function uploadAvatar(User $user, $file): void { }
    
    // Statistics
    public function getUserStatistics(User $user): array { }
    public function getLoginHistory(User $user): Collection { }
    
    // Permissions
    public function assignRole(User $user, Role $role): void { }
    public function revokeRole(User $user, Role $role): void { }
    public function checkPermission(User $user, string $permission): bool { }
    
    // And 20 more methods...
}

⚠️ Why It's Bad

Violates SRP - Too many responsibilities
Hard to maintain - Changes affect many areas
Difficult to test - Too many dependencies
Team conflicts - Many developers editing same file
Hard to understand - Too much to comprehend

✅ Solution

php

<?php

// Split into focused classes

// User CRUD operations
class UserService
{
    public function createUser(array $data): User { }
    public function updateUser(int $id, array $data): User { }
    public function deleteUser(int $id): bool { }
}

// Authentication operations
class AuthenticationService
{
    public function login(string $email, string $password): bool { }
    public function logout(User $user): void { }
    public function resetPassword(string $email): void { }
}

// Email operations
class UserNotificationService
{
    public function sendWelcomeEmail(User $user): void { }
    public function sendPasswordResetEmail(User $user): void { }
    public function sendVerificationEmail(User $user): void { }
}

// Profile operations
class UserProfileService
{
    public function updateProfile(User $user, array $data): void { }
    public function uploadAvatar(User $user, $file): void { }
}

// Permission operations
class UserPermissionService
{
    public function assignRole(User $user, Role $role): void { }
    public function revokeRole(User $user, Role $role): void { }
    public function checkPermission(User $user, string $permission): bool { }
}

🔓 Missing Validation

❌ Problem

php

<?php

class UserController extends Controller
{
    public function store(Request $request)
    {
        // No validation!
        $user = User::create([
            'name' => $request->name,
            'email' => $request->email,
            'password' => Hash::make($request->password),
            'role' => $request->role,
        ]);

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

⚠️ Why It's Bad

Security risk - Invalid/malicious data accepted
Data integrity issues - Corrupt data in database
No user feedback - Users don't know what's wrong
Hard to debug - Errors occur later in the process

✅ Solution

php

<?php

// Form Request with validation
class StoreUserRequest extends FormRequest
{
    public function authorize(): bool
    {
        return $this->user()->can('create', User::class);
    }

    public function rules(): array
    {
        return [
            'name' => ['required', 'string', 'max:255'],
            'email' => ['required', 'email', 'unique:users,email', 'max:255'],
            'password' => [
                'required',
                'string',
                'min:12',
                'regex:/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])/',
                'confirmed',
            ],
            'role' => ['required', 'string', 'in:user,admin,moderator'],
        ];
    }

    public function messages(): array
    {
        return [
            'password.regex' => 'Password must contain uppercase, lowercase, number, and special character.',
        ];
    }

    protected function prepareForValidation(): void
    {
        $this->merge([
            'email' => strtolower(trim($this->email ?? '')),
        ]);
    }
}

// Controller using Form Request
class UserController extends Controller
{
    public function store(StoreUserRequest $request)
    {
        // Validation happens automatically
        $user = $this->userService->createUser($request->validated());
        return response()->json(new UserResource($user), 201);
    }
}

🔢 Magic Numbers

❌ Problem

php

<?php

class OrderService
{
    public function calculateTotal(Order $order): int
    {
        $subtotal = $order->items()->sum('price');
        
        // What is 0.15?
        $tax = $subtotal * 0.15;
        
        // What is 1000?
        if ($order->user->is_premium) {
            $discount = 1000;
        }
        
        // What is 50000?
        if ($subtotal > 50000) {
            $shipping = 0;
        } else {
            $shipping = 500;  // What is 500?
        }
        
        return $subtotal + $tax - $discount + $shipping;
    }
}

⚠️ Why It's Bad

Hard to understand - Numbers lack context
Hard to maintain - Values scattered everywhere
Duplication risk - Same values repeated
Error-prone - Easy to use wrong value

✅ Solution

php

<?php

class OrderService
{
    // Define constants with meaningful names
    private const TAX_RATE = 0.15;
    private const PREMIUM_DISCOUNT = 1000; // cents
    private const FREE_SHIPPING_THRESHOLD = 50000; // cents
    private const STANDARD_SHIPPING_COST = 500; // cents

    public function calculateTotal(Order $order): int
    {
        $subtotal = $order->items()->sum('price');
        $tax = (int) round($subtotal * self::TAX_RATE);
        $discount = $this->calculateDiscount($order);
        $shipping = $this->calculateShipping($subtotal);
        
        return $subtotal + $tax - $discount + $shipping;
    }

    private function calculateDiscount(Order $order): int
    {
        return $order->user->is_premium ? self::PREMIUM_DISCOUNT : 0;
    }

    private function calculateShipping(int $subtotal): int
    {
        return $subtotal > self::FREE_SHIPPING_THRESHOLD 
            ? 0 
            : self::STANDARD_SHIPPING_COST;
    }
}

🚨 Ignoring Transactions

❌ Problem

php

<?php

public function transferFunds(User $from, User $to, int $amount): void
{
    // No transaction - if any step fails, partial transfer occurs!
    
    // Step 1: Deduct from sender
    $from->decrement('balance', $amount);
    
    // Step 2: Add to recipient (if this fails, money is lost!)
    $to->increment('balance', $amount);
    
    // Step 3: Create transaction record (if this fails, no audit trail!)
    Transaction::create([
        'from_user_id' => $from->id,
        'to_user_id' => $to->id,
        'amount' => $amount,
    ]);
}

⚠️ Why It's Bad

Data inconsistency - Partial operations succeed
Lost data - Money could disappear
No rollback - Can't undo partial changes
Audit problems - Incomplete transaction records

✅ Solution

php

<?php

use Illuminate\Support\Facades\DB;

public function transferFunds(User $from, User $to, int $amount): bool
{
    return DB::transaction(function () use ($from, $to, $amount) {
        // All or nothing - if any step fails, everything rolls back
        
        // Step 1: Validate balance
        if ($from->balance < $amount) {
            throw new InsufficientBalanceException();
        }
        
        // Step 2: Deduct from sender
        $from->decrement('balance', $amount);
        
        // Step 3: Add to recipient
        $to->increment('balance', $amount);
        
        // Step 4: Create transaction record
        Transaction::create([
            'from_user_id' => $from->id,
            'to_user_id' => $to->id,
            'amount' => $amount,
            'status' => 'completed',
        ]);
        
        return true;
    });
}

🤐 Silent Failures

❌ Problem

php

<?php

public function processPayment(Order $order): bool
{
    try {
        $payment = $this->paymentGateway->charge(
            $order->user->payment_method,
            $order->total
        );
        
        $order->update(['payment_status' => 'paid']);
        
        return true;
    } catch (\Exception $e) {
        // Silently catching and ignoring the error!
        return false;
    }
}

⚠️ Why It's Bad

Lost information - No record of what went wrong
Hard to debug - No logs or traces
User confusion - No feedback about failure
Monitoring blind spot - System appears fine

✅ Solution

php

<?php

use Illuminate\Support\Facades\Log;

public function processPayment(Order $order): bool
{
    try {
        $payment = $this->paymentGateway->charge(
            $order->user->payment_method,
            $order->total
        );
        
        $order->update([
            'payment_status' => 'paid',
            'payment_id' => $payment->id,
        ]);
        
        Log::info('Payment processed successfully', [
            'order_id' => $order->id,
            'payment_id' => $payment->id,
            'amount' => $order->total,
        ]);
        
        return true;
        
    } catch (PaymentDeclinedException $e) {
        // Log specific error
        Log::warning('Payment declined', [
            'order_id' => $order->id,
            'amount' => $order->total,
            'reason' => $e->getMessage(),
        ]);
        
        // Update order status
        $order->update(['payment_status' => 'declined']);
        
        // Notify user
        $order->user->notify(new PaymentDeclinedNotification($order));
        
        return false;
        
    } catch (\Exception $e) {
        // Log unexpected errors
        Log::error('Payment processing failed', [
            'order_id' => $order->id,
            'error' => $e->getMessage(),
            'trace' => $e->getTraceAsString(),
        ]);
        
        // Update order status
        $order->update(['payment_status' => 'failed']);
        
        // Re-throw for higher level handling
        throw new PaymentProcessingException(
            'Failed to process payment',
            0,
            $e
        );
    }
}

🔒 Direct Model Access in Controllers

❌ Problem

php

<?php

class PostController extends Controller
{
    public function index()
    {
        // Direct Eloquent queries in controller
        $posts = Post::with('author')
            ->where('is_published', true)
            ->orderBy('created_at', 'desc')
            ->paginate(20);
        
        return response()->json($posts);
    }

    public function store(Request $request)
    {
        // Direct model creation in controller
        $post = Post::create($request->all());
        
        return response()->json($post, 201);
    }
}

⚠️ Why It's Bad

Not testable - Hard to mock Eloquent
Not reusable - Logic locked in controller
Violates architecture - Controller knows about database
Hard to refactor - Changes affect many controllers

✅ Solution

php

<?php

// Repository handles data access
class PostRepository implements PostRepositoryInterface
{
    public function getPublishedPosts(int $perPage = 20): LengthAwarePaginator
    {
        return Post::with('author')
            ->where('is_published', true)
            ->orderBy('created_at', 'desc')
            ->paginate($perPage);
    }

    public function create(array $data): Post
    {
        return Post::create($data);
    }
}

// Service handles business logic
class PostService
{
    public function __construct(
        private PostRepositoryInterface $postRepository
    ) {}

    public function getPublishedPosts(int $perPage = 20): LengthAwarePaginator
    {
        return $this->postRepository->getPublishedPosts($perPage);
    }

    public function createPost(array $data): Post
    {
        // Business logic here
        $data['slug'] = Str::slug($data['title']);
        return $this->postRepository->create($data);
    }
}

// Controller stays thin
class PostController extends Controller
{
    public function __construct(
        private PostService $postService
    ) {}

    public function index()
    {
        $posts = $this->postService->getPublishedPosts();
        return PostResource::collection($posts);
    }

    public function store(StorePostRequest $request)
    {
        $post = $this->postService->createPost($request->validated());
        return new PostResource($post);
    }
}

📋 Anti-Patterns Summary

Anti-Pattern	Impact	Solution
Fat Controllers	High	Service Layer + Repositories
N+1 Queries	Critical	Eager Loading
Missing Types	Medium	Strict Types + Type Hints
God Classes	High	Single Responsibility
Missing Validation	Critical	Form Requests
Magic Numbers	Low	Constants
No Transactions	Critical	DB Transactions
Silent Failures	High	Proper Error Handling
Direct Model Access	Medium	Repository Pattern

⚠️ Remember: These aren't just "best practices" - they prevent real bugs, security issues, and maintenance nightmares. Take them seriously!

⚠️ Common Mistakes & Anti-Patterns ​

🎮 Fat Controllers ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🔄 N+1 Query Problem ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🚫 Missing Type Declarations ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

👹 God Classes ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🔓 Missing Validation ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🔢 Magic Numbers ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🚨 Ignoring Transactions ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🤐 Silent Failures ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

🔒 Direct Model Access in Controllers ​

❌ Problem ​

⚠️ Why It's Bad ​

✅ Solution ​

📋 Anti-Patterns Summary ​

⚠️ Common Mistakes & Anti-Patterns

🎮 Fat Controllers

❌ Problem

⚠️ Why It's Bad

✅ Solution

🔄 N+1 Query Problem

❌ Problem

⚠️ Why It's Bad

✅ Solution

🚫 Missing Type Declarations

❌ Problem

⚠️ Why It's Bad

✅ Solution

👹 God Classes

❌ Problem

⚠️ Why It's Bad

✅ Solution

🔓 Missing Validation

❌ Problem

⚠️ Why It's Bad

✅ Solution

🔢 Magic Numbers

❌ Problem

⚠️ Why It's Bad

✅ Solution

🚨 Ignoring Transactions

❌ Problem

⚠️ Why It's Bad

✅ Solution

🤐 Silent Failures

❌ Problem

⚠️ Why It's Bad

✅ Solution

🔒 Direct Model Access in Controllers

❌ Problem

⚠️ Why It's Bad

✅ Solution

📋 Anti-Patterns Summary