Оценка кода

[Redjik]

Rest
Есть контроолер/да или даже пофиг экшен, по сути у нас нет вьюх, а есть обьекты трансформеры (которые рендерят обьект в нужный формат - json, xml, whatsoever), что-то сделал с сущностью, прогнал через трансформер - отдал.
Для каждой сущности можно/нужно сделать свой трансформер (ибо у каждой сущности частенько свои правила рендеринга).

И как же тут поступить с эксепшенами, даже тот же блин 404?
В экшен/контроллер пихать трансформеры всех возможных ошибок? Да нифига!

Поэтому и кидается эксепшен, а хендлер уже отдает этот эсксепшен в нужном формате.

SRP в действии.

 

[Redjik]

@Redjik, так объединяйтесь, создайте свой мануал. Вон как react в js сообществе появился, огромное кол-во людей заинтересовалось функциональщиной.

прикол в том, что я работаю уже давно над статьей - 10 причин моей ненависти к Ларавел (которая сводится к тому, что народ просто не умеет писать код )
в итоге вышел на другую статью, которую скорее всего и опубликую
SOLID как стиль программирования в команде.
Как показывает практика, если все придерживаются SOLID - то весь код очевиден, все находится на своих местах, нет неожиданных решений.

 

[fixxxer]

10 причин моей ненависти к Ларавел (которая сводится к тому, что народ просто не умеет писать код )

Вот именно что.

Все там нормально, просто не надо на нем писать как на yii. (Eloquent говно, но его и трогать не надо. Мне вот вообще lumen-а хватает).

 

[Adelf]

@Redjik, я б тож не прочь с единомышленниками как-то приятнее. А статейку уже полгода пишешь похоже Хотя я уже представляю что там будет.

@fixxxer, а я с Eloquent работаю. Из-за него всю бизнес-логику приходится делать в Anemic стиле. Все хочу посмотреть хороший реальный пример с Rich моделью... необязательно в laravel или PHP. Думаю, мне понравится.

@grigori, ну тебе ответили уже. Твой редирект - это не исключительная ситуация, а поведение контроллера, принятое вашей командой как совершенно нормальное.

 

[Sufir]

@fixxxer, на Yii тоже можно нормально, я сейчас как раз такой эксперимент ставлю. Только всякую хрень вроде AR и Yii::$app не трогать... бороться с ним, конечно местами приходится, но в целом всё получается.

 

[grigori]

@grigori, ну тебе ответили уже. Твой редирект - это не исключительная ситуация, а поведение контроллера, принятое вашей командой как совершенно нормальное.

Еще раз для тех, кто в танке Когда ошибка зависит от входного параметра - твой error handler идет лесом, и начинается логика контроллера.
Для одного URL ошибка должна быть одна, для другого - другая.
if (не нашла) { if ($Request->isCanonicalDomain(){404}else{header('Location: /zzz/404')}} .
Пробросом исключения из модели в handler, как ты предлагаешь, такое не сделать потому что надо смотреть URL.
В read-only коде это неважно, а в долгоиграющем проекте с A/Б-тестированием доля подобных задач близка к половине.

 

[Adelf]

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

 

[grigori]

@Sufir, не переживай, это они потроллить и AR удобный, просто в части ситуаций удобнее его отделить от логики. и бороться не надо - это фреймворк, набор библиотек, а не coding guidelines

 

[Yoskaldyr]

Помоему, ты процедурно мыслишь немного. В самом простейшем случае у Exception есть message. В случаях посложнее можно ClientNotFoundException, etc. насоздавать. и там по соглашениям, ключи для локализированного вывода ошибок. Или если уж совсем не вмешивать в модель такую инфу, да - делай маппинг вроде - ClientNotFoundException -> client_not_found -> локализация сообщения и вывод. Это лишь только вариант. Возможностей много.

Под "в одном месте" я подразумевал не один метод с кучей if/else, а цепочку вызовов этого:

/**
* 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);
}

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

P.S. Мне нравится подход @Adelf-а, но у себя я могу его применить только для админок и для простых CRUD приложений, а то довольно большие участки кода будут с переизбытком try/catch

 

[Adelf]

И как поступать в ситуациях когда отсутствие сущности это норма и это только условие для разветвления логики в контроллере?

Если это нормальный flow для тебя, то делай свои if - какие проблемы. Я не за отсутствие if в контроллерах, а за SRP

И что делать если отсутствие одной и той же сущности в разных частях контроллера должно выдавать совершенно разные сообщения об ошибке?

А приведи хороший пример.

 

[Yoskaldyr]

А приведи хороший пример.

Просто из моей практики пользователям нужно полная расшифровка ошибки что именно произошло и почему именно он не может сделать что он хочет.
Например, стандартный функционал форума, перемещение сообщений между темами, если например тема получатель уже удалена, то пользователю надо вывести сообщение об ошибке:
Извините Вы не можете перенести эти сообщения, т.к. тема получатель не найдена
а не сухое
Запрашиваемая тема не найдена

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

 

[Adelf]

В этом случае из модели будут приходить разные исключения. И можно писать разные тексты.
В случае когда пользователь просто идет куда-то не туда - какой-нибудь ThreadNotFound упадет дальше, через контроллер в Event Handler.
А когда перемещают тему, код из модели, который будет перемещать тему, сделает catch(ThreadNotFound) когда будет запрашивать тему для перемещения - потому что есть бизнес-требование - отдельная осмысленная ошибка на случай, если темы, куда надо перемещать, нет. DestinationThreadNotFound какой-нибудь.

P.S. А как ты реализовываешь отдельные сообщения? Все равно получится примерно также...

 

[fixxxer]

а я с Eloquent работаю. Из-за него всю бизнес-логику приходится делать в Anemic стиле

Вообще это необязательно. Можно "забыть", что это ActiveRecord и делать вид, что работаешь с plain model, во всех местах, кроме репозиториев. Ну и with() в репозиториях агрегатов делать сразу на всё, чтобы получить полностью загруженные агрегаты. Получается что-то похожее, но все равно фигня какая-то И слишком много плясок с бубном, чтобы протестировать.

Есть, в принципе, analogue, он более-менее, но кривоват. Основной фатальный недостаток - фабрика его проксей-врапперов создается глубоко в его внутренях, и изменить поведение маппинга тех же VO из-за этого фиг получится по-простому. Проще форкнуть.

 

[Yoskaldyr]

@Adelf, Я не о бизнес логике говорю, а о тупых пользователях. Даже небольшой процент тупарей может вынести мозг как техподдержке так и программистам и это тоже время и деньги.
В общем случае мне надо своя расшифровка на каждый экшн контроллера.
Т.е. чтобы выдавало что-то такое:
Вы не можете сделать {человекочитаемое_описание_действия}, потому что {описание ошибки}

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

 

[fixxxer]

Текст ошибки можно брать из intl конфига по имени класса исключения. То, что вываливается в исключения (то есть прошло валидацию ввода) - обычно простое и однозначное, максимум одна подстановка getMessage() в формат. Ну или можно извратиться и сделать RenderableException, это, конечно, попахивает, но в каком-то случае может быть удобно.

 

[grigori]

@Yoskaldyr, у тебя получается вторичный контроллер, модель для генерации текста ошибки по результату работы модели логики и вывод из модели по шаблону. HMVC
Если копипастить классы исключений, которые будут отличаться названием, придем сначала к иерархии исключений, затем - к необходимости поменять 10 файлов из 30, а потом - к monkey patching.

 

[Yoskaldyr]

@fixxxer вот именно что попахивает
Просто если нужен полный контроль за текстом возвращаемой пользователю ошибки, то придется или использовать много лапши из try catch в контроллерах, или использовать какие-то соглашения по преобразованию текста ошибки, класса эксепшена, данных откуда он он был выкинут в человекочитаемый текст ошибки (и это очень неявно и размазывает логику по разным частям системы). Но даже в этом случае всегда будут случаи когда соглашений не хватит.

 

[Yoskaldyr]

А много лапши из try catch это ничем не лучше чем лапша из if else

 

[Yoskaldyr]

Если копипастить классы исключений, которые будут отличаться одной строкой, придем к необходимости поменять 10 исключений из 30.

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

 

[grigori]

или именованные фабрики для исключений - самих классов мало, но покрывают все варианты