Comments 19
2. А вот пример в Вашем коде, как не нужно делать:
return response()->json([
'success' => false,
'exception' => $e->getMessage()
]);
Лучше отвечать кодом HTTP. Так же, фреймворк предоставляет для этого все нужное, используя настройки окружения (debug флаг) для более развернутой информации про ошибку
3. try-catch конструкция внутри контроллера — лишнее. Есть ExceptionHandler из коробки
4. Вот в этом месте
$orderService->createOrderFromRequest($request)
Вы рискуете получить лишние данные прямо в сервисный слой. Используйте метод "$requests->validated()"
Вы рискуете получить лишние данные прямо в сервисный слой. Используйте метод "$requests->validated()"
Да, так действительно будет гораздо лучше. Ну или определить внутри request метод, который вернет сущность (для более удобной работы) вместо массива. Спасибо за замечание)
GET /order
Для следующих случаев:
- Заказ не существует.
- Заказ не найден в рамках существующих фильтров.
GET /order
— не корректный запрос/uriGET /orders
— получить коллекцию заказов. Если в рамках существующих фильтров ничего не найдено, вернуть пустую коллекцию с кодом 200 (запрос же корректно был обработан). Как альтернатива, можно вернуть код 400, но как по мне — это уже дело вкуса…GET /order/{id}
— вернет 200, если заказ (ресурс) с таким id существует. 404, если заказ не найден, не корректный id или клиенту нельзя знать о существовании такого idGET /order/{id} — вернет 200, если заказ (ресурс) с таким id существует. 404, если заказ не найден, не корректный id или клиенту нельзя знать о существовании такого id
Полагаю что вы вообще не понимаете в чем суть проблемы отборажения бизнеслогики на статусы HTTP.
Первой много, а второго мало.
По шагам
404, если заказ не найден
Это ок.
404, если зне корректный id
Это не ок. Не пользователь должен догадаться из вашего "что-то пошло не так". А он должен получить внятную информацию. Особенно если это пользователь API.
404, если клиенту нельзя знать о существовании такого id
Это тоже не ок. Поскольку совершенно другая ситуация которая требует иной реакции.
Тут люди свой собственный пол сделали небинарным. А вы хотите завернуть всю широту возможных случаев в несколько десятков кодов…
Ваш код 404 не несет всей нужной информации. Т.е. нарушает сам REST по вашим же словам.
Как исправлять будете?
Можно ли использовать сервисы внутри сервисов?
Я бы однозначно не рекомендовал такую практику, так как этим вы нарушаете single responsibility принцип, делая ваш код, к тому же, достаточно запутанным.
Тут нет ошибки? Потому что во втором пункте вы по сути предлагаете использовать сервисы (OrderService
и NotificationService
) внутри сервиса (CreateOrderOperation
)
А всё что относится к http и валидации глубже контроллера не следует пропускать.
Почему сразу не передавать MailEventInterface
?
NotificationService::notify(MailEventInterface $event): void;
Заодно можно будет избавится от массива, а сразу передавать нужные параметры в эвент. В противном случае каждый раз надо будет заходить в реализацию makeNotificationEvent
, смотреть какой эвент должен создаться при order_created
и какой массив параметров он ожидает
public function createOrder(CreateOrderRequest $request)
{
$this->notificationService->notify(new OrderCreatedEvent($order));
}
$result = $createOrderOperation->createOrderFromRequest($request);
Это не сервисный слой, а насмешка над ним. Идея разделения на слои в том, что каждый слой отвечает за что-то свое: контроллер отвечает за взаимодействие с пользователем (прием HTTP-запросов, формирование ответов), а сервис отвечает за бизнес-логику, связанную с созданием заказа. У вас же сервис выполняет работу контроллера: анализирует HTTP-запрос и формирует HTTP-ответ.
Это не сервис, это тот же самый толстый контроллер, который вы просто назвали Operation. В то же время контроллер у вас вообще ничего не делает. Зачем он вообще нужен? Удалите контроллер и переименуйте Operation в Controller и все будет правильно.
Из-за отсутствия разделения на слои с кодом может быть неудобно работать. Если например, я захочу создать заказ из CLI скрипта: надо откуда-то взять Request (откуда?), заполнить его (чем? Это нигде не определено, откуда я узнаю, какие в нем параметры?). Если бы у вас был по настоящему отделенный от контроллера сервисный слой, вы легко могли бы создать заказ из CLI или из теста.
Можно ли использовать сервисы внутри сервисов?
Я бы однозначно не рекомендовал такую практику, так как этим вы нарушаете single responsibility принцип, делая ваш код, к тому же, достаточно запутанным.
Разве? Вот есть у нас ряд действий: создать заказ, уведомить на почту, уведомить смской,… за каждое действие отвечает свой сервис.
Теперь нам надо чтоб в каком то случае всё это произошло в определенной последовательности и чтоб мы эту операцию могли вызывать из разных мест приложения. Эта операция и становится сервисом, его область ответственности — содержать логику последовательности действий, он будет изменяться, когда мы захотим изменить набор действий, их порядок. Код станет проще, поскольку эта логика теперь лежит в одном месте и вызывающему коду о ней думать не надо.
Еще немного про сервисный слой в PHP