Может войдёшь?
Черновики Написать статью Профиль

Хорошие практики Laravel: принцип единственной ответственности (Single Responsibility Principle)

Best practices Хорошие практики

Небольшое вступление

В мире Laravel существует очень серьезная, на мой взгляд, проблема. Laracasts, книги, видео туториалы, статьи и даже документация показывают нам использование плохих практик. Понятно, что делается это для популяризации фреймворка, снижая порог вхождения для новичков. Действительно, благодаря такому подходу, человек может написать работающее веб приложение при минимальных усилиях. И это хорошо. Плохо то, что разработчик продолжает писать низкокачественный код даже в сложных приложениях, в результате чего они порой становятся абсолютно неподдерживаемыми. Это значит, что любое изменение функционала в таком приложении занимает в разы, а иногда и в десятки раз больше времени разработчика и, соответственно, денег клиента. Также, эти правки образуют новые баги, ломают другие части приложения и т.д.

Проанализировав десятки реальных коммерческих приложений, написанных на Laravel, я понял, что практически все разработчики игнорируют даже самые простые хорошие практики написания кода.

Отталкиваться я буду не от заезженных терминов вроде DRY, SOLID, описания паттернов и пр. Подобных статей довольно много и их авторы, зачастую не понимая сути подобных принципов, вновь и вновь пересказывают их. А примеры кода в этих статьях зачастую не имеют ничего общего с кодом приложений из реального мира. Моя же цель - дать новичкам действительно полезный материал, с помощью которого они бы могли заметно улучшить качество своих приложений.

Принцип единственной ответственности (Single responsibility principle или SRP)

Хотите писать более качественный и поддерживаемый код, чем пишет 95% Laravel разработчиков? Изучите, впитайте и везде применяйте принцип единственной ответственности (далее SRP). Вы также можете прочитать о других хороших практиках Laravel, а также о хороших практиках разработки в контексте PHP.

Сейчас же мы возьмем кусочек кода контроллера, подобный которому вы можете увидеть практически во всех реальных Laravel приложениях. Затем, шаг за шагом, мы будем переписывать наш код, следуя при этом SRP. Итак, код, с которым мы будем работать:

public function store(Request $request)
{
    $request->validate([
        'title' => 'required|max:255',
        'content' => 'required',
        'make_id' => 'required'
    ]);

    if (auth()->guest() || !auth()->user()->hasRole('moderator') || !auth()->user()->canAddContent()) {
        return redirect('/cars/models')->with('error', 'У вас нет прав для добавления новой модели');
    }

    $make = Make::find($request->make_id);

    if (!$make) {
        return redirect('/cars/models')->with('error', 'Неверная марка');
    }

    $model = $make->model()->create($request->all());

    if ($request->hasFile('image') && config('app.uploading_enabled')) {
        Image::make($request->file('image'))
             ->resize(300, null, function ($constraint) {
                 $constraint->aspectRatio();
             })
             ->save($public_path('images/models') . DIRECTORY_SEPARATOR . $model->id . '.jpg');
        }
    }

    return redirect('/cars/models')->with('message', 'Модель успешно добавлена');
}

Этот код добавляет новую модель автомобиля (model), привязывает его к марке автомобиля (make), обрабатывает загруженное изображение и выполняет несколько других функций.

Но SRP говорит нам о том, что этот метод контроллера должен выполнять лишь одну функцию. Контроллер должен лишь обработать запрос и отправить ответ. В результате нарушения этого важного принципа, контроллеры раздуваются до невероятных размеров и вносить изменения в существующий код, не сломав его, становится все сложнее и сложнее. Тестировать такой код также намного труднее, а иногда и вовсе невозможно. Давайте также вспомним о концепции тонких контроллеров и начнем рефакторинг.

1

Контроллер не должен заниматься валидацией, поэтому первое, что мы сделаем - создадим Request-класс и перенесем валидацию в него. Также обратите внимание на этот кусок кода:

$make = $this->make->find($request->make_id);

if (!$make) {
    return redirect('/cars/models')->with('error', 'Неверная марка');
}

Его мы заменим правилом валидации exists:makes,id. В результате этого, метод rules нашего ModelRequest-класса будет выглядеть так:

class ModelRequest extends FormRequest
{
    public function authorize()
    {
        return true;
    }

    public function rules()
    {
        return [
            'title' => 'required|max:255',
            'content' => 'required',
            'make_id' => 'required|exists:makes,id'
        ];
    }
}

Внедрим ModelRequest-класс в наш контроллер, который уже стал заметно тоньше:

public function store(ModelRequest $request)
{
    if (auth()->guest() || !auth()->user()->hasRole('moderator') || auth()->user()->canAddContent()) {
        return redirect('/cars/models')->with('error', 'У вас нет прав для добавления новой модели');
    }

    $model = $this->model->create($request->all());

    if ($request->hasFile('image') && config('app.uploading_enabled')) {
        Image::make($request->file('image'))
            ->resize(300, null, function ($constraint) {
                $constraint->aspectRatio();
            })
            ->save($public_path('images/models') . DIRECTORY_SEPARATOR . $model->id . '.jpg');
        }
    }

    return redirect('/cars/models')->with('message', 'Модель успешно добавлена');
}

2

Следующим шагом будет вынос бизнес логики в сервис-класс. В данном случае, мы вынесем работу с изображением:

class ModelService
{
    public function handleUploadedImage(Request $request, $modelId)
    {
        if ($request->hasFile('image') && config('app.uploading_enabled')) {
            Image::make($request->file('image'))
                 ->resize(300, null, function ($constraint) {
                     $constraint->aspectRatio();
                 })
                 ->save($public_path('images/models') . DIRECTORY_SEPARATOR . $model->id . '.jpg');
        }
    }
}

Используя контейнер Laravel, мы внедрим и будем использовать сервис-класс в контроллере, после чего последний будет выглядеть так:

protected $model;

protected $modelService;

public function __construct(Model $model, ModelService $modelService)
{
    $this->model = $model;

    $this->modelService = $modelService;
}

public function store(ModelRequest $request)
{
    if (auth()->guest() || !auth()->user()->hasRole('moderator') || auth()->user()->canAddContent()) {
        return redirect('/cars/models')->with('error', 'У вас нет прав для добавления новой модели');
    }

    $model = $this->model->create($request->all());

    $this->modelService->handleUploadedImage($model->id); // Обратите внимание на эту строку.

    return redirect('/cars/models')->with('message', 'Модель успешно добавлена');
}

3

Метод handleUploadedImage также выполняет слишком много задач. Следуя SRP, нам необходимо вынести часть логики в отдельные методы сервис-класса:

class modelService
{
    public function handleUploadedImage($modelId)
    {
        if ($this->canHandleImage()) {
            Image::make(request()->file('image'))
                 ->resize(300, null, function ($constraint) {
                     $constraint->aspectRatio();
                 })
                 ->save($this->getImagePath($modelId));
        }
    }

    protected function canHandleImage()
    {
        return request()->hasFile('image') && config('app.uploading_enabled');
    }

    protected function getImagePath($modelId)
    {
        return public_path('images/models') . DIRECTORY_SEPARATOR . $modelId . '.jpg';
    }
}

4

Должен ли контроллер делать проверку прав пользователей? Конечно же нет. Поэтому перенесем эту логику туда, где ей самое место: в политиках.

class ModelPolicy
{
    public function create(User $user)
    {
        return isModerator() && auth()->user()->canAddContent();
    }
}

Вызывать логику авторизации возможно либо из файла маршрутов, либо из контроллера:

$this->authorize('create', Model::class);

5

В оригинальном коде мы имели два сообщения об ошибках и одно сообщение об успешном выполнении метода. После рефакторинга о сообщениях об ошибках будет заботиться уже Request-класс. Контроллер "не должен знать" о конкретном тексте, поэтому давайте также вынесем последнее сообщение в языковой файл. Теперь наш контроллер выглядит так:

public function store(ModelRequest $request)
{
    $this->authorize('create', Model::class);

    $model = $this->model->create($request->all());

    $this->modelService->handleUploadedImage($model->id);

    return redirect('/cars/models')->with('message', __('car.model_added'));
}

6

О чем еще не должен знать контроллер? Правильно, о конкретном URI, который может быть изменен в любое время. Вместо него мы будем использовать название маршрута.

После всех модификаций, наш контроллер делает только то, что должен и вообще, выглядит просто шикарно:

public function store(ModelRequest $request)
{
    $this->authorize('create', Model::class);

    $model = $this->model->create($request->all());

    $this->modelService->handleUploadedImage($model->id);

    return redirect()->route('models.index')->with('message', __('car.model_added'));
}

Вот и все. Теперь, если клиент попросит нас внести какие-либо изменения в действующий функционал, мы будем уверены, что сделаем это быстро и ничего при этом не сломаем.

Если хотите увидеть больше подобных статей, ставьте звезды в репозитории хороших практик Laravel. Это показывает есть ли интерес к этой теме, а также здорово мотивирует автора на написание других полезных материалов.

Обсуждение хороших практик Laravel

Как вы считаете, полезен ли этот материал? Да Нет

Комментарии (43)

covobo

Рефактор получился не плохим.

Добавлю свое представление: метод handleUploadedImage следует видоизменить, передавать ему не Request, а непосредственно File (иначе как можно переиспользовать этот метод?).
А при config('app.uploading_enabled') == false, метод handleUploadedImage должен выкидывать Exception (ибо метод был вызван, но выполниться он не может, это как-то странно, что он молча ничего не сделает).

AlexeyMezenin

Спасибо за дополнение.

Переиспользовать метод можно, ведь в любом случае этот метод зависит от данных в объекте Request. Тестируется это тоже отлично. Плюс же заключается в том, что мы не передает множество данных, а просто внедряем Request. Если бы мы работали с моделью или другими классами, там да, нужно передавать непосредственно данные (массив, создаваемый $request->all()), хотя многие передают объект Request.

На счет исключения, я стараюсь использовать их только там, где они действительно необходимы. В данном случае нам не нужно знать есть ли там вообще изображение или нет. В более сложном случае подход, описанный тобой, безусловно выглядит лучше.

Если у тебя есть дополнения к описанным практикам Laravel, буду благодарен, если поделишься ими здесь

Proger_XP

Отличная статья, и написана хорошо (только в заголовках не принято ставить точки в конце, лучше это исправить).

Однако раз текст претендует на «beyond junior», т.е. на уровень повыше начинающего, то надо предупредить о том, что чрезмерное размазывание кода по разным сущностям — это такое же зло, как и пихание всего в один класс (контроллер в данном случае).

К сожалению, известные мне фреймворки поощряют и то, и другое, причём сложно сказать, какое из зол меньше — при размазанном коде много времени тратится на поиск того, где все-таки выполняется код (чем грешит и сам Laravel со своей тучей фасадов и IoC).

Как всегда, никакие практики и принципы не заменяют собственной головы на плечах.

AlexeyMezenin

Спасибо, точки убрал.

Не уверен на счет размазывания кода, не думаю, что видел такое в приложениях. Наоборот, весь код обычно в контроллерах. В моем примере присутствует размазывание?

Ed

Мне кажется вы не замечаете главной причины возникновения проблемы плохик практик. Возможно и я заблуждаюсь, но как настоящий новичек (php «со словарем» → сделал себе блог; не теряюсь в html и css) выскажу свое мнение.

Прочитав всю документацию по ларавел на русском, а затем и на английском для последней версии, плюс еще с десяток статей, мало что понял. Общую картинку получил, но как начать что-то делать все же было не ясно. В итоге вернулся к проверенному способу дающему результат «делай как я», а именно смотря на ютубе ролики от пары авторов начал писать блог, а затем менеджер проектов. К сожалению их уроки еще в процессе съемок, так что дальше приходится самостоятельно.

Так вот проблема в том, что как оказалось, эти уроки и учат плохим практикам. Желая писать правильно прочел несколько статей подобной вашей. Но! опять мало что понял(( Т.е. явно не для новичков материал. Для новичков другой подход нужен. Хоть многие и смеются «очередной блог на ларавел», но это именно то, на чем учатся!

Когда дело доходит до понимания того, что пишите вы, уже поздно. Уже привык писать не правильно. Нужно переучиваться, а это всегда проблематично, ведь проще и быстрее написать как знаешь.

Сделайте хоть кто-нибудь полный урок, начиная с кода «как правильно создать запись в БД на ларавел», а попутно придется объяснить и про политики и про сервис котроллеры.

AlexeyMezenin

Я всерьез думал об этом когда ко мне обратилась менеджер Packt с предложением написать самоучитель по Laravel. Много думал и пришел к выводу, что писать очередную книгу смысла нет, а написать книгу для новичков и сразу по ходу объяснять «как правильно делать» ну очень сложно. Человек так устроен, что он не может впитать слишком много информации за короткий промежуток времени, поэтому лучше всего начать обучение с обычной книги для новичков или видеокастов, а уже потом расти. Для меня этот продход работает.

А переучиться не так сложно. Все мы когда-то писали полную жесть и будущие мы будут считать, что сейчас мы тоже пишем убогий код. Мы все время растем, это нормально.

Ed

Убедили, буду продолжать) Благо в гугле почти все можно найти. Хотя с хомстедом промучался долго, несмотря на кучу мануалов по его установке. А еще с Mix, тоже было не просто, т.к. упорно не ставился npm на хомстеде, в итоге стал с ошибками, но скомпилил таки css.

С php было проще, общую логику понял и пишу себе поглядывая в гугле как делается очередной необходимый кусочек функционала. Тут же слишком много всего, куча пакетов с кучей версий, которые не так просто подключить (как было с mix), а главное еще нужно понять какой для чего реально нужен. Много статей прочитал для начинающих, и во многих сказано: «ставим это и то и еще это», а вот для чего не сказано. И в итоге, чтобы запилить «хелло ворлд» папка с проектом вырастает до 200Мб непонятно чего))
Может не учебник, а хотябы упорядоченый, толковый и желательно актуальный (под 5.5) список ссылок на готовые статьи по теме соберете?) Начиная с первой про среду разработки и разбор базовых и реально необходимых доп.пакетов... А то ведь найти можно все, но отличить полезное от вредного в найденном новичку не под силу обычно.

(если честно я не знаю зачем вам тратить на это время, но раз уж вы подобное делаете, я готов нагло воспользоваться)))

AlexeyMezenin

Спасибо за идею, если будет время, займусь. А пока, вот такой вот репозиторий есть.

Adobe

Я как-то имел неосторожность подобную тему затронуть в группе laravel, меня чуть говном не закидали за такую дерзость. А что вы имеете против html кода в контроллерах? А может вы еще и от AR отступились? Барбара Лисков, кто она вообще такая и что она себе позволяет.

AlexeyMezenin

Понимаю, тоже часто с подобным отношением сталкиваюсь. Не все понимают важность использования хороших практик для создания поддерживаемых приложений. Если есть дополнения к практикам в репозитории, буду рад о них услышать. )

Proger_XP
  1. А может вы еще и от AR отступились?

Мне кажется, вы вообще коммунист.

Падение уровня программистов — неизбежное следствие снижения порога входа. «Попсовый Laravel», так сказать.

kaa

exists:makes.id должна быть запятая exists:makes,id

AlexeyMezenin

Спасибо, исправил.

BAHzer

Институтские истины на самом деле.

AlexeyMezenin

Учитывая то, что практически все реальные приложения пишутся игнорируя описанное в статье, не такие уж и «истины».

eorlyans

Добрый!

После четвертого шага должно было появиться $this->authorize('create', Model::class); ?

Это просто опечатка? Или политика как-то хитро привязана к модели/контроллеру и ничего дополнительно делать не надо? Или это делается где-то в другом месте?

AlexeyMezenin

Пропустил этот момент. Добавил в статью. Спасибо большое.

Silm

Замечательная статья, большое спасибо!

Плохо понял второй шаг, точнее то что касается сервисных классов. Можете пояснить концепцию сервисных классов? В чем необходимость выносить логику именно в них и какую именно логику надо выносить в них, а не в модель?

Я обычно стремлюсь помещать всю логику обработки данных в модель, то есть, например запрос на добавление файла: после валидации контроллер дергает метод модели отвечающий за создание новой сущности и передает в нее нужную информацию из реквеста. Модель же уже дергает свои методы, например для ресайза изображения и сохранения нужным образом файла, пишет запись в бд и тд.

Можете посоветовать структуру файлов и директорий проекта? Я по этому поводу нахожусь в большом замешательстве и в основном сваливаю все что отвечает за логику работы с данными в файлы в app\Models\

AlexeyMezenin

Все, что связано с данными, лучше хранить в моделях. Если есть преобразование данных, тогда Eloquent Resource классы (в 5.5+), либо отедльные классы для преобразования.

Все остальное хранить лучше в классах с бизнес логикой и сервисах, в зависимости от архитектуры. На счет структуры папок, я использую стандартную. Сервис классы в app\Services, модели в app\.

Silm

Спасибо, за ответ.

Пока не понял что надо выносить в сервис и почему не в модель. Вот, в шаге 2, метод handleUploadedImage, он разве не связан с данными?

AlexeyMezenin

Данные — это БД в большинстве случаев.

Silm

Я хочу сказать, что обычно при загрузке изображения всё равно надо сделать запись о нем в БД. Путь и имя файла, возможно еще какие то данные об изображении, чтобы потом можно было манипулировать этим.

Вот если, скажем, есть таблица images, в ней хранится путь до файла, тайтл, теги, пути до миниатюр и тп. Получается что создание записи в таблице никак не может происходить отдельно от загрузки файла, создания миниатюр и прочих преобразований.

То есть должен быть какой то 1 метод, который отвечает сразу за создание записи и сохранением изображения, который должен вызываться в конструкторе. Иначе получается что есть 2 публичных метода, которые не могут использоваться по отдельности и всегда должны вызываться последовательно.

Вот я всё никак не могу придумать как это должно быть организовано...

Abraham

Метод сохранения файла вернет имя файла, если он сохранился без проблем и его надо созранять в базу уже. Если модель не сохранилась в базу то файл желательно удалить что бы не засорять место на диске неиспользуемыми файлами. Да и путь незачем в базе хранить. Путь должен быть где то в конфиге, в базе только имя файла.

Abraham

Данные — это данные, не обязательно база данных, хоть в текстовый файл можно писать, в том то и суть mvc что контроллеру все равно что там делает модель с данными, сохраняет их в MySQL, MongoDB, Redis, или в текстовый файл. Это уже в Ларавеле совместили модель с ORM и потому многие стали думать что модель и ORM єто одно и то же.

Silm

Что насчет такого подхода:
Модели описывают данные: типы, сеттеры, геттеры, логику изменения состояния объекта, скоупы.
А манипуляции вроде создания новой записи и тп производятся через сервисные классы.

То есть, например, меняем:

PHP
$model $this->model->create($request->all());
$this->modelService->handleUploadedImage($model->id);

на

PHP
$this->modelService->createModel($request->all());

Сервис уже обращается к модели и к своим закрытым методам для сохранения файла, возвращает созданную модель. Тогда получается есть конкретный метод для создания полноценной сущности вместе с файлом. Также тогда не понадобится модель внедрять в контроллер.

Как считаешь?
Или быть может правильнее иметь createModel в самой модели, а уже она будет запускать сервисный класс?

VolCh

По хорошему контроллер вообще не должен работать напрямую с моделью, а только с сервисами. Его основная ответственность — адаптировать параметры своего вызова в вызов сервиса, а результат от сервиса в свой результат. Модель должна дергаться уже в сервисе.

Что должно быть в самом сервисе, а что сервисом делегироваться в модель (сущности) вопрос не простой в общем случае, но основной принцип — у модели не должно быть зависимостей, кроме других моделей и чистых функций типа стандартных функций математики или работы со строками/массивами. Прежде всего не должно быть зависимостей от инфраструктуры типа файловой системы, внешних апи, переменных типа $_FILES или оберток для них типа request().

Silm

Спасибо, очень полезно.

Abraham

Вот эти политики на мой взгляд очень неудобные и громоздкие. В статье по ссылке рекомендуют использовать их а не, например, Entrust. Однако почему не обьясняется. В Entrust есть готовый middleware, который можно добавлять к роутам и группам роутов, и делается это намного проще, а тут так все запутанно. Писать кучу методов вручную, в разных файлов если можно взять готовую библиотеку. Более того, Entrust ещё содержит такую штуку как permissions, что позволяет пользователю самому определять что разрешено для конкретной роли. Это удобно для всяких CRM, где админ будет решать каким ролям пользователей что можно делать. Как сделать такую фичу с политиками без костылей и велосипедов непонятно

AlexeyMezenin

Ты не совсем понимаешь что такое политики, поэтому рекомендую начать знакомство с ними с документации.

Почему следует использовать политики вместо сторонних решений? Потому что это стандартный инструмент фреймворка и стоимость разработки и поддержки при этом на порядок ниже.

Ну и один камень в огород overkill решений вроде Entrust - это проблемы с быстродействием (N+1, например), баги и более сложная (часто нечитаемая) итоговая логика. Эти проблемы возникают из-за неумения новичков обращаться с пакетами и из-за непонимания как именно они работают "под капотом". А именно новички, прочитав очередной туториал, начинают использовать этот пакет в каждом своем приложении, даже там, где есть всего две роли - пользователь и админ и нет необходимости динамически назначать разрешения.

Abraham

Документация не описывает что это и как работает, она на уровне «зацените какая в ларавеле фича есть».
Стоимость разработки и поддержки ниже если надо пляски с бубнами?
Ну например нам не надо назначить ролям разрешения. Но все равно политики основываются на хардкоде. А если потом понадобится возможность редактировать роли то придется писать велосипед. Вы предлагаете велосипеды писать вместо того чтоб использовать готовое решение.

Abraham

И какая там более сложная нечитаемая логика?

PHP
    public function handle($requestClosure $next$roles)
    {
        if (!
is_array($roles)) {
            
$roles explode(self::DELIMITER$roles);
        }

        if (
$this->auth->guest() || !$request->user()->hasRole($roles)) {
            
abort(403);
        }

        return 
$next($request);
    }

Тут же все элементарно.

Abraham

А вот ещё одна «хорошая» практика:

Хорошо:

PHP
public function __construct(User $user)
{
    
$this->user $user;
}

....

$this->user->create($request->all());

Что если понадобится два экземпляра User в каком то методе? Создавать две переменные? А если в одном методе надо один User, в другом два в третьем коллекция? А так на самом деле чаще всего и бывает. А еще может понадобиться пару других моделей. И непонятно в чем преимущество. В том что бы создать кучу экземпляров разных классов, при том что они возможно никога не понадобяться?

Ну и другие советы там есть — использовать только Blade, при том что в нем некоторых фич других шаблонизаторов просто нет. Нет того что есть даже в том же Smarty.
Совет не работать с веб сокетами вручную а использовать Laravel Echo, Pusher — но это по сути набор готовых фич, для по настоящему сложной логики они не подойдут.
Ну и совет не использовать MongoDb — то есть если в каком то случае она будет выгоднее, то все равно лучше пожертвовать преимуществами ради «хороших практик».

Silm

Я вот пока не понял для чего внедрять пустой экземпляр в класс контроллера.

Обычно внедряются прямо в метод

PHP
public function action(User $firstUserUser $secondUser)
{
    ...
}
Abraham

Ну так вот в примере показано — типа вставляйте в конструктор. Из этой строчки PHP$this->user $user; видно что надо ещё и атрибуты обьявить

VolCh

Тут он исполняет роль некоего подобия репозитория, а не представляет собой конкретного пользователя, предназначен для использования методов типа $user = $this->user->find($request->get('user_id')), $activeUserQuery = $this->users->newQuery()->ofActive() и т. п. По хорошему свойство и параметр $users надо назвать.

В конструктор внедряется репозиторий, а в методы конкретные пользователи.

VolCh

Правильное направление. Но полностью до SRP ещё далеко. Например, наличие в сервисе вызовов \request() говорит об отвественности сервиса парсить суперглобалы. Да ещё с помощью не очень явной зависимости.

iLegionXS

Здравствуйте, автор статьи. Прочитав ее, в самом начале я понял, что вы критикуете фреймворк из-за того что разработчики основную часть кода рекомендуют писать в контроллерах, это их дело. Но вы забываете, что Laravel основан на MVC, но он не обязан придерживаться всех его правил. Если разработчики решили что основной код должен находиться в контроллерах значит на это была причина. И да, это тоже довольно удобно при дебаге. Так же хочу сказать, что большая часть кода написана за вас, и если у вас получаються «жирные» контроллеры, то не стоит задуматься об их разбиении?

andrey_sunday

Разработчики Laravel ничего не рекомендуют, они просто демонстрируют возможности фреймворка. В их задачи не входит обучение принципам ООП, если вы решили использовать фреймворк, то предполагается, что вы хорошо знаете PHP7, а также принципы и паттерны ООП или работаете под руководством сильного программиста. Статья хорошая, в том плане, что показывает конкретный пример применения одного из принципов ООП, что должно побудить мыслящего начинающего, программиста изучать и применять наработанные годами хорошие практики. Кроме того, надо понимать, что в рамках какого-либо принципа, может существовать множество практических реализаций с разной глубиной и степенью детализации в зависимости от того, что вы разрабатываете. Претензии к фреймворку, что он плодит плохих программистов, сродни тому, что Google translator плодит плохих переводчиков полиглотов.

dimablack

При создании ModelRequest-класс, помимо валидации, там же нужно и реализовать проверку прав пользователей, и тогда наш реквест, добавить

return auth()->user()->can('create', Model::class);

в метод

public function authorize()

будет выглядеть так:

class ModelRequest extends FormRequest
{
    public function authorize()
    {
        return auth()->user()->can('create', Model::class);  //проверка прав пользовтеля перед валидацией
    }

    public function rules()

    {
        return [
            'title' => 'required|max:255',
            'content' => 'required',
            'make_id' => 'required|exists:makes,id'
        ];
    }
}

и с контролерра, тогода убираем

$this->authorize('create', Model::class);

и тогда у нас вконце поличится вот так:

public function store(ModelRequest $request)
{
    $model = $this->model->create($request->all());

    $this->modelService->handleUploadedImage($model->id);

    return redirect()->route('models.index')->with('message', __('car.model_added'));
}

помимо чистого кода и тонкого контролера, делать это нужно для оптимизации, чтобы сервер лишний раз не делал валидацию, а потом уже проверял, можно ли пользователю это делать или нет. И плюс представим ситуацию, а нас есть некая форма, пользователь ее заполняет, и заполняет неправильно, срабатывает валидация, пользователь пробует еще пару раз(тратит времья), дальше валидация проходит, и потом срабативаєт Policy(политика прав пользователя), и припустим что у него нет прав на даное действие и она не пройшла. В итоге, трата времени пользователя.

dimablack

для новичков и не очень, советую черпать информацию из англоязичних ресурсов, так как сообщество там намного больше, соотвественно и информации в разы больше, и есть куча видеоуроков, где на интуитивно понятном английском обясняють и показывают, что и как делать вот очень хорошие уроки

https://laracasts.com/series/laravel-6-from-scratch

ice_stranger

Не первый раз вижу попытки для передачи параметров в валидатор создавать отдельный класс. На мой взгляд этот подход усложняет читаемость кода и засоряет проект лишними классами. Подчеркну, передача параметров и процесс валидации - это разные понятия, валидация формы происходит в ларавелевском Валидаторе, поэтому передача параметров в валидатор в контроллере не нарушает принципа SRP.

Мой вариант:

public function store(Request $request)
{
    $this->authorize('create', Model::class);

    $request->validate([
         'title'     => 'required|max:255',
         'content'   => 'required',
         'make_id'   => 'required|exists:makes,id',
         'thumbnail' => 'nullable|max:255'
   ]);

   $model = Model::create(request->all());

   return redirect()->route('models.edit',['model'=> $model->id])
       ->with([
            'message' => _('car.model_added'),
        ]);
}

созданию файла тут вообще не место, передаем урл созданного файла

Rasjin

Не понял следующий момент.

Изначально в коде указано:
$make = Make::find($request->make_id);

Далее уже откуда-то взялся кусок кода другого вида:
$make = $this->make->find($request->make_id);

А в итоге, заменяя это валидацией:
'make_id' ⇒ 'required|exists:makes,id'
вообще проверяем наличие поля id в таблице(exists:table,column), а не наличие записи c указанным id, которое проверялось в первоначальном коде.

Как это понимать?

mastershifu

Все проблемы изза того что новички не знают что в приложении может быть несколько точек входа, например REST или консоль.
Вот и пишут всю логику в веб контроллере.
А когда придется написать еще API для какого нибудь мобилного приложения всю бизнес логику в веб контроллере надо будет скопипастить и в REST контроллеры.
Можете подумать «скопировать несколько строк? пф, проще простого». Но проблемо то не в копировании.
Проблемы начнутса когда современем бизнес логика начинает меняться и вам придется во всех точках доступа менять ваш код.

Написать комментарий

Разметка: ? ?

Авторизуйся, чтобы прокомментировать.