Оценка кода

[Lionishy]

@AmdY, я понимаю, что здесь исключения не нужны и даже в вредны, и с @hell0w0rd вполне согласен: скорее всего в большинстве случаев достаточно даже простого кода ошибки. Но мой небогатый опыт общения с PHP программистами показывает, что почему-то они именно выкидывают исключения.

 

[Sufir]

@Sufir Ломоносов отличный пример, базой его достижений стал немецкий язык, на котором тогда даже читались лекции и публиковались, более того, русский вообще был в опале.

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

 

[grigori]

 

[Adelf]

@Lionishy, чем же они вредны? Запрашивают ресурс, которого нет. Почему это не исключительная ситуация?

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

public function postUpdate(ClientUpdateRequest $request, $id)
{
    $client = $this->service->get($id);

    $this->authorize('manage', $client);

    $this->service->update($client, $request);

    return redirect()->....;
}

Валидация формы - в классе ClientUpdateRequest.
Если по $id не найдется нужный $client - выкинется исключение.
Тоже с авторизацией доступа к изменениям клиента.
Дальше update - опять-таки если что не так - исключение.
И редирект.
А вы что? Каждый раз после вызова get client by id проверяете не null ли? Не глупо ли?

 

[grigori]

@Lionishy, чем же они вредны?

так они давно все сказали уже:

неочевидно, какой эксепшен ожидать, и лишняя строчка в стеке

в тему про одинарные кавычки быстрее

 

[hell0w0rd]

public function postUpdate(ClientUpdateRequest $request, $id)
{
    $client = $this->service->get($id);

    $this->authorize('manage', $client);

    $this->service->update($client, $request);

    return redirect()->....;
}

Ну то есть у тебя $this->service->get кидает HttpNotFoundException?

 

[Adelf]

@hell0w0rd, нет. Он кидает типа EntityNotFound. Ты ж понимаешь, что это уже модель, там о Http не знают. В 404 ответ оно в хендлере превращается.
Но вот, например, authorize может кидать какой-нибудь HttpNotAuthorizedException.

 

[grigori]

@Adelf, а что значит "В 404 ответ оно в хендлере превращается."?
Где над контроллером сидит этот хендлер? Какой-то сверх-контроллер получается.

 

[Adelf]

@grigori, он не в контроллере. Он выше там... в ларавельке стандартный exception handler.

 

[grigori]

стандартный не превратит EntityNotFound в 404, нужен свой - получается, альтернативный контроллер

 

[Adelf]

В laravel есть класс App\Exceptions\Handler который генерируется для каждого приложения при создании и регистрируется где надо как exception handler. Т.е. он как бы стандартный, но тем не менее под нашим контролем.

Соответственно, там метод:

/**
* Render an exception into an HTTP response.
*
* @param \Illuminate\Http\Request $request
* @param \Exception $e
* @return \Illuminate\Http\Response
*/
public function render($request, Exception $e)
{
return parent::render($request, $e);
}

его мы уже можем как хотим менять.

 

[Yoskaldyr]

@Adelf, а если типов entity много? Т.е. не только Client как в примере, и для всех надо выдавать осмысленное сообщение об ошибке, например, надо вывести не "Запись не найдена", а "Пользователь не найден", "Товар не найден" и т.д.? В таких случаях все перехватывать в одном месте для всех типов?

 

[hell0w0rd]

Ну вот потому мне laravel и не нравится. Это какая-то неявная хрень.

const user = service.get(req.params.id);
if (!user) {
  return new Response(404, 'User not found');
}

return new JsonResponse(user);

Из такого кода все понятно, откуда берется ошибка.

 

[grigori]

В принципе, эта неявная хрень есть во всех фреймворках. Сейчас все на соглашениях.

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

 

[Вурдалак]

@hell0w0rd, теперь представь, как твой код будет выглядеть для service.changeEmail(userId, newEmail) : void.

В таких случаях все перехватывать в одном месте для всех типов?

Ты можешь разнести этот mapping по конфигам, аннотациям.

 

[hell0w0rd]

@hell0w0rd, теперь представь, как твой код будет выглядеть для service.changeEmail(userId, newEmail) : void.

Да тут смысл не в сигнатуре метода. Мне сам подход не нравится, когда ошибки улетают куда-то за пределы контроллера. Расставляй пачку try-catch, возвращай ошибку, что больше нравится.

 

[fixxxer]

Мне сам подход не нравится, когда ошибки улетают куда-то за пределы контроллера

Потому что nodejs навернется, да?

 

[Вурдалак]

Да тут смысл не в сигнатуре метода. Мне сам подход не нравится, когда ошибки улетают куда-то за пределы контроллера. Расставляй пачку try-catch, возвращай ошибку, что больше нравится.

Ты не всегда можешь знать все возможные типы исключений на самом деле, потому что они все по большей части runtime, и в контракте не описываются. Поэтому, добавляя новое исключение, ты будешь бегать по всем контроллерам и добавлять в try .. catch очередную копипасту + ты получишь неконсистентный API, когда для одного и того же исключения будут разные ошибки.

 

[grigori]

> разнести этот mapping по конфигам, аннотациям.
а симфони умеет по аннотациям передать exception в соответствующий обработчик?

дайте ссылку на пример как это делается?

 

[hell0w0rd]

Потому что nodejs навернется, да?

не... сейчас коллбэками пользуются только бородатые, нормальные люди перешли на промисы, async-await, ошибка в любом случае останется внутри промиса и в конечном итоге перехватится в аналоге error-handler'a в ларавеле.

Но смысл такой, до error-handler'a, на мой взгляд, должны доходить ошибки, которые для клиента оборачиваются в 50x.