Laravel: 9 Typical Mistakes Juniors Make

Laravel: 9 Typical Mistakes Juniors Make

Monday April 10, 2023

Mistake 1. Not Using Route Groups

Where possible combine routes into groups. For example, you have routes like this:

Route::get('dashboard', [HomeController::class, 'index'])->name('dashboard')->middleware(['auth']);
Route::resource('donation', DonationController::class)->middleware(['auth']);
Route::resource('requisition', RequisitionController::class)->middleware(['auth']);
 
Route::name('admin.')->prefix('admin.')->group(function () {
    Route::view('/', 'admin.welcome')->middleware(['auth', 'admincheck']);
    Route::resource('donor', DonorController::class)->middleware(['auth', 'admincheck']);
    Route::resource('details', OrganisationDetailController::class)->middleware(['auth', 'admincheck']);
});

Here, we have all routes that have middleware auth and three routes that also check if the user is admin. But those middlewares are repeating each time.

It would be better to put all routes into the group to check middleware for auth and then inside have another group for admin routes.

This way when the developer opens the routes file, he will immediately know which routes are only for authenticated users.

Route::middleware('auth')->group(function () {
    Route::get('dashboard', [HomeController::class, 'index'])->name('dashboard');
    Route::resource('donation', DonationController::class);
    Route::resource('requisition', RequisitionController::class);
 
    Route::name('admin.')->prefix('admin.')->middleware('admincheck')->group(function () {
        Route::view('/', 'admin.welcome');
        Route::resource('donor', DonorController::class);
        Route::resource('details', OrganisationDetailController::class);
    });
});

Mistake 2. Not Using Route Model Binding

Often I see beginner coders in the controller manually search for the data, even if in routes Model Binding is specified correctly. For example, in your routes you have:

Route::resource('student', StudentController::class);

So here it's even a resource route. But I see some beginners still write Controller code like this:

public function show($id)
{
    $student = Student::findOrFail($id);
 
    return view('dashboard/student/show', compact(['student']));
}

Instead, use Route Model Binding and Laravel will find Model:

public function show(Student $student)
{
    return view('dashboard/student/show', compact(['student']));
}

Mistake 3. Too Long Eloquent Create/Update Code

When saving data into DB I have seen people write code similar to this:

public function update(Request $request)
{
    $request->validate(['name' => 'required']);
    $user = Auth::user();
    $user->name = $request->name;
    $user->username = $request->username;
    $user->mobile = $request->mobile;
    // Some other fields...
    $user->save();
    return redirect()->route('profile.index');
}

Instead, it can be written shorter, in at least two ways.

First, in this example, you don't need to set the Auth::user() to the $user variable. The first option could be:

public function update(Request $request)
{
    $request->validate(['name' => 'required']);
    auth()->user()->update([$request->only([
        'name',
        'username',
        'mobile',
        // Some other fields...
    ]);
    return redirect()->route('profile.index');
}

The second option put validation into Form Request. Then into the update() method you would need just to pass $request->validated().

public function update(ProfileRequest $request)
{
    auth()->user()->update([$request->validated());
    return redirect()->route('profile.index');
}

Mistake 4. Not Naming Things Properly

Many times beginners name things however they want, not thinking about other developers who would read their code in the future.

For example, shortening variable names: instead of $data they call $d. Always use proper naming. For example:

Route::get('/', [IndexController::class, 'show'])
  ->middleware(['dq'])
  ->name('index');
Route::get('/about', [IndexController::class, 'about'])
  ->middleware(['dq'])
  ->name('about');
Route::get('/dq', [IndexController::class, 'dq'])
  ->middleware(['auth'])
  ->name('dq');

What is this middleware and method in Index Controller called dq? Well, in this example, if we would go into app/Http/Kernel.php to find this middleware, we could find something like this:

class Kernel extends HttpKernel
{
  // ...
  protected $routeMiddleware = [
    'auth' => \App\Http\Middleware\Authenticate::class,
    'admin' => \App\Http\Middleware\EnsureAdmin::class,
    'dq' => \App\Http\Middleware\Disqualified::class, 
    'inprogress' => \App\Http\Middleware\InProgress::class,
    // ...
  ];
}

It doesn't matter what's inside this middleware, but from the name of the middleware file it is disqualified. So everywhere instead of dq it should be called disqualified. This way if other developers would join the project, they would have a better understanding.

Mistake 5. Too Big Controllers

Quite often I see juniors writing huge Controllers with all possible actions in one method:

• Validation
• Checking data
• Transforming data
• Saving data
• Saving more data in other tables
• Sending emails/notifications
• ... and more

That could all be in one store() method, for example:

public function store(Request $request)
{
    $this->authorize('user_create');
 
    $userData = $request->validate([
        'name' => 'required',
        'email' => 'required|unique:users',
        'password' => 'required',
    ]);
 
    $userData['start_at'] = Carbon::createFromFormat('m/d/Y', $request->start_at)->format('Y-m-d');
    $userData['password'] = bcrypt($request->password);
 
    $user = User::create($userData);
    $user->roles()->sync($request->input('roles', []));
 
    Project::create([
        'user_id' => $user->id,
        'name' => 'Demo project 1',
    ]);
    Category::create([
        'user_id' => $user->id,
        'name' => 'Demo category 1',
    ]);
    Category::create([
        'user_id' => $user->id,
        'name' => 'Demo category 2',
    ]);
 
    MonthlyRepost::where('month', now()->format('Y-m'))->increment('users_count');
    $user->sendEmailVerificationNotification();
 
    $admins = User::where('is_admin', 1)->get();
    Notification::send($admins, new AdminNewUserNotification($user));
 
    return response()->json([
        'result' => 'success',
        'data' => $user,
    ], 200);
}

It's not necessarily wrong but it becomes very hard to quickly read for other developers in the future. And what's hard to read, becomes hard to change and fix future bugs.

Instead, Controllers should be shorter and just take the data from routes, call some methods and return the result. All the logic for manipulating data should be in the classes specifically suitable for that:

• Validation in Form Request classes
• Transforming data in Models and/or Observers
• Sending emails in events/listeners put into the queue
• etc.

Mistake 6. N+1 Eloquent Query Problem

By far the no.1 typical reason for poor performance of Laravel project is the structure of Eloquent queries. Specifically, N+1 Query problem is the most common: running hundreds of SQL queries on one page definitely takes a lot of server resources.

And it's relatively easy to spot this problem in simple examples like this:

// Controller not eager loading Users:
$projects = Project::all();
 
// Blade:
@foreach ($projects as $project)
    <li>{{ $project->name }} ({{ $project->user->name }})</li>
@endforeach

But real life examples get more complicated and the amount of queries may be "hidden" in the accessors, package queries and other unpredictable places.

Also, typical junior developers don't spend enough time testing their application with a lot of data. It works for them with a few database records, so they don't go extra mile to simulate future scenarios, where their code would actually cause performance issues.

A few of my best resources about it:

• Premium tutorial: Optimizing Laravel Eloquent and DB Speed: All You Need to Know
• Course: Better Eloquent performance

Mistake 7. Breaking MVC Pattern: Logic in Blade

Whenever I see a @php directive in a Blade file, my heart starts beating faster.

See this example:

@php
$x = 5
@endphp

Except for very (VERY) rare scenarios, all the PHP code for getting the data should be executed before coming to show it in Blade.

MVC architecture was created for a reason: that separation of concerns between Model, View and Controller makes it much more predictable where to search for certain code pieces.

And while the M and C parts can be debated whether to store the logic in Model, in Controller, or in separate Service/Action classes, the V layer of Views is kinda sacred. The golden rule: views should not contain logic. In other words, views are only for presenting the data, not for transforming or calculating it.

The origin of this comes to the fact that Views could be taken by a front-end HTML/CSS developer and they could make the necessary changes to styling, without needing to understand any PHP code. Of course, in real life that separation rarely happens in teams, but it's a noble goal from a pattern that comes from outside of Laravel or even PHP.

For most mistakes in this list I have a "read more" list of links, but here I have nothing much to add. Just don't store logic in Views, that's it.

Mistake 8. Relationships: Not Creating Foreign Keys

Relationships between tables are created on two levels: you need to create the related field and then a foreign key. I see many juniors forget the second part.

Have you ever seen something like this in migration?

$table->unsignedBigInteger('user_id');

On the surface, it looks ok. And it actually does the job, with no bugs. At first.

Let me show you what happens if you don't put constrained() or references() in that migration file.

Foreign key is the mechanism for restricting related operations on the database level: so when you delete the parent record, you may choose what happens with children: delete them too or restrict the parent deletion in the first place.

So, if you create just a unsignedBigInteger() field, without the foreign key, you're allowing your users to delete the parent without any consequences. So the children stay in the database, with their parent non-existing anymore.

Mistake 9. Not Reading The Documentation

I want to end this list with an elephant in the room. For many years, the most popular of my articles and tweets are with information literally taken from the documentation, almost word for word.

Over the years I've realized that people don't actually read the documentation in full, only the main parts of the ones that are the most relevant to them.

So many times, developers surprised me by not knowing the obvious features that were in the docs.

Junior developers earn mostly from ready-made tutorials or courses, which is fine, but reading the official docs should be a regular activity.

• Posted on April 10, 2023
• Click for Source