https://laraveldaily.com/storage/377/Add-a-heading-(5).png
Some time ago I made a YouTube series called Code Reviews. From that series and other reviews, I’ve collected the 9 most common repeating mistakes Laravel beginners make.
Not all of those are really serious flaws, most of them are just not the most effective ways to code. Then it’s an open question why do you use a framework like Laravel and don’t actually use its core features in full?
So, in no particular order…
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);
});
});
Read more
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']));
}
Read more
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');
}
See how shorter the code is?
If you want to dive deeper into Eloquent, I have a full course Eloquent: The Expert Level
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.
I have an example of such transformation of a typical Controller method in this article: Laravel Structure: Move Code From Controller to… Where?
There are various approaches how to structure the code, but what should be avoided is one huge method responsible for everything.
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> ()</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:
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.
You can also watch my video about it: Laravel Foreign Keys: How to Deal with Errors
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.
Laravel News Links
