Чистый код. №2: Чистим функции. Теория и практика. По книге Роберта Мартина

[@viper_vlad]

Да, хорошо получилось. Но если мы говорим про Ларавел, то есть пару моментов: 0. Ты не прав, мы можем кидать exception, сохранив интерфейс API. 1. Твой try $params = $request->validated() не отработает должным образом, т.к. метод validated возвращает список отвалидированных данных. А сама валидация происходит до вызова метода контроллера. И соответсвенно, клиенты нашего API в случае ошибки будут получать 422, вместо ожидаемого 200. Если во всем проекте используется одинаковый формат вывода ошибок валидации данных (200 HTTP-код, плюс "resultCode" в body), то самый лучший вариант - переопределить глобально формат ответа в случае Illuminate\Validation\ValidationException. В Ларавел это делается довольно легко. И тогда нам не нужно будет в каждом контроллере отдельно json ответ писать с "resultCode" в случае ошибки валидации 2. Метод checkIfProductExists тоже не нужен. Проверку на уникальность следует вынести в CreateCatalogProductRequest. Для такой проверки в Laravel есть правило unique. Т.е., что-то типа 'product_id' => 'required|unique:products' и 'sku' => 'required|unique:products' 3. persistProductInDB очень плохо получился. В том плане, что он возвращает массив с ответом. Почему функция, которая отвечает за сохранение данных в БД, взялась формировать ответ для респонса? persistProductInDB должна возвращать bool, т.е., должно быть return $newProduct->save(). А уже в createCatalogProductAction мы проверяем и формируем array для респонса. А вообще, на мой взгляд, нужно очень осторожно относится к возврату результата в array. Если в рамках одного класса такой подоход еще более менее норм, но если ты будешь возвращать какой-то array из каких-то сторонних классов, то может произойти что угодно. Т.к., у массива нет четкой структуры и ты не можешь быть уверен, какие данные хранятся в массиве, который ты получил. А каждый раз проверять структура массива тоже не вариант. 4. Насколько верно называть методы внутри контроллера, используя постфикс "Action"? Ведь с точки зрения роутера в ларавел, метод createCatalogProduct это же тоже action laravel.com/api/9.x/Illuminate/Routing/Route.html

[@freelancer_eyes]

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

[@rm-rf88]

А смысл resultCode при наличии и так кодов самого HTTP? Если 200 вернулся - ну максимум в body что-то типа "message" => "OK", а если не 200-ка, то "error" => "message_error", плюс просто указать код 422/400/500

[@viper_vlad]

@@rm-rf88 при рефакторинге ты не меняешь работу кода (в том числе интерфейс API). Твой API используют клиенты. Ты не можешь просто так взять и поменять интерфейс API, не уведомив клиентов. А если клиент API - мобильное приложение, например, то тогда тебе ещё нужно подумать об обратной совместимости. А это уже выходит за рамки рефактиронга.

[@rm-rf88]

@@viper_vlad Да я не до конца досмотрел. Я в общем думал, когда писал пост. Не стал уж исправлять просто. Ну а в целом, если это апи, то делается v2 и там уже по уму. А на v1 остается ответ как есть. Ну это так.

[@ТимофейХайлайн]

На 6:20 автор сказал, что первый принцип солид - это что функция должна выполнять только одну четко оговоренную в имени функции операция, так вот в книги Чистая Архитектура от Мартина в 7 главе 79 странице написано, что это распространенное заблуждение, а а singe responsibility принцип говорит о том, что «модуль должен иметь одна и только одну причину для изменения»

[@ВладимирМороз-в9б]

Замечательное видео. Очень жду продолжения.

[@freelancer_eyes]

Следующий ролик уже в духовке. Выпекается :)

[@yashkevich8164]

Весьма полезное видео. Жду Тестов теперь на это дело)

[@freelancer_eyes]

Спасибо на добром слове. Рад, что видео оказалось полезным. Честно говоря, писать тесты на этот конкретно код я не буду. Его бы по-хорошему снести и заново переписать, не обязательно с Laravel. Но обязательно с предварительным покрытием тестами :)

[@ihorrud5088]

Видео огонь! Чем дальше в лес, тем больше партизанов)))

[@freelancer_eyes]

И тем они жирнее!

[@pep421]

Спасибо за видео. Где же вы были раньше. Подача материала божественна. Вы учитель от бога.

[@freelancer_eyes]

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

[@pandalove6795]

С exception и json ответом, можно поступить так. Написать функцию обертку, которая будет в try catch вызывать основную функцию, и если все хорошо, то из функции получать сообщение и устанавливать resultCode, а иначе в catch получать ошибку и возвращать resultCode 1 и сообщение из ошибки. А уже после того, как с фронтом договоритесь, можно будет легко убрать эту обертку.

[@views-lu6ph]

Большое спасибо за Ваш канал! Круто! Было бы здорово, чтобы вы делали трансляции, когда кодите реальный проект делаете + давали бы краткие комментарии. Настоящего кодинга на ютубе мало!! Еще Вы обещали когда-то раньше, если я правильно понял, показать как делаете рефакторинг!

[@freelancer_eyes]

Спасибо за разговор в чате во время премьеры! Лайвкодинг на реальном проекте вряд ли получится. По крайней мере сейчас. Работа очень много времени занимает, а оттуда показать ничего не получится. Но как только появится идея, которую можно закодить на абстрактном уровне, а потом перетащить в рабочий проект - я попробую снять видео. Хотя это тяжеловато мне пока - кодить во время съёмки. Я же начинающий влогер.

[@user-ms5pc2vj8u]

Спасибо за материал! Было очень интересно послушать!

[@freelancer_eyes]

Благодарю за поддержку! Рад быть полезным.

[@deen812x]

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

[@freelancer_eyes]

Спасибо за поддержку. Рад быть полезным!

[@vitaliy0192]

Видео понравилось, буду смотреть еще. Но явно перепутаны понятия уровня абстракции и функционал. Если этот момент понимать, то смотрится хорошо. Лайк, коммент, колокольчик.

[@dmitryalinsky5911]

На счёт практического примера. Но сразу скажу, что посмотрел только исходную функцию и результат. По этому может вы и сами озвучили такой вариант в процессе. Я недавно столкнулся не с рефакторингом, а с интеграцией одной платёжной системы и у них в API был тот же подход с ожидание статуса 200, но с разными кодами ответа и сообщениями. Немного подумав, я решил вынести все варианты ошибок в отдельный класс и унаследовать его от \Exception. Вышло примерно так (названия специально изменил): class AnyPaymentException extends \Exception { public function __construct(public array $error) { } public const ERROR_1 = [ 'code' => -1, 'message' => 'Ошибка 1', ]; public const ERROR_2 = [ 'code' => -2, 'message' => 'Ошибка 2', ]; public const ERROR_3 = [ 'code' => -3, 'message' => 'Ошибка 3', ]; } ------------------------------------------------------------------------------------------------- В сервисе по обработке запроса я сделал примерно как и вы, раскидав все проверки по отдельным функциям. В случае проблемы, выкидывал AnyPaymentException, передавая соответствующую константу. А в экшене контроллера я сделал try - catch блок, и в случае ошибки, выполнял роллбэк транзакции и возвращал $exception->error со статусом 200.

[@freelancer_eyes]

Вполне здравое решение в этой ситуации. Мы же не можем изменить поведение стороннего API. Раз они работают в таком стиле - нам приходится под них подстраиваться. Но внутри своего кода вы, похоже, сделали всё возможное, чтобы код оставался красивым и эффективным. Супер.

[@shiryshev]

Было полезно, спасибо! Час пролетел незаметно.

[@freelancer_eyes]

А что случилось? С каких пор чистка функций стала для тебя полезным занятием? 😱🤔

[@shiryshev]

@@freelancer_eyes Гигиена прежде всего! Я чищу свои функции минимум раз в день, а то и два :)

[@romanchubich2013]

С именованием уже давно страдаю. В одном проекте у меня база с именами вида item_id. В коде я тоже сделал $item_id. В другом исправил имена полей в бд на itemId, чтобы в коде было эстетичнее. До сих пор маюсь между: ведь в бд item_id лучше выглядит. А если в бд и коде разные имена, так вообще с ума можно сойти.

[@freelancer_eyes]

Тут такое дело - нужно смотреть стандарты, принятые в языках, фреймворках, средах. К несчастью, PHP PSR (1 и 12) этого вопроса не проясняют: This guide intentionally avoids any recommendation regarding the use of $StudlyCaps, $camelCase, or $under_score property names. Всё, говорят, можно, как хотите. Лишь бы одинаково на уровне модуля, приложения, библиотеки, и т.п. В настоящее время фактическим стандартом имён PHP переменных стал $camelCase. Вот его я бы и рекомендовал использовать, стиснув зубы :) При этом в базах данных under_score действительно удобнее, нагляднее и т.п. Ну, и ничего страшного! Если у вас в коде встретится $fooBar = select foo_bar (простите за псевдокод), то вы будете чётко видеть, что эти имена - из разных миров как бы. И вообще говоря, если вы работаете в PHP-коде с именами полей базы данных, то скорее всего это происходит в каком-то репозитории или другом специальном месте. То есть, имена полей БД не рассыпаны по всему коду и не "мозолят глаза" :)

[@romanchubich2013]

@@freelancer_eyes Кстати, только сейчас я понял, что именовать в виде item_id заставляет шрифт, в котором I (заглавная i) и l (строчная L) выглядят одинаково. Например, "resultItemId" выглядит совершенно нечитабельно. Поменял шрифт в системе и имена в Navicat заиграли новыми красками. Осталось найти где в PhpStorm настраивается шрифт той области, где отображается бд. Тогда можно будет обозвать поля бд в стиле camelCase и обрести счастье.:)

[@arta4649]

Вижу страдания на протяжении всего видео. Вам скорее проще было б переписать весь проект, чем рефачить этот код))) Но как всегда познавательно, спасибо!

[@freelancer_eyes]

Страдания в основном от другого происходили. Меня прерывали постоянно. То собаки под окнами начинали драться, то приходил человек с газонокосилкой, то ещё что-нибудь в таком духе. От этого мысль скакала, я останавливался и начинал заново, а в голове - мешанина, что я уже успел рассказать, а что нет :) В общем, непросто дался ролик. А вот насчёт «проще переписать с нуля» - это всё-таки миф. Каким бы ни был код, если он работающий, его всегда проще зарефачить. Только сначала тестами покрыть :)

[@arta4649]

@@freelancer_eyes Да,на 56 минуте собакен Вас поддержал!:)

[@freelancer_eyes]

@@arta4649 не, это они к следующему раунду готовились. Может, там лювак по бананам бегал (у нас банановая ферма за забором). Но что-то они злобствовали, да :)

[@NuChamu]

Работал на маркетплейсе и бывали одинаковые SKU для разных товаров от разных поставщиков

[@freelancer_eyes]

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

[@compolomus9719]

а самое главное, чтоб протестировать такой плохой код, придется делать кучу тестов, на каждое ветвление и прочее

[@freelancer_eyes]

Да! Совершенно верно подмечено. Я хотел специально об этом упомянуть, но увлёкся и позабыл. Спасибо огромное за уточнение!

[@ACLeo_UA]

В целом все хорошо и правильно, НО! С возвратом null плохо. Мартин вроде писал, НИКОГДА не возвращать null.

[@freelancer_eyes]

Возвращение null хорошо укладывается в логику типизации в текущем PHP, к примеру. То есть, если мы можем сказать public function fake(): ?Foo то совершенно понятно, как с этим работать без каких-либо сайд-эффектов. Например, если мы ищем чего-то с помощью ORM::find($id) и не нашли искомое, то что нам возвращать? False? Void? Или выбрасывать exception? Но exception может быть здесь неуместен. А возвращение null поддерживается типизацией, IDE, и ни к каким неожиданным результатам не приведёт. Я так думаю (с)

[@fedorovrs]

@@freelancer_eyes вот я тоже совсем не оценил этого совета "не возвращать null". В php он везде, он удобен и понятен. И слава богу тут нет еще и undefined!

[@Фанат-щ9ь]

Эх научится бы работать с контроллерами и тому подобное, а вообще было бы интересно создать пример на php с расчётом разных оплат работников

[@freelancer_eyes]

А какая у вас проблема в плане работы с контроллерами? Что там вызывает трудности?

[@Фанат-щ9ь]

@@freelancer_eyes Здравствуйте. У меня нет чёткого понимания как работать с ООП это первая проблема а вторая это паттерны, про структуры я вообще молчу)

[@freelancer_eyes]

А работать хотелось бы с каким-то фреймворком типа Laravel? Зреет у меня идея сделать курс всё-таки по Laravel, базовый такой. Надо спросить у сообщества - будет ли такое интересно. А вам лично будет интересно?

[@Фанат-щ9ь]

@@freelancer_eyes Мне лично любая тема на вашем канале интересна ибо она помогает развиваться как разработчику в том числе будет занимательно посмотреть обзор на то как вы понимаете laravel.

[@shaurma8742]

Вы на PHP чистый код пишите? Это как чистить футболку, в чане с г@вном.

[@ЛеопольдКотов-к3й]

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

[@vitmih380]

create_catElog_product() И эти люди учат правильному неймингу ))))

[@freelancer_eyes]

А вы расслышали, что это чужой код?

[@vitmih380]

@@freelancer_eyes Извините пожалуйста ( Голова посыпана пеплом в огромном количестве...

[@freelancer_eyes]

@@vitmih380 нет проблем :)