рекомендации по архитектуре #48
Comments
|
Хорошая мысль, на выходных посмотрю |
|
Почитал статью, доводы вполне хорошие. Но есть несколько "но":
final - это конечно классная штука, но тестирование она усложняет, например phpunit пошлет нафиг, если попытаться замокать подобное. Максимум, что могу редложить - это добавить пояснение в пункты 3.4 и 3.7, что:
Увы, тут очень легко отстрелить себе ногу. Я бы не рекомендовал подобное. Например для сложных кейсов что бы покрыть метод тестами иногда лучше изменить модификатор приватного метода на protected. Я понимаю, что это костыль. Но юзать штуки типа mockery/softmocks для тестирования по сути сгенерированного кода - по моему костыль еще больший.
GRASP как бы уже включает это |
|
по поводу тестирования - есть 2 подхода: stubs (classical TDD) и mocks (mockist TDD) подробнее. т.е. можно тестировать с поведенческой т.зр, а можно с т.зр. данных. ИМХО подгонять архитектуру под тесты не стоит. лучше подумать как протестировать имеющуюся архитектуру. например вводить интерфейс для класса с единственной целью - тестирования, это ошибочный подход. возможно придется прибегнуть к магическим инструментам, благо в php возможностей для этого масса. думаю что скоро появятся инструменты, которые позволят решить эти задачи, если их еще нет. и вообще, создание системы обусловлено необходимостью иметь некий функционал, тесты тут это побочный продукт. а у вас получается тесты важнее функционала выходят. TDD, когда сначала тесты, потом уже код - ИМХО извращенский подход к разработке. но это конечно все дело вкуса... я предпочитаю писать функционал, а уже потом покрывать тестами наиболее важные и сложные классы и методы, содержащие логику. а шаблонный код, инфраструктурный (вроде методов контроллеров), или геттеры и сеттеры структур имхо вообще покрывать тестами смысла нет (напрямую по крайней мере). достаточно проверить алгоритмы через данные на входе и выходе. использовать функциональные тесты, acceptance тесты. |
Вообще, не в сложном кейсе дело, а в лени и плохом коде. Если не получается протестировать - нужно рефакторить класс. |
Уже есть, soft-mocks. Увы, это то, что не должно существовать. Подобные инструменты работают за счет переписывания тестируемого кода. Как результат вы тестируете не свой код, а какой-то там сгенерированный в надежде на то, что его поведение совпадет с тестируемым.
Для меня тесты - это некий слепок функциональности моего кода. Если я что-то меняю по хорошему тесты должны упасть (иначе я написал хреновые тесты).
А вот это уже не корректное утверждение. Вы можете считать что-то "важным" только на данный момент, пройдет пол года и важной может оказаться косвенная функциональность вашего кода.
Стоит, вот вам пример: я поправил один из контроллеров, не покрытых тестами, контроллер работает с внешним API. Мне потребовался день на разборки и тестирование, точно ли я ничего не сломал (в душе я до сих пор не уверен). Тесты сэкономили бы это день.
Ну что вы, сложный код - это далеко не всегда плохой код. Весь вопрос куда вы выносите эту сложность, либо на уровень метода, либо на уровень взаимодействия методов, либо на уровень взаимодействия объектов, либо на уровень взаимодействия сервисов, либо ... Вот вам пример, у меня с коллегой на днях был небольшой спор по теме "сложный ли это код": // ...
public function process(array $events): void
{
foreach ($events as $event) {
foreach ($this->processors as $processor) {
try {
$processor->process($event);
} catch (\Throwable $exception) {
// logging
}
}
}
}Лично я не считаю этот код сложным, он же утверждал, что как минимум try-catch необходимо вынести в приватный метод, что бы метод было легко читать. |
А зачем мокать реализацию? Мокайте интерфейс, а реализацию запихните в сервис который будет реализовывать этот интерфейс (SOLID). Реализацию можно отдельно протестировать. Так тесты будут проще и короче, а сложность кода меньше. Сверху можно покрыть интеграционными тестами в которых PHPUnit уже не нужен.
Это говорит о том, что у вас плохой код и в таких случаях, как правило, код из приватного метода выносится в отдельный сервис. |
пример неправильной архитектуры. контроллер не должен взаимодействовать с внешним API. это должен делать соответствующий сервис, который должен быть покрыт тестами. есть правда такой подход - "толстый контроллер", когда в контроллерах куча всякой логики, куча private методов, и т д. ИМХО это неверный путь. вообще, контроллер, это в каком-то смысле антипаттерн, потому что нарушает принцип единственности ответственности... скорее следует делать Action классы под каждый конкретный метод. просто некоторые фреймворки завязаны на контроллеры как основную сущность, поэтому без них не всегда получается. бизнес-логики в контроллере быть не должно, хотя бы из тех соображений, что одна и та же логика может быть вызвана из обработчика веб-запроса, из queued-задачи, из консольной команды.. вообще имхо стоит использовать DDD в разработке, и всю логику а также структуры данных держать максимально несвязанными с классами фреймворка и сторонних библиотек, возможно имеет смысл ввести "уровень приложения", как прослойку между бизнес-логикой и инфраструктурой в виде фреймворка... хотя это конечно уже другая история. вообще архитектура это область очень объемная, абстрактная, и дискуссионная. в этой задаче я пытался найти несколько конкретных, выраженных в коде, "маркеров" плохой / хорошей архитектуры, и сформировать соответствующие требования. например по поводу abstract/final классов, методов. |
И эта история не про DDD, а про архитектуру и связность.
Принципиально ничего не поменяется, следование SOLID ради SOLID бесполезно.
Отличие фреймворка от остальной инфраструктуры в том, что это фреймворк ваш код вызывает, а не вы вызываете код фреймворка, если что. @index0h |
Это все замечательно, но согласитесь, вы мокаете не только интерфейсы? Например для сервисов, которые гарантировано будут в одном числе можно конечно дополнительно вводить интерфейс, но не всегда в этом есть смысл.
Бывают нюансы. Опять же сложность никуда не денется, вы ее просто вынесете на уровень выше. Я не спорю, для многих ситуаций это вполне годный кейс, но не для всех.
Ок, контроллер на вход получает некий скоп данных, проверят их и передает в http клиент, ждет ответа и преобразовывает ответ для вывода. Что мне выводить в сервис?
Я про тест свойств ничего не говорил. |
если задача контроллера это просто передача запроса во внешний сервис, и единственное, что делается - это валидация, то тестирование валидации должно давать гарантию отсутствия ошибок в вашем сервисе, а остальное уже на совести внешнего сервиса. если бы я делал данный функционал, то у меня было бы минимум 3 класса на это: 1) класс входных данных для сервиса. 2) класс данных из сервиса 3) класс, взаимодействующий с этим сервисом. метод, который аргументом принимает объект входных данных, а возвращает объект выходных данных. в теле осуществляет вызов метода, и возможно валидацию вопроса и ответа. эти 3 класса вне контроллера, отвечают за свои частные задачи, и их можно покрыть тестами без проблем. а в контроллере логики быть не должно. вот у меня как-то так выглядел бы метод контроллера: class SomeContorller
{
public function someExternalCall(Request $request): array
{
$dataToService = SomeDataToService::createFromArray($request->getBody());
$service = new SomeExternalServiceInteractor();
$dataFromService = $service->someExternalMethod($dataToService);
return $dataFromService->asArray();
}
}и тестировать тут по сути то нечего... тестировать надо используемые тут классы. и чего тут можно сломать, что тут можно поменять, чтобы сломать - не понятно. хотя конечно и этот метод протестировать не помешает, но изменение в логике сервиса, или структуре входных/выходных данных никак не затронет этот код. @EvgeniiR говоря про инфраструктуру, я имел ввиду, что создавая классы-наследники классов фреймворка (веб контроллеры, консольные команды, обработчики задач в очереди) - они не должны содержать бизнес-логику. бизнес-логика должна быть вынесена в отдельные классы, |
|
@alexgivi А в целом всё так, только DDD тут не существеннен, и я не оч люблю баззвордами разбрасываться. |
|
@alexgivi Ок, двайте внимательно посмотрим на ваш контроллер.
Не стоит воспринимать тесты как способ только проверки важного функционала, это ложное утверждение. Тесты - это некий слепок работоспособности вашего кода. По хорошему вы что-то поменяли - тесты должны упасть.
Буквально все что угодно, я добавлю сюда |
|
@index0h // вот вам класс фабрики, раз фабричный статический метод - не кошерно.
class SomeMethodIncomingDataFactory
{
/**
* @throws InvalidArgumentException
*/
public function createFromArray(array $data): SomeMethodIncomingData
{
...
}
}
class SomeExternalServiceInteractor
{
/**
* @throws ExternalMethodLogicException
* @throws ExternalMethodErrorException
*/
public function someExternalMethod(SomeMethodIncomingData $data): SomeMethodResult
{
...
}
}
class SomeContorller
{
public function someExternalCall(Request $request): array
{
try {
$factory = new SomeMethodIncomingDataFactory();
$someMethodIncomingData = $factory->createFromArray($request->getBody());
$service = new SomeExternalServiceInteractor();
$result = $service->someExternalMethod($dataToService);
return $dataFromService->asArray();
} catch (InvalidArgumentException $e) {
throw new ApiException($e->getMessage, ApiException::INVALID_INPUT_DATA_CODE, $e);
} catch (ExternalMethodLogicException$e) {
throw new ApiException($e->getMessage, ApiException::SOME_API_LOGIC_EXCEPTION_CODE, $e);
} catch (ExternalMethodErrorException$e) {
throw new ApiException($e->getMessage, ApiException::SOME_API_ERROR_CODE, $e);
}
}
}представленный здесь сервис - уровня бизнес логики API этого приложения. для взаимодействия с внешним API может быть использован другой класс. он будет вызван внутри а по поводу - код изменится, тесты упадут - это не должно касаться например приватных методов, и т д. я думаю не будем сводить идею к абсурду. в общем спорить можно бесконечно, если очень хочется... я лично не вижу в этом смысла. наверное и отвечать не стоило, задачу только забивать. но просто хотел недопонимание устранить. |
|
Чёт не особо чего поменялось. Дёргать фабрики в контроллере - не кошерно, работа с реквестом должна быть вынесена в аргумент-резолверы. Полотно из исключений нужно переделать на респонсы. |
|
с использованием магических инструментов фреймворка можно многое написать лаконично и кратко. правда как оно работает человек, не работавший с данным фреймворком, поймет только после глубокого изучения документации. хотя это конечно верно. все равно проекты в большинстве своем намертво завязаны на фреймворк, и все разговоры в стиле "написать фреймворк-независимый компонент" - больше из разряда красивых слов. если в каком-то фреймворке контроллеры не наследуются от класса фреймворка, то можно конечно класть туда бизнес-логику, ведь это получается чистый класс, который можно использовать и в других местах. но в laravel, yii2 такого нет, даже в symfony хотя и можно писать чистые контроллеры, но "для удобства" существуют базовые классы. вместо new SomeClass писать $this->getContainer()->get(SomeClass::class) - принципиальной разницы не вижу. только тут возникает дополнительная зависимость. в общем спорить можно... только зачем, вот в чем вопрос - все равно подходы разные, единого универсального рецепта нет и пока не предвидится... если у вас есть какие-то авторитетные источники, на которых основаны ваши взгляды (что-то вроде книг Боба Мартина) - то welcome, делитесь, будет очень полезно и интересно ознакомиться. но вроде пока такой литературы нет. обычно документация к фреймворку является таки руководством по архитектуре. |
У меня на осмысление Аргумент Резолверов ушло минут 20 от силы. Взглянуть на ResolverInterface в принципе достаточно чтобы понять чего они из себя представляют.
На удобство крудоклепателей если только. С 4 Симфони сервисы сервисы сделали приватными по дефолту, чтобы люди не лезли куда не надо, и не юзали контейнер как Сервис-локатор, вторым покзанным вами способом дела делать точно не нужно.
Документация по фреймворку лишь описывает как дергать методы фреймворка, нет там ничего про архитектуру.
Гуглите Dependency Injection, которого не хватает в ваших примерах, и выбирайте понравившийся вам источник. |
Я бы не говорил "должна". Я бы сказал - работа с реквестом может быть вынесена в аргумент-резолверы или парам-конверьеры. Мапить реквест на DTO может быть удобно, но надо помнить, что на каждый запрос инициализируются все аргумент-резолверы, парам-конверьеры и их зависимости. Если к разработке резолверов и конвертеров подходить не аккуратно, то с ростом проекта это крайне негативно скажется абсолютна на всех запросах к приложению. Порой, лучше ограничиться инъекцией сервисов и возможно делегировать маппинг им. |
Чушь полная и экономия на спичках. Проблемы оптимизации инициализации приложения решаются по другому, но у вас их явно нет. |
|
К слову о |
То есть по вашему Lazy Services, lazy load у event subscribers и инициализация сервисов при обращении к ним это чушь? Вы считаете нормальным инициализировать 100500 сервисов на каждый запрос и использовать только один из них? |
Во первых - мне нет дела сколько там сервисов инициализируется, пока это занимает сотые доли от времени процессинга запроса, а оно именно столько и занимает, если мы говорим об аргумент резолверах. Во вторых - вас никто не принуждает их инициализировать. В третьих - те, для кого не приемлемо инициализировать всё приложение на каждый запрос, не пишут на пхп, или хотя бы берут roadrunner. Повторюсь дабы закрыть, наконец, эту тему - не хотите ничего ободрать - не лезьте на ёлку, и возьмите другой язык без умирающей модели выполнения. Взяли пых - полезли на ёлку - так делайте нормально уже. |
нашел хорошую статью
https://habr.com/ru/post/482154/
из нее можно сформулировать несколько рекомендаций:
конечно, следовать этим принципам по всей строгости весьма трудно, но принципы хорошие.
в целом можно было бы создать раздел по архитектуре, где заложить подобные рекомендации, на более абстрактном уровне.
вообще, документ получается довольно большим, можно было бы вынести каждый раздел в отдельный файл. получится уже что-то вроде книги) еще как вариант можно краткие правила собрать в одном файле, и дополнительно более подробно расписать в отдельных файлах по каждому разделу, дополнив документы ссылками на интересные статьи, книги, репозитории и т д.
The text was updated successfully, but these errors were encountered: