PHP-линч #15 • Symfony #[MapRequestPayload], часть 2

Поділитися
Вставка
  • Опубліковано 14 лис 2024

КОМЕНТАРІ • 26

  • @myks1992
    @myks1992 Рік тому +4

    Топовые стрим) Валентин скоро всех научит правильно писать и даже разработчиков Symfony. Вообще тоже раньше считал, что Symfony это лучшие практики, но со временем понял, что это не так. Код читается очень плохо во многих компонентах. Документация очень слабая и в итоге иногда по догадкам или по случайностям узнаешь, что оказывается, например, настроить тип стерилизации camelCase на snake_case можно в framework: serializer: name_converter: ‘serializer.name_converter.camel_case_to_snake_case’. Ужас) Поэтому тоже стал частично переписывать свои компоненты. Начал с Notifier. Ждём Ваш гидратор, разбор фишек DI контейнера Symfony и ещё кучу примеров качественного кода!)

  • @SiriusBlackNuar
    @SiriusBlackNuar Рік тому

    "С этой точки зрения мой код корректный, красивый и хороший ... " Класс теперь это будет мой девиз 😂

  • @solvex8304
    @solvex8304 11 місяців тому

    В денормалайзере можно задать опцию [ 'ignored_attribute => ['id'] ] и этот id задать позже. А как это провернуть в #[MapRequestPayload]?

  • @kinvain
    @kinvain Рік тому

    Пожелание. Можете в описание или прикрепленным комментарием добавлять ссылки, о которых говорите?

  • @kinvain
    @kinvain Рік тому

    22:50 а вы жёсткий... 😂

  • @kinvain
    @kinvain Рік тому

    Комментирую по ходу.
    7:10 вот это кстати очень большая проблема с атрибутами. Когда обсуждали что интерфейс можно заменить атрибутом то у интерфейса есть огромное преимущество. Можно перейти в его использование в один клик. С атрибутом придется делать два - сначала в сам атрибут, потом туда где он используется. Но возможно это беда ide.

  • @kinvain
    @kinvain Рік тому

    13:52 яп сделал бы NullValidator. Или FailValidator. Короче что то чтобы вообще убрать это if. Тестирование стало на один кейс проще.
    ЗЫ. Собсна что вы и предлагаете...

  • @KostiantynPopelnukh
    @KostiantynPopelnukh Рік тому +1

    Классное разбор! Не могу только понять, почему в симфе такой грязный код?

    • @kinvain
      @kinvain Рік тому

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

  • @djoncyber8126
    @djoncyber8126 Рік тому

    Отлично

  • @loadmore
    @loadmore Рік тому

    Чем-то я не понимаю (с) симфони )))

  • @kinvain
    @kinvain Рік тому

    17:12 нет, не можем. сложение массивов не перезаписывает ключи первого если такие же ключи есть во втором.

  • @kinvain
    @kinvain Рік тому

    15:22 когда сам смотрел то тоже не пониал зачем так. Ну хотя бы сделали их в виде invokable классов - было бы проще переходить. А так вообще не понятно. Пока докрутил до строчки где вызывается метоз из переменной уже забыл как эта переменная была определена.

  • @kinvain
    @kinvain Рік тому

    31:52 Symfony просто решили изобрести gRPC.

  • @kinvain
    @kinvain Рік тому

    19:40 который уже забыли где он был объявлен и чему равен. Нет бы просто захардкодить 400.

  • @kinvain
    @kinvain Рік тому

    Декораторы - тема. Пытаюсь команде донести что не стоит ло́житб весь функционал в один класс, а сделайте маленькие классы и соберите их в фабрике. Но нет... Главное возражение - нам непонятно что и где происходит. А в итоге меняем маленький кусочек и вся стстема сыплется.

    • @VladGapanovich
      @VladGapanovich Рік тому +2

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

    • @kinvain
      @kinvain Рік тому

      @@VladGapanovich Холивара ради, но мне кажется вы ошибаетесь. Почему декороатор - это не про декопозицию логики? Декоратор добавляет объектам новую функциональность. Так что это как раз про декомпозирование и про то что не нужно держать весь код в одном классе. Я увидел что вы написали "или небизнесовый" логики, но всё же подушню - если логика необязательная то зачем её было добавлять?
      Логер - отличный пример. Можно до кучи добавить сбор метрик. Но и бизнесовая логика не всегда должна лежать в одном классе. Конкретный пример из моего проекта, где данный паттерн был успешно применён. Дальше я буду говорить про бизенс-логику как про конкретную операцию над пользователем. Но дело в том что для того что бы эта операция была проведена нужно сделать ещё минимум два действия. Они не относятся к самой логике что мы выполянем над счётом. Но без них мы не можем работать поэтому они тоже часть бизнес-логки, просто они не имеют отношения к самому счёту.
      Наше приложение слушает сообщения от стороннего сервиса. Сообщение состоит из заголовка, самого сообщения и подписи. В зависимости от заголовка нужно выполнить разные действия - проверить доступность средств у пользователя, создать блокировку средств, отменить блокировку или же простое диагностическое сообщение (типа ping). Но для каждого типа нужно выполнить как минимум - проверку подписи полученного сообщения, основную бизнес-логику и залогировать ответ. И декоратор решает эту проблему на раз.
      Есть тонкий класс с бизнес-логикой. Относительно тонкий потому что там куча кода, но она относится только к тому что бы проверить достаточность денег на счету. Данный класс обёрнут проверкой подписи входящего сообщения, а так же установкой подписи ответа. Два последних класс переиспользуются в других бизенсовых кейсах.
      Безусловно можно было бы вынести это в отдельные сервисы и подключать их в виде зависимостей. И тогда бы каждый бизнесовый класс имел бы кусок кода для проверки, блок бизнес-логики, кусок для подписи. И если нужно поправить пред- и пост- условия то пришлобы править все бизнесовые классы.
      Безусловно можно было бы сделать абстактный класс, который бы требовал сервисы для проверки и подписи, а так же требовал реализацию бизнес-метода. И тогда бы бизенсовые классы переопределяли конструктор своими зависимостями.
      Можно вообще всё в index.php положить и сказать что это и есть принцип единой ответсвенности так как существует только одна причина внесения изменений - если поменялись бизнес-требования для обработки сообщений.
      Но моё сугубо лично имхо, декораторами не стоит лечить всё подряд, но они позволяют существенно упросить понимание кода. И "убрать мусор под ковёр" скорее про то случай когда в класс FundsAvailableCheck будут положены сервисы для валидации подписи, для валидации самого сообщения, для проверки счетов пользователя, логирования входа и выхода, подпись исходящего сообщения, логирование фатальных ошибок.
      Когда сейчас мне говорят - не работает подпись. То я иду в конкретный класс и смотрю что там может быть не так. Если у пользователя не заблокировались денежные средства то это другой класс, который делает именно это - блокировку денег. А если придут и скажут нужно каждое полученное сообщение сохранять в виде отдельного файла на Amazon S3 то я сделаю отдельный класс, который именно это и будет делать и оберну им свои сервисы. Сами же сервисы останутся нетронутыми потому что для отмены блокировки, проверки баланса или блокирования средств совершенно не важно был ли записано входящее сообщение в файл, отправлено на почту, добавлено в базу или что там ещё придумали.
      Конец.

    • @VladGapanovich
      @VladGapanovich Рік тому

      @@kinvain Под необязательной бизнес логикой имелось ввиду следующее: Например у нас есть отправка уведомления по почте и через смс. Но нам всегда нужно отправлять уведомления по почте, и иногда делать это и по смс. Благодаря декоратору у нас был EmailNotifier и его декоратор, который добавляет смс нотификацию. И в зависимости от потрубностей мы бы использовали нужное.
      Касаемо именно вашего кейса - это очень легко решается обычной композицией. Делает класс А, у которого есть метод handle, который принимает сообщение и callback. Callback описываете аннотацией. Далее в сервисах специфичных для ваших сообщений вызываете метода handle класса А, где в callback описываете специфичную для этого кейса логику. Как итог: код декомпозирован и легко читается. К сожалению при декорироании код читается сложнее так как нужно смотреть в конфигурацию конечного класса, чтобы понять в какой последовательности и что будет вызываться. И если он не приносит пользы в виде "нескольких реализаций" как в случае с моим примером с нотификацией - это оверхед, который можно заменить обычной композицией или ещё проще: просто прямым вызовом нужных сервисов
      Так же декораторы ухудшают тестирование. Хоть Валентин и говорил в видео, что это не так (насколько я помню), но на самом деле это так. Например ваши хэндлеры сообщений (насколько я могу судить из полученной информации) не имеют побочных зависимостей и с лёгкостью могут быть протестированы юнит тестами. Однако декораторы это усложняют. Да, вы можете протестировать отдельные части. Но чтобы убедиться, что всё собрано (сконфигурировано) правильно - вам нужно получить из контейнера итоговый класс. Соответственно это уже минимум интеграционный тест.
      Надеюсь что мой коментарий будет вам полезен

    • @kinvain
      @kinvain Рік тому

      @@VladGapanovich Если честно то я ничего не понял.
      Почему SMSNotifier вдруг декоратор именно EmailNotifier? Я согласен что СМС может быть необязательным. И внутри себя он проверят - нужно ли в зависимости от портебностей отправлять СМС или нет. Но в любом случае он выполняет свою логику и передаёт вызов дальше. Но мне кажется что это не совсем правильный пример. Точнее это совсем неправильный пример. Декораторы расширяют логику, а не подменяют её. В вашем примере декоратор либо может работать либо нет. Поэтому, имхо, у вам и "притензия" что логка скрывается, сложно тестировать. Если мы обоернули одни сервис другим то сервис всегда будет работать. Потому что нам надо выполнить и логику декоратора - получить результат, а потом этот результат передать в следущий сервис.
      Про тестрирование тоже не понятно. Что сложного в интеграционном тесте? Есть юнит-тесты и каждый такой тест проверяют что каждый сервис работает так как от него ожидается. Что СМС отправляется, что мыло отправляется, что подпись проверяется, что средства блокируются, что ответ подписывается. При этом нам совершенно не важно что происходило с сообщением до так как на вход в тесте мы уже подаём правильный объект. Безусловно нам нужно убедиться что собранные декораторы будут работать так как мы ожидаем. В моём случае что получив сообщение мы сначала проверим подпись, потом выполним блокировку средств, потом сформируем ответ и добавим в него подпись. Конечно мы можем сделать юнит тест, который проверит что наша условная фабрика собрала именно такой объект, который производит ожидаемый результат. Но просто не надо терстировать юнитом целое приложение.
      Вот как это примерно работает у нас pastebin.com/JV7T1Jp6
      Каждый процессор заточен на одно действие и каждый процессор меняет сообщение. Поэтому это и декоратор так как каждый следующий шаг расширяет действия другого сервиса.
      Про callback я вообще не понял. Списываю это на свою ограниченность. Выглядит как то что логика уходит в аннотации. То есть должна быть ещё какая-то прослойка, которая будет разбирать аннотации и выполнять это. Буду очень признателен если вы сможете привести пример.

    • @VladGapanovich
      @VladGapanovich Рік тому

      @@kinvain Касаемо примера с EmailNotifier и вашего кода - я подготовил снипет с кодом: 3v4l org GMrqN (сори за ссылку. Ютуб удаляет комментарий)
      Насчёт тестирования: С интеграционными тестами всё нормально. Просто если есть возможность написать код не хуже, но чтобы его можно было протестировать юнитами полноценно - это лучше. На эту тему можете почитать про пирамиду тестирования. Именно в вашей реализаци вы можете обойтись только юнитами, так как у вас есть фабрика, которая гарантирует нужную вложенность и которую можно протестировать юнитами.
      Но из-зп этого у вас ухудшается user land код. Так как вам нужно будет сначала создать процессор, а потом запроцессить сообщение, вместо того чтобы получить сразу готовый процессор. Плюс на каждый процессор своя фабрика. Если бы вы собирали свои процессоры на уровне конфигурации симфони, то без интеграционных тестов вы бы не смогли гаронтировать что ваши процессоры собраны правильно с нужным набором декораторов и нужным их порядком. Такая же проблема присутствует и в моём 2 решении.

  • @radlibek
    @radlibek Рік тому

    А где чат?)

  • @vladimirtatarsky9928
    @vladimirtatarsky9928 Рік тому +1

    Реплея чата нет

    • @kinvain
      @kinvain Рік тому

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

    • @PHPPoint
      @PHPPoint  Рік тому +1

      Чат появился.UA-cam его с большой задержкой иногда выкладывает. Если кто-то знает, как ускорить этот процесс, напишите.

  • @kinvain
    @kinvain Рік тому +1

    28:23 моё сугубо личное имхо, но 145-ая строчка это просто какое-то дно...