Популярные ошибки разработчиков
Принципы SOLID, DRY, KISS, YAGNI и прочие, являются само собой разумеющимися и обсуждать их - тема, которая выходит за рамки данного ресурса. Для этого есть Telegram чат .
Рекомендую также прочитать раздел «Плохие привычки» » из книги «Архитектура сложных веб-приложений» указанной на странице «Ссылки» .
# Логика в контроллерах
Наверняка ты уже слышал фразу «Толстые контроллеры - это плохо».
Приводить объёмные примеры логики в контроллере не вижу смысла. Рекомендую почитать книгу указанную выше. В ней присутствуют хорошие примеры и объяснения что делать. Также смотри определение контроллера по GRASP
Здесь же скажу кратко: Контроллер принимает входящий запрос, вызывает необходимые действия (сервисы, экшены, команды) и возвращает ответ клиенту. Ничего более в нем быть не должно.
# Слишком тонкие контроллеры
Некоторые разработчики буквально воспринимают понятие "тонкие контроллеры" и пишут такой код:
1public function store(Request $request, PostService $postService)2{3 return $postService->create($request);4}
Здесь явное нарушение SRP: PostService зависит от Request и отвечает за Response, В результате такой сервис нельзя нигде переиспользовать, а смысл контроллера сводится к нулю.
# DI в конструкторе контроллера
Следующий пример - результат наличия логики в контроллере и, как следствие, попытка "разгрузить" методы. Требовать зависимости необходимо в конструкторе сервиса, а не контроллера.
1public function __construct(MyService $myService) 2{ 3 $this->myService = $myService; 4} 5 6public function store() 7{ 8 $this->myService->handle(); 9}10 11public function update()12{13 $this->myService->handle();14}
Что в этом коде такого, спросишь ты?
- Он бесполезен. Визуально кажется, что сервис используется в нескольких методах и логично инициализацию вынести в одно место, но публичные методы контроллера не должны вызывать друг друга. Следовательно сервис используется только в одном методе на один запрос и больше не будет вызван в рамках жизненного цикла для текущего контроллера, поэтому использование $this->service в контроллере обычно не имеет смысла. Исключением могут быть приватные методы контроллера, но размещать логику в таких методах может быть не самым удачным решением по сравнению с сервисными классами.
- Не всем методам может понадобиться один и тот же сервис, зачем его загружать для всех запросов? Незачем.
- И самое важное, в жизненном цикле конструктор контроллера вызывается до middlewares. Согласно документации , в конструкторе контроллера можно указывать «middleware» (для порядка и удобства рекомендую указывать их только в роутах), а значит сам контроллер создаётся до этапа выполнения "мидлварок" и в момент внедрения зависимостей ещё будут недоступны некоторые функции, например, сессия и куки, а значит отсутствовать и авторизация. И если конструктор сервиса зависит от подобных вещей, то работать правильно он не сможет, а отладка бага может занять некоторое время.
Поэтому запрашивай необходимые сервисы в методах контроллера:
1public function store(MyService $myService)2{3 $myService->handle();4}5 6public function update(MyService $myService)7{8 $myService->handle();9}
Последний пример не является нарушением DRY. Даже если в разных методах сервис используется одинаково, то это всё равно не повод выносить куда-либо, ибо "экшены" контроллера независимы друг от друга и в любой момент может понадобиться внести изменения в один из них. И если методы получились полностью одинаковыми, то всё равно вызов логики - это не сама логика.
# Логика в шаблонах major
Иногда начинающих ставит в тупик требование не использовать логику в шаблонах, «как же тогда выводить данные?».
Всё просто, всё что можно подготовить заранее - вынеси и подготовь заранее. В шаблоне должна остаться только логика отвечающая за само отображение. Можно вызывать сервисы и хелперы, обращаться к константам, но никаких запросов к БД из шаблонов быть не должно.
1@foreach (Product::get() as $product)2 @php($total += $product->price * $product->qty)3 4 <img src="{{ Storage::disk('products')->url($product->cover) }}" />5 6 {{ new \Carbon\Carbon($product->created_at)->format('Y-m-d') }}7@endforeach8 9Total price: {{ $total }}
Кейсы бывают разные и порой сложно полностью отказаться от использования php и определения новых переменных, но всегда старайся свести к минимуму, особенно избавиться от каких-то вычислений не связанных с вёрсткой. В примере выше, подсчитывается общая цена при переборе товаров. Это ужаснейшая идея реализовывать подобную логику в шаблоне. Либо считай заранее в сервисе/контроллере, либо вызывай сервис из шаблона, который подсчитает, но не пиши прямо в блейде.
Представь, что тебе надо будет изменить формулу ценообразования и учитывать текущие акции, код может вырасти на десятки строк, а у тебя 5 разных шаблонов, где отображается товар, в каждый будешь копипастить? Вроде смешно, но это история из реального проекта и смешного в ней нет ничего.
Чем чище html, тем лучше.
1@foreach ($products as $product)2 <img src="{{ $product->cover_url }}" />3 4 {{ $product->created_at->format('Y-m-d') }}5@endforeach6 7Total price: {{ $totalPrice }}
Также не стоит забывать, что шаблон может быть переиспользован на странице несколько раз и если в нем присутствует тяжёлая логика, например, какой-то цикл с вызовом сервиса, то каждое использование шаблона будет повторно запускать этот код, поэтому крайне рекомендуется передавать в шаблон максимально подготовленные данные, на сколько позволяет ситуация, конечно же.
# Логика в маршрутах major
В документации логика в роутах используется для наглядности примеров, не более.
В роутах можно что-то протестировать быстро для себя, но потом обязательно удалить или перенести в контроллер, хуже места для написания любой логики придумать сложно.
1Route::get('posts', static function () {2 $posts = Post::active()->paginate(config('post.per_page'));3 4 return view('post.index', compact('posts'));5});
Даже что-то крайне простое (возврат шаблонов, редиректы) лучше вынести в контроллер или использовать готовые для этого методы.
1Route::get('posts', [PostController::class, 'index']);2 3Route::view('posts', 'posts.index');4 5Route::redirect('old-posts', 'posts');
Стоит отметить, что на практике использование Route::view() неоправданно, т.к. это вносит двойственность (где-то шаблоны возвращаются роутами, а где-то контроллерами) и чтобы добавить какую-либо логику, необходимо всё равно создавать контроллер.
# Валидация в контроллере
Сама по себе валидация в контроллере не критична, т.к. контроллер отвечает за обработку запроса и ответ в случае какой-либо ошибки. Но это загрязняет методы контроллера и всё же нарушает SRP, тем более в Laravel есть готовый механизм для таких задач.
1public function store(Request $request) 2{ 3 $data = $request->validate([ 4 'heading' => ['required', 'string', 'max:255'], 5 'content' => ['nullable', 'string'], 6 'status' => ['required', 'boolean'], 7 ]); 8 9 Post::create($data);10}
Вместо валидации в контроллерах используй FormRequest
1public function store(PostCreateRequest $request)2{3 Post::create($request->validated());4}5 6public function update(PostUpdateRequest $request, $id)7{8 Post::where('id', $id)->update($request->validated());9}
Обрати внимание:
- для каждого запроса используется свой FormRequest. Не стоит делать общий PostRequest, даже если правила в конечном итоге получились одинаковыми.
- Не используй в контроллере ручное создание валидатора Validator::make() без явной на то необходимости. В случае возникновения ошибки валидации, фреймворк сам вернёт ошибку и завершит выполнение. Иначе при использовании Validator::make() необходимо также вручную сформировать ответ.
# Запросы в циклах major
Код ниже сгенерирует по 2 запроса на каждую итерацию. Если в $ids 100 элементов, значит запросов будет 200, если 500, то 1000...
1foreach ($ids as $id) {2 $post = Post::first('id', $id);3 $post->update(['status' => 1]);4}
Есть редкие кейсы, когда выгоднее или проще создать много запросов, чем писать несколько, но сложных, особенно если код будет выполняться в фоне, не важно сколько времени это может занять и какую нагрузку дать на сервер.
Но в подавляющем большинстве случаев необходимо избегать использования запросов к БД из циклов.
1Post::whereIn($ids)->update(['status' => 1]);
Также обязательно прочти «Ленивая загрузка связей», т.к. тема тоже относится к данной проблеме, но менее интуитивна для начинающих.
# Ленивая загрузка связей major
Сколько запросов к БД происходит в примере ниже? Попробуй посчитать самостоятельно, при условии, что каждый пост имеет 5 комментариев и всего выбрано 10 постов (такие вопросы могут задавать на собеседованиях).
1public function index()2{3 $posts = Post::active()->paginate(10);4 5 return view('posts.index', compact('posts'));6}
1@foreach ($posts as $post)2 {{ $post->heading }}3 4 @foreach($post->comments as $comment)5 {{ $comment->message }}6 {{ $comment->user->name }}7 @endforeach8@endforeach
Посчитал? Теперь давай разберёмся. Мы выбираем 10 постов из БД одним запросом и передаём коллекцию в шаблон. В шаблоне мы перебираем посты и обращаемся на связь $post->comments. Каждое такое обращение это +1 запрос к таблице comments, в котором подгружаются комментарии к посту. Значит на 10 постов +10 запросов, итого уже 11. Далее происходит перебор комментариев (мы условились что их 5 у каждого поста) и у каждого комментария есть обращение на связь $comment->user, чтобы получить автора комментария, т.е. ещё +1 запрос на каждый комментарий, а значит 10*5 = +50 запросов. Итого имеем 61 запрос к БД.
Эта называется «проблема N+1». В реальности постов и комментариев на одной странице может быть больше и количество запросов легко переходит в сотни (например 10 постов по 30 комментариев в итоге даст 311 запросов).
Чтобы исправить данную проблему существует понятие «Eager Loading» или «Жадная загрузка», смысл которой заранее получить все необходимые данные для отображения, а не делать запросы из циклов по требованию (ленивая загрузка). В Laravel это предусмотрено, читай раздел документации «Eager Loading» .
Исправим наш запрос в контроллере с помощью with():
1public function index()2{3 $posts = Post::with('comments.user')->active()->paginate(10);4 5 return view('posts.index', compact('posts'));6}
По итогу получаем всего 3 запроса к БД вместо 61: посты, комментарии и авторы комментариев.
Обрати внимание:
- Начиная с 8-ой версии появилась возможность отключить ленивую загрузку, чтобы случайно не обратиться к связям, которые не были загружены заранее. Подробнее читай в документации
- Laravel сам загрузит все необходимые связи указанные в цепочке. Нет необходимости указывать отдельно: with('comments', 'comments.user'), хоть это и не будет ошибкой.
# Получение текущего пользователя major
Популярнейшая проблема встречается в коде начинающих:
1User::find(Auth::user()->id);2// or3User::find(Auth::id());
В примере выше происходит бесполезная выборка из БД. Auth::user() уже возвращает модель авторизированного пользователя. Нет смысла вытаскивать данные ещё раз через User::find()
1$user = Auth::user();2$user->email;3 4$userId = Auth::id();
Обрати внимание:
- Если пользователь не авторизирован, то Auth::user() вернёт null
- Фасад Auth и хелпер auth() эквивалентны
# Обновление данных minor
Чтобы обновить данные, часто можно встретить такой код:
1$post = Post::find($id);2 3$post->update($data);4// or5$post->fill($data);6$post->save();
В данном примере нет проблем, если получаемые данные нужны для каких-либо действий, например, для ответа клиенту.
Если же задача просто обновить данные в БД, то Post::find() не нужен, т.к. это лишний SELECT запрос. Достаточно выполнить UPDATE запрос напрямую, без выборки:
1Post::where('id', $id)->update($data);
Следует различать методы модели и билдера:
Illuminate/Database/Eloquent/Model::update
Illuminate/Database/Eloquent/Builder::update
В первом примере используется метод модели, во втором - билдера. Оба варианта правильные, но второй предпочтителен, если нужно только обновить данные. Стоит отметить, что события, при таком обновлении, не будут вызваны.
# Mass assignment minor
Немного спорный вопрос, балансирующий между "краткостью" и "контролем".
В первом примере заполнение полей происходит вручную и код выглядит громоздко.
1$post = new Post();2$post->heading = $request->get('heading');3$post->description = $request->get('description');4$post->content = $request->get('content');5$post->save();
Во втором примере используется массовое заполнение и код выглядит компактно.
1$post = Post::create($request->validated());
На практике второй вариант обычно предпочтителен, т.к. имеет лаконичный формат и снижает риск опечаток в именах полей. Требует указания $fillable в модели.
С другой стороны, первый вариант, более явный, сразу видно какие поля сохраняются и удобно добавить новое, но необходимо вручную перечислять все поля.
Можно использовать оба варианта вместе:
1$post = new Post();2$post->fill($request->validated());3$post->user_id = Auth::id();4$post->active = 1;5$post->save();
# Тяжелые выборки major
В первом примере код выбирает все записи из БД. Это хорошо работает пока в таблице пару десятков или даже сотен записей.
1Post::all();2//or3Post::get();
Но когда количество записей в таблице заметно возрастает, то начинаются проблемы с производительностью и такие запросы долго выполняются и потребляют большое количество памяти.
Поэтому рекомендуется не использовать метод all(), а ограничивать количество записей, например, с помощью пагинации или явно указав лимит.
1Post::paginate(10);2// or3Post::limit(100)->get();
Иногда действительно необходимо получить все записи из небольшой таблицы, например, цвета товара или статусы заказа. Даже в таком случае лучше перестраховаться и ограничить выборку.
Другой вариант используется для выборки всех записей из большой таблицы, например, нам необходимо обработать каждую запись или экспортировать в файл. Для этого необходимо использовать метод chunk(), который будет порционно доставать записи, снижая нагрузку на БД и экономя память. Подробнее можно почитать в документации
1Post::chunk(100, static function ($posts) {2 // Process the records3 // $posts->each(...);4});
# Делегирование выборки на коллекции
Популярная ошибка у новичков, которые не задумываются как работает их код, а смотрят лишь на результат. Оба примера ниже дают одинаковый результат, но работают с разной эффективностью.
1Post::get()->where('user_id', Auth::id());2// or3Post::get()->sortBy('created_at');4// or5Post::get()->first();
В первом случае, всё что после методов get() или all() является коллекцией моделей. Запрос к БД уже выполнен. Т.е. выбираются все записи из таблицы и затем на уровне php фильтруется/сортируется коллекция.
Правильный вариант - выбирать запросом только необходимые записи и сортировать их на уровне БД.
1Post::where('user_id', Auth::id())->get();2// or3Post::orderBy('created_at')->get();4// or5Post::first();
Всё, что можно сделать на уровне БД, то лучше делать там, т.к. СУБД имеет оптимизированные алгоритмы для таких задач. Редкие кейсы со сложными сортировками и группировками, которые не позволяет сделать СУБД, можно выполнять на уровне коллекций или массивов в php.
# Хардкод
Избегай использования жёстко заданных значений прямо в коде.
1public function update(Post $post)2{3 $post->status = 'updated';4 5 Mail::to('admin@localhost')->send();6 7 return redirect()->withSuccess('Post successfully updated');8}
Выноси значения в конфиги, переводы, константы и базу данных.
1public function update(Post $post)2{3 $post->status = Post::STATUS_UPDATED;4 5 Mail::to(config('mail.to.admin'))->send();6 7 return redirect()->withSuccess(__('messages.post_updated'));8}
# Использование env() critical
Использовать хелпер env() следует только в файлах конфигурации.
Это связано с тем, что после кэширования конфига, env() не будет возвращать значения из файла .env
Об этом говорится в документации , но к сожалению, часто упускается новичками.
Получать значения необходимо только через config()
# Высокая связанность / High coupling major
«Качественный дизайн должен обладать слабой связанностью (low coupling) и сильной связностью (high cohesion)»
Тема IoC (Inversion of Control) достаточна обширна и выходит за рамки данного ресурса,
поэтому если не знаком, то рекомендую поскорее закрыть этот пробел, хотя бы в общих чертах.
Также необходимо ознакомиться с документацией
Service Container .
Рассмотрим простой метод:
1public function index()2{3 $myService = new MyService();4 $myService->handle();5}
Проблема данного кода, что он имеет высокую связанность, т.к. в нем создаётся новый объект. Ситуация усугубляется ещё больше, когда данный сервис начинает требовать какие-либо зависимости, а те зависимости, в свою очередь, требуют свои зависимости и так далее...
Также такой код будет затруднительно тестировать, т.к. сервис нельзя "замокать".
Вместо прямого создания объекта необходимо использовать DI, реализация которого уже есть в Laravel:
1public function index(MyService $myService)2{3 $myService->handle();4}
И если следовать букве D из «SOLID», то в данном примере MyService должен быть абстракцией, а не конкретным классом (реализацией).
# Дублирование частей QueryBuilder-а
Часто необходимо выбирать данные из БД по одинаковым условиям. Чтобы избежать дублирования кода, используются «Scopes»
1public function index()2{3 Post::where('active', 1)->get();4}5 6public function show()7{8 Post::where('active', 1)->first();9}
Кроме дублирования, скоупы повышают читаемость запроса (если правильно именованы).
1public function index() 2{ 3 Post::active()->get(); 4} 5 6public function show() 7{ 8 Post::active()->first(); 9}10 11// App\Models\Post12public function scopeActive(Builder $query)13{14 $query->where('active', 1);15}
Также можно использовать глобальные скоупы, которые будут применяться ко всем запросам автоматически. Это может быть удобно, чтобы гарантировать выборку только активных записей (по такому принципу работает soft delete). Но рекомендую прежде подумать, что будет выгоднее для твоего приложения, прописывать везде скоупы или withoutGlobalScope(). Также глобальные скоупы снижают очевидность запроса, т.е. сложно, посмотрев на QB, сказать какие записи выбираются без проверки глобальных скоупов и этот момент придётся держать в памяти.
# Хранение файлов в public
По непонятным причинам многие новички пишут пользовательские файлы в public
директорию.
Согласно документации , для хранения файлов
есть директории /storage/app для приватных файлов и storage/app/public
для общедоступных.
Технической проблемы "хранения в public" нет, кроме того факта, что все файлы будут всегда доступны напрямую, и нельзя программно контролировать их доступ, например, по временной ссылке. Также это не интуитивный момент для других разработчиков, они, как и сторонние пакеты, будут использовать storage/app и в проекте появится двойственность, одни файлы будут в storage, другие в public.
В Laravel есть мощная система по работе с файлами, с конфигурацией на storage/app и которую легко можно изменить под собственные нужды, вплоть до написания собственного драйвера.
Ещё одна популярная ошибка это использование хелпера storage_path() вместо фасада Storage. Вся нужная информация есть в документации и рекомендую почитать простую инструкцию «Как работать с файлами» .
# Нативная работа с датами
По умолчанию, в Laravel используется мощный пакет для работы с датами Carbon , который наследуется от нативного DateTime .
Поля модели такие как created_at, updated_at, deleted_at и другие, которые имеют каст, содержат не строку, а объект \Carbon\Carbon, поэтому нет смысла и необходимости повторно оборачивать в \DateTime
1$date = new \DateTime($post->created_at);2$date->add(new \DateInterval('P1D'));3$date->format('Y-m-d');
Код выше работает, потому что $post->created_at имеет метод toString(). В результате объект конвертируется в строку, чтобы создать новый объект, но уже нативный, а не Carbon
Вместо этого сразу можно работать с датой как с объектом:
1$date = $post->created_at->addDay();2$date->format('Y-m-d');
# Настройка document_root critical
Популярная ошибка у невнимательно читающих документацию , в которой сказано, что запросы должны приходить в public/index.php
Чаще всего проблема вызвана попыткой завести лару на виртуальном хостинге (shared hosting), который не позволяет управлять корневой директорией web-сервера.
В результате начинающие перемещают index.php в корень фреймворка и/или размещают .htaccess в котором настраивают перенаправление в public
1# .htaccess2RewriteRule ^$ public/index.php [L]3RewriteRule ^(.*)$ public/$1 [L]4# or any variants
Это серьёзная проблема безопасности, а в случае с Apache ещё страдает и производительность. Приложение должно находиться за пределами публичной директории, а web-сервер "смотреть" в public
1# Nginx2server {3 root /srv/example.com/public;4}
1# Apache2<VirtualHost>3 DocumentRoot /srv/example.com/public4</VirtualHost>
Если нет возможности изменить document_root, то читай «Как установить Laravel на хостинг».
# Именования
Данная тема не относится к самому фреймворку и вроде всё понятно и нечего обсуждать, но каждый день в чатах скидывают код, в котором совершены одни и те же ошибки именования.
1$post = Post::get(); // Collection2 3$model = Post::first(); // Model4 5$user = Auth::id(); // integer6 7$data = $user->getPathArray(); // array
Принцип простой - названия переменных, методов, классов и т.п. должны отражать содержимое или предназначение. Обрати внимание, что "содержимое" это не тип данных.
Если переменная содержит много элементов (массив, коллекция), то имя во множественном числе, иначе в единственном.
1$posts = Post::get(); // Collection2 3$post = Post::first(); // Model4 5$userId = Auth::id(); // integer6 7$userPhotos = $user->getPhotos(); // array