Pull to refresh
36
0
Николай Пунин @png

Пользователь

Send message

>>Вынесение метода, нужного в одном месте конкретного сервиса, на уровень сущности, по-моему, лишнее.

Этот метод работает с данными сущности и на основе этого принимает какие-то решения, генерирует какие-то события. Вообще-то это называется инкапсуляция. В реальной жизни, эти методы не слабо так разбухают (хотя есть и достаточно компактные). Мы на эти методы пишем свои отдельные тесты, а на сервис свои.

Термин "умные объекты" здесь ввел не я, а один из комментаторов. Если есть умные объекты - должны быть и тупые. Что из них что, до конца не понятно, в какой момент тупой объект умнеет?

Мне ближе понятие обычных объектов, которые предполагают объектно-ориентированный стиль мышления. А ещё есть структуры данных и анемичные модели, которые автор этого термина называет антипаттерном.

А что вы понимали под умными объектами?

Чем ваш подход размещения бизнес логики в сервисе отличается от классического transaction script?

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

Однако, всё же хочу отметить, если стараться, не усердствовать и следовать правилам(например, этим), а так же здравому смыслу, то в целом код получается достаточно элегантным.

>>пропагандирующему "просто другой подход, Entity - это умный объект, со своими методами". 

Я его не пропагандирую, я на нем пишу. Года этак с 2010. Для полного погружения в тему советую почитать Фаулера, а этак же что-нибудь на тему DDD (кстати, там в комментариях, люди неплохие ссылки нагнали, т.к. у Эванса - во-первых, очень много воды, во-вторых, слегка устаревшие примеры, так сейчас уже никто не пишет, технологии сильно поменялись, в-третьих, прямо скажу, очень тяжелый язык повествования, я бы эту книгу догонял бы под конец, чтобы добавить мелкий штришки к общей мозаике).

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

Примерно код может выглядеть так.

import org.springframework.context.ApplicationEventPublisher;


@Service
@Transactional
@RequiredArgsConstructor
public class UserServiceImpl implements UserService {
    private final GosUslugiClient gosUslugiClient;
    private final UserRepository repository;
    private final ApplicationEventPublisher eventPublisher;
    public User checkFio(UUID userId) {
        final var user = repository.findById(userId).orElseThrow(() -> new UserNotFoundException(userId));
        // дергаем сторонний сервис
        // предположим, сервис не доступен, ихмо тут клиент выкидывает 
        // эксепшен, а пользак или кто там снаружи, потом нажмет кнопочку ещё раз
        // если ненайдено - пустой результат   
        final Optional<GosuslugiPersonDto> personDto = gosUslugiClient.findFio(user.snils()); 
        // здесь передаем результат в доменную модель, которая сама 
        // решит, как с ними поступить, и на основе своих действий 
        // может сформировать одно или несколько событий
        
        // есть у меня один проект, где я этот вызов оформил функцией, 
        // чтобы не получить GodObject из цетрального сервиса, т.к. там 
        // очень много интеграций, тогда получаем много сервисов-интеграций, 
        // который инжектят в себя клиент и условный UserService. 
        // Решение с функцией очень понравилось, т.к. ушел от тучи 
        // тупого кода, и всё на своих местах        
        final var event = user.verifyData(personDto);
        repository.save(user);
        // spring-event, чтобы обеспечить слабую связанность между сервисами, 
        // например, метод доменной модели формирует событие, что 
        // данные были обновлены (или не формирует, как повезет)
        
        // мы публикуем это событие, а него могут быть сервисы-подписчики, 
        // которые, например, разошлют почту/смс - о том, о чем нужно. 
        
        // при этом, если мы не уйдем в асинхронность(или в реальные события 
        // - а там и до event driven development - один шаг), 
        
        // всё это будет в одной транзакции и если подписчик по какой-то 
        // причине развалится, то произойдет откат всей операции
        
        // здесь очень один тонкий момент, какие вызовы нужно делать явно, 
        // а какие утаскивать в события:
        //  - перетащишь всё что только можно в сервис, там и до GodObject не далеко
        //  - будешь увлекаться событиями - получишь, тяжелую 
        //     поддержку и непонимание разработчиков что произошло 
        //     и в каком событии(потому что, чтобы такое поддерживать, нужно очень 
        //     глубокое погруженые в бизнес-процессы, а тут через полгода приходишь 
        //     на проект пару багов поправить и сам забыл, что там и где.). 
        
        // А цепочек из 2-х и более событий (когда обработчик события 
        // генерит свои события) вообще лучше не допускать
        
        // для себя выработал подход, в события выносим то, что 
        // напрямую не меняет текущую модель, а является чем-то 
        // второстепенным
        // например, 
        //   -- результат вызова - нужно сохранить в модель, это в событие нельзя. 
        //   -- вызов формирует какой-то запрос - а ответ, приходит асинхронно. 
        //      Такое тоже в событие нельзя (или иногда можно, но очень аккуратно 
        //      с полным пониманием процесса). Высокий риск, что асинхронный ответ 
        //      придет до того, как будет закрыта основная транзакция, получим 
        //      просто конфликт версий при закрытии транзакции, т.к. та же кафка шустрая
        eventPublisher.publishEvent(event);
    } 
}

А теперь усложнимся, и вернемся к самому первому вопросу, на который я побоялся сразу развернуто отвечать, но раз вы проявили заинтересованность, то попробую крупными штрихами. Вы говорили, что дергаете сразу 4 сервиса. Это слишком много, если они все синхронные, то каждый из них под нагрузкой будет подтупливать. Например, в легком режиме, один сервис отвечает 0.5 секунды, но иногда он отвечает 10-15 секунд. Если таких 4, то мы уже получаем 40-60 секунд, а это как минимум слабая отзывчивость синхронного сервиса, как максимум таймауты.

А если вы дернули 3, а 4-й упал? получается, вы первые 3 дернули зря, т.к. просто забыли их данные.

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

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

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

>> Вы про толстые ДТОшки?

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

>> Всю статью не покидало ощущение, что очередная борьба за качество кода.

так и есть.

>>Вызов был воткнут в класс нарушавший все буквы в SOLID . Буквально.

Ну и нормально. Есть другая крайность, тупое следование всем принципам. Я тоже иногда осознанно игнорирую что-нибудь из SOLID в угоду простоте, читаемости, качеству. Тут чуйка какая-то должна быть. Наверное, оно вырабатывается с опытом.

Кстати, она не плохо срабатывает во время дебатов внутри команды. По-обсуждаем, а потом получается сильно лучше, чем когда один писал.

>>В общем код на голову лучше чем там где борятся...

Однозначно да. А чтобы не бояться, нужно через TDD писать. Если код покрыт качественно тестами, бояться не чего. Другое дело, писать их тоже надо уметь.

подправил статью. используете @Value, но все почему-то лепят @Data

моя тоже. Но пользую не так часто, потому что редко приходится пользоваться API c checked exception. Видимо такие прилетают задачи.

>>equals и hashcode

ну это элементарно, даже не стоит внимания. Хотя я ещё встречаю разрабов, которые не могут назвать контракт equals-hashcode на собесах.

>>ломбок - это сокращение генерируемого бойлерплейта

Только ли? Если убрать механику реализации за скобки, мы получаем новый синтаксический сахар в язык. Именно так воспринимают его многие - как часть языка, которую можно включить и будет типа удобно. Посмотрите-почитайте авторов котлина в той части, где они бьются за то, чтобы не вводить в язык новый синтаксический сахар. Посмотрите их аргументы почему новые фичи вводить не надо. Если смотреть через эту призму, там не всё так прекрасно, как вы говорите.

>>предлагаете инъекции сервисов прямо в сущность?

Такого я не предлагал. Что куда передавать по данным - вопрос к разработчику. Нужно смотреть на решаемую задачу.

А какая парадигма у вас используется domain model или transaction script(если говорить в Фаулеровской терминологии)? Это и будет ответ как вам лучше поступать в том или ином случае.

Тонну геттеров и сеттеров можно заменить ключевым словом record + builder. Собирать dto через setter - я бы не стал.

1-2 у нас просто другой подход, Entity - это умный объект, со своими методами. Всё работа - поднял объект из базы -> вызвал на нем метод с параметрами -> объект поменял состояние -> сохранил измененный объект. Не допустимо туда выставлять что-то через setter. Этот setter - при ближайшем рассмотрении вырождается в бизнес-метод со своим смыслом и логикой. Всё по классике ООП.

4 - у нас 17-ая java - record лучше )

Php он как JS, был рожден для одного, а потом стал использоваться для другого. Да, из-за этого какие-то библиотечные вещи кажутся плохо продуманными. Лично мне кажется, что php используют не для того, для чего он предназначен, в этом вся беда. В этом плане тот же Ruby выглядит более идеологически выдержанным.

Что касается разрабов и мест работы. Мне близка мысль про то, что везде оно одно и тоже примерно. Есть хорошие специалисты, а есть весьма странные личности.
Лет 10 назад процент этих странных личностей в php зашкаливал, т.к. для php очень низкий порог входа, чтобы потом считать себя разработчиком.
Однако сейчас положение дел немного поменялось. С появлением всяких java-курсов и течения "войти в ИТ", появилось куча java-истов, которые ну немного до разраба не дотягивают. Можно тоже самое сейчас сказать про java, внятный хорошо сделанный проект с хорошей командой(ну то есть хорошее место работы) нужно ещё поискать. Про интервьюверов вообще молчу...

Комментарии уже закрыты, а хотел сказать спасибо за статью. Плюсанул в карму :)

в этом месяце получил похожую проблему.

был у меня монитор AOC 27'' 4K, подключал к ноуту HP ProBook 4G. Ноут 2019-ого года. у него есть порты type-c и hdmi (какой версии hdmi - я не нашел внятного описания ни где).

Работало всё прекрасно на 4K@30Гц.

Год жил с этим монитором, потом он сломался, я его сдал а вместо него взял другой. выбор пал на asus gaming VG289Q1A 28'' 4K. Приехал монитор, а подключаю его через HDMI, а максимальное разрешение не ставится. Оказалось монитор не поддерживает частоту в 30Гц. только 50 или 60. Внезапно. Неожиданно и обидно. Купил дорогую железку, а она "не работает". В итоге, принял решение, сдать монитор обратно в магазин, а вместо него взять AOC U27P2CA 28'' 4K. У него всё есть, в том числе type-c. Планирую подключать к ноуту через него. И то, я до конца не уверен, что я воткну всё в ноут и всё заведется.

Ковырялся в спеках современных 4K мониторов, везде стоит 60. Список поддерживаемых режимов нашел только для мониторов asus. У остальных просто страничка на сайте.

И проблема есть, производители мало что пишут в описании, что-то более детальное не доступно, и в этом беда.

За статью большое спасибо. Всё очень четко.

Про параметризированные тесты накину кейз неправильного использования, который достаточно часто меня расстраивает.

Это попытка разработчика загнать ВСЁ в параметризированные кейзы. Точнее в один параметризованный тест. Это приводит к тому, что этот параметризированный тест всё чаще становится всё крупнее и сложнее (к тому же для некоторых кейзов делает что-то лишнее).

Я бы в этом случае подумал о серии мелких простых тестов, или серии параметризированных тестов более простых, чем одна большая enum-ка и монструозный тест. Либо о комбинации. Основные тесты - параметризированы (возможно, несколько параметризированных), а особые случаи, которые не легли в общую схему достаточно легко, выделены в отдельные тесты.

Интересная статья, где-то узнал себя :) Мысли вслух, по этому поводу.

Прежде чем что-то бежать менять нужно вникнуть в проект и понять всю его боль… бежать что-то менять после 2-х недель на проекте — это как-то опрометчиво.
Чтобы что-то явно менять — нужна явная санкция менеджмента, рефакторинг, тесты, качество кода на ревью — это всё про время, у менеджмента должно быть явное понимание на что мы тратим время и почему, поэтому, то, что менеджер в этой истории занял такую позицию — понятно и очевидно, пришел странный чувак — чего-то хочет, чего-то выступает.

Путей разрешения несколько, вариант первый — и самый логичный, искать другую команду, а не воспитывать эту. Работать надо единомышленниками, которые считают, что нормально — это убить 1.5 дня на рефакторинг перед выполнением задачи, которые перед комитом смотрят warring-и компилятора, у которых стоит плагин для sonarCube, смотрит цикломатическую сложность кода и не дает комитить явную фигню.
Должен быть devops — и внятное CI, отчеты sonarCube и кодопрокрытия тестами. Развиваться лучше в рамках такой команды.

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

По подходу выше, отсутствие тех же тестов ни является критикал, т.к. всё же и так работает, а по-моему оно всё ломает, т.к. теже тесты определяют архитектуру решения. Хотя речь не только о тестах, тесты как пример. Даже подход работать хорошо в своем месте и никому на мозги не капать — не очень хорошо работает. В моем случае — находились люди, которые меняли мой код, получили упавшую сборку в CI (т.к. локально тесты никто не запускает), и тупо удаляли тест, который не работал. Короче, всё очень сложно.

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

Такое лучше через assert не реализовывать. Лучше правда такое делать так, как описано в #comment_8207963
Мне такой вариант нравится больше, чем assert. Хотя там над API нужно серьезно думать.
Но всё же, я больше склоняюсь к мысли, что всё это не нужно.

По мне лучше делать так:

1. Явные проверки закрыть UnitTest-ами.
2. Входные данные отбраковывать через throw new IllegalState или какими-нибудь чеками из guava (там их мало, мне пришлось дописывать свои, но в том же ключе).
3. И в тестовой среде, и production код должен работать одинаково.
4. Идти по пути максимальной понятности кода.

Таким образом, мы коду задаем жесткое API и уже заранее знаем что может произойти, и как это обработать. Если всё сделать правильно, то лишние assert не особо нужны. Действительно, зачем они нужны, если на любое некорректное поведение в системе мы получаем Runtime.

Кроме, того это замечательно ложится на EJB, в которых транзакции автоматом откатываются при Runtime.

Далее, мне сложно понять, почему это проверки могут так сильно тормозить, мы же не архиваторы пишем. Чаще упираешься в базу (или в алгоритмические ошибки), а не в лишнее сравнение. Мне кажется в большинстве случаев это стрельба из пушки по воробьям и кустарная оптимизация. Толку от нее мало, зато сложностей туча. Например, утащили мы класс во время рефакторинга в другой пакет, и получили, что assert не запускаются. По мне, это лишние контракты в коде.
Про мусор в коде, уже писали. Ведь это фактически мертвый код.

Но возможная другая ситуация, вам дали поддерживать систему, в которой разработчик забил на хоть какую-то целостность входных данных. Это привело к тому, что любые не такие входные данные (а система то, сложная) приводят к NPE. Чтобы понять, что не так. нужно раз 10 воспроизвести этот NPE и подебажить часов 5-10 (я не шучу, так и было). Вот там-то да, я щедро раcставлял assert, т.к. ошибки были однотипные, что сильно экономило время их разбора.

Почитал ветку комментария #comment_8208035. Полностью согласен с вашими оппонентами. Особенно красноречив последний комментарий. Я тоже так делаю. Idea рулит!
Коллеги, по самой статье есть ряд замечаний и фактических ошибок.

По содержанию, толком не описано зачем же нужны assert и когда их использовать стоит, а когда не стоит. какие есть достоинства и недостатки.
Если этого нет, то это просто частичный копипаст документации.
Статьи про эту тему на харбе уже были(например, вот этот вариант более информативен). Что нового дает именно ваша статья?

По примеру кода полно фактических ошибок использования assert:

1.
assert initReferenceSet();

В assert никогда нельзя вставлять вызовы функций. эти функции могут менять внутреннее состояние программы. В вашей ситуации получите, что с выключенным assert initRederenceSet никогда не выполнится, т.е. получите багу на production, которая не повторится на тестовой среде (я предполагаю, что в production они выключены, т.к. нафига они там нужны — это отладочный функционал).

2.
assert addToReferenceSet(e);
//....
assert removeFromReferenceSet(o);

Проблемs такие же, что и в пункте 1.
3.
assert referenceSet.size() == size() :
                "Cool size diverged from reference size";

А это вообще ворох проблем. К пункту 1 добавляется возможный невнятный NullPounter в режиме дебага, которого не будет на production.

Всё же, желательно, чтобы код с включенными assert и без них работал и работал одинаково.

Теперь о проблемах assert.
1. Народ правильно писал выше. На production они не нужны, т.к. не хорошо, когда приложение разваливается на некорректных данных. Обрабатывать эти assert не удобно. Об этом уже писали, не буду повторяться.
2. throw new IllegalStateException во всех отношениях лучше. Об этом тоже писали.
3. Они действительно захламляют код, и при определенной неаккуратности, его ещё и ломают.

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

я могу поработать из дома, если приболел :)
Но дома работать сложно, обстановка что-ли не та )
Хорошие рубисты тоже стоят не дешево

Information

Rating
Does not participate
Location
Тамбов, Тамбовская обл., Россия
Date of birth
Registered
Activity