Всем привет!

Теперь перейдем к рекомендациям для авторов Pull Request (PR), они же Merge Request.

1) не нужно тратить время ревьювера на то, что могут сделать роботы) Я про чистку import и форматирование кода. Рекомендую поставить эти действия на автовыполнение при сохранении в IDEA. Т.об. ревьювер увидит более корректный код, и ему будет проще найти серьезные ошибки, ради которых мы и проводим ревью кода.

2) не стоит отправлять на сервер код, для которого не проверена компилируемость и прохождение тестов. Да, я надеюсь ваш DevOps пайплайн это проверяет, но даже в таком случае это лишняя нагрузка на инфраструктуру, а также лишний цикл ошибка-исправление-запуск CI и замедление скорости попадания кода на тестовую среду и в конечном итоге на ПРОМ.

3) также рекомендую подключить SonarLint в IDEA и чинить баги SonarQube локально. Причины те же, что в предыдущем пункте

4) как я уже писал - ревью кода тяжелая работа. С увеличением размера кода сложность этой работы увеличивается, причем нелинейно. Поэтому декомпозируйте задачу, уменьшайте каждого размер PR. Если изменений по одной декомпозированной фиче все равно достаточно много - разбивайте PR на несколько commit. Рекомендуемый размер PR - не более 100 строк.

5) а чтобы облегчить понимание сути изменений в PR - делайте содержательные комментарии, как именно - я уже описывал в https://t.me/javaKotlinDevOps/81 Если кроме информации в отдельных commit и в тикете, по которому сделан PR, есть еще какая-то высокоуровневая логика, понимание которой важно для ревью - напишите об этом в шапке PR или отдельным комментарием к PR.

6) если ревьювер "пропал" (это плохой, но вполне реальный кейс) - постарайтесь достучаться до него по всем возможным каналам и понять причину отсутствия реакции на PR. Если оперативное ревью невозможно - найдите другого ревьювера.

7) снова о психологии) замечания к PR - это не замечания к вам лично, это замечания к вашему коду. Поэтому относится к замечаниям надо спокойно, возможно с помощью ревьювера вы узнаете что-то новое о сервисе, фреймворке, платформе или даже языке программирования. Ошибки делают все, это нормально. Главное не доводить их до ПРОМа)

8) Если копнуть глубже - ваш код на самом деле не ваш, это код команды. Вся команда отвечает за фичу. Т.е. вы с ревьювером в одной лодке, ваша общая цель - быстро вывести качественную фичу на ПРОМ.

9) как бы ни был велик соблазн пропихнуть в PR еще пару тройку мелких правок - ему нужно сопротивляться. По себе знаю, что это сложно) Но scope PR должен соблюдаться, это сильно упрощает ревью!

10) код-ревью - это диалог, а замечания - не приговор. Ревьювер может что-то не понять, не учесть или просто ошибаться. В этом случае нужно ему вежливо объяснить детали

11) ну и снова скажу что текст - ограниченный способ коммуникации. Чтобы избежать недопонимания - пообщайтесь с ревьювером лично если есть возможность, позвоните ему или поговорите по зуму

12) если PR сложный и\или работа над ним шла долго - по возможности гляньте его сами, перед тем как добавлять ревьюверов. Посмотрите именно как PR, чтобы увидеть что изменилось и что увидит ревьювер. Возможно, найдете какие-то простые баги сами или поймете, что нужно добавить комментарий

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

14) и еще одна важная вещь: недостаточно просто поправить замечание в коде. Напишите что-то типа "сделано" в ответ на каждое замечание. Скажу по себе - ревьювер вам мысленно скажет спасибо!)

#code_review

Всем привет!

Поговорим про инфраструктуру и DevOps, не относящуюся напрямую к ревью кода, но сильно облегчающую его проведение.

1) обязательно должна быть автоматическая проверка Pull Request (PR) на компилируемость, прохождение тестов и статический анализ кода. Одно из распространенных названий этой процедуры - prcheck. Те ошибки, что могут найти роботы - должны искать роботы)

2) обязательно должна автоматически запускаться контрольная сборка с запуском тестов после вливания кода в master\develop\release. Так мы как можно раньше локализуем проблемы, которые могут возникнуть после слияния. Несмотря на наличие prcheck и ручное ревью после слияния вполне могут появится ошибки. Особенно важен этот и предыдущий пункт если у вас не микросервис и над кодом работают несколько команд. Т.к. в этом случае ошибка, попавшая в master, заблокирует работу всех команд.

3) рекомендую сделать документ с требованиями по коду (guideline), которые невозможно автоматизировать. То, что можно автоматизировать - описывать текстом не нужно, а следует добавить в статический анализатор. SonarQube, номер один на этом рынке, дает нам кучу правил из коробки, кроме того есть дополнительные плагины с правилами, например, https://github.com/spotbugs/sonar-findbugs, а также можно создавать свои правила. Правила нужно поддерживать его в актуальном состоянии и строго им следовать. Это может быть файл в формате Markdown (.md) в Bitbucket, тогда с ним тоже можно работать через PR, либо статья в формате wiki. Главное чтобы четко было понятно, где лежит последняя версия правил

4) также должны быть правила наименования веток, PR и формата commit

5) в случае монолита будет полезным механизм, определяющий какого рода код изменился и автоматически добавляющий в PR ответственных за данную функциональную область

6) как я уже говорил ранее может быть полезен отдельный чатик для призыва ревьюверов в PR) email уведомления тоже полезны, но почту читают реже, письма могут попасть в спам

#code_review

Всем привет!

До сих пор говоря про ревью кода, я акцентировал внимание на команде. Когда же может быть полезно кросс-командное ревью.

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

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

Также кросс-ревью может быть полезно, если модули, выпускаемые командами, интегрированы друг с другом. С одной стороны может показаться, что это нарушает инкапсуляцию, которую дает нам API. Но бывают случае, когда между "соседними" командами возникает недопонимание - почему API сделано именно так или почему его стали использовать именно таким образом. Для начала в таких случаях стоит собраться и обсудить API. Если же этого мало - возможно, погружение в код соседей поможет понять причины.

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

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

#code_review

Всем привет!

Поговорим про комментариях к commit.
Вот статья, где приводится пример интересного комментария: https://dhwthompson.com/2019/my-favourite-git-commit

Вообще, я двумя руками за содержательные комментарии, объясняющие что изменилось и почему.
Но этот мне не нравится)
Почему?
1) это перебор. Время, которое мы можем потратить на разработку, конечно. Соответственно, размер комментария должен быть пропорционален его важности и\или объему измененного кода.
2) для того, чтобы поделится знаниями о проблеме, комментарии git не лучший вариант. Они не индексируются поисковиками, если речь про открытые проекты Github. В случае закрытых проектов часто доступ к git-у открыт не для всех. Какой выход - написать статью или пост о решенной проблеме.
3) даже для формирования what's new такой комментарий плохо подходит, т.к. слишком большой и отвлекает внимание на себя от других изменений.

Что думаете?

#git #code_review

Всем привет!

Ну и наконец немного расскажу о плохих практиках ревью кода.

Суббота. Баг на ПРОМе. Подключение из дома. Какой-то странный код, напоминающий спагетти. Много нецензурных выражений и энергичных пожеланий в адрес его автора)))
Не надо до такого доводить.

#code_review #joke

Всем привет!

В последние годы стала "модной" тема null safety. Суть в том, что не нужно хранить и передавать null значения, чтобы не напороться на Null Pointer Exception. В том же Kotlin null safety встроена в язык - все типы по умолчанию не могут содержать null.
И на самом деле это правильный подход. Но есть нюансы)

Рассмотрим такой случай - мы идем куда-то за данными, данные по бизнес-процессу там обязаны быть. Например, мы прихранили id записи где-то в пользовательском контексте в начале процесса и идем за данными в конце процесса. Но данных нет. Следуя null safety можно просто создать пустой объект - например, с помощью конструктора. Как вариант, часть полей этого объекта будет проинициализирована значениями по умолчанию.

Так вот - в случае, когда данных нет из-за какой-то нештатной редко воспроизводимой ситуации: неверные тестовые данные, на сервис идет атака с перебором всех возможных значений, в процессе операции данные некорректно мигрировали, кривая архитектура - лучше просто "упасть", т.е. выбросить исключение. Есть такой принцип - fail fast. Т.к. создавая пустой объект, мы во-первых надеемся что он будет корректно обработан выше, а это может быть не так. А во-вторых - а зачем передавать управление дальше?

P.S. Как всегда - напомню каждую ситуацию нужно рассматривать индивидуально, чтобы различать отсутствие данных как часть бизнес-процесса и нештатную ситуацию.

#kotlin #code #patterns #principles #nullsafety #fail_fast

Всем привет!

В продолжение темы функциональности IDEA и подготовки кода к code review, см. https://t.me/javaKotlinDevOps/148
Некоторые проверки и исправления можно подвесить на одно из двух событий:
1) на сохранение файла: Alt-Alt - Actions on Save. Рекомендую включить Reformat code и Optimize import.
2) перед commit на Git сервер - открыть окно Commit (Ctrl-K) и нажать там шестеренку. Рекомендую включить Analyze code, Check TODO и если выполнение тестов занимает приемлемое время - то еще и прогон тестов.
Легко заметить, что набор опций в обоих случаях похож, но на сохранении можно включить только те, где фиксы применяются без участия человека. В частности и Cleanup, и Analyze выполняют правила из набора инспекций (Inspections), только в первом случае включаются только те правила, где есть quick fixes, которые можно применить автоматически.
Насчет Cleanup - IMHO его вполне можно включить на commit, главное перепроверить набор активных правил: Shift-Shift - Inspections, а там включить фильтр Cleanup only. К слову - там еще есть профили с набором правил, можно добавить свой.
И еще важный момент - инспекции из IDEA можно запустить из командной строки, и т.об. включить в CI процесс: https://www.jetbrains.com/help/idea/command-line-code-inspector.html
Они частично повторяют проверки SonarQube, но не идентичны им.

#idea #code_review #clean_code #git

Всем привет!

Про DI и DI.

Аббревиатура DI может расшифровываться на Dependency Inversion, а может как Dependency Injection.

Dependency Inversion - это буква D из SOLID - базовых принципов разработки.
Означает, что высокоуровневые классы не должны зависеть от конкретных реализации, и в Java API любых классов лучше использовать интерфейсы везде, где это возможно. Почему такая ремарка: интерфейс с единственной реализацией - очень странная штука) Но я отвлекся) Следование принципу облегчает тестирование и расширение функциональности системы, т.к. позволяет легко заменить любую реализацию.

Dependency Injection - это механизм внедрения зависимостей, важнейшая особенность которого - собственно внедрение зависимостей отдается на откуп внешнему модулю. Самые известный пример - Spring c его IoC контейнером, но есть и другие заточенные конкретно на эту задачу и поэтому более шустрые альтернативы.

Если подходить формально - это два разных понятия, кроме аббревиатуры никак не связанные. Но с другой стороны Dependency Injection по сути - это инструмент, сильно облегчающий реализацию принципа Dependency Inversion. А хороший инструмент помогает писать правильный код. Важное замечание - Spring IoC не обеспечит за вас реализацию инверсии зависимостей. Если метод API завязывается на конкретную реализацию или уровни приложения связаны циклически - Spring тут не поможет. Поможет предварительное проектирование на уровне кода и TDD.

#code_architecture #interview_question #arch #patterns #solid

Всем привет!

На собеседовании я иногда задаю вопрос: приведите пример нарушения принципа Single responsibility. Или альтернативный вариант - а вот если в методе, к примеру, activateCard мы заодно отбросим метрики или залогируем результат - это нарушение принципа или нет.
На первый взгляд ответ - нет. Метрики и логи - это технический код, не бизнес функционал. Он может понадобиться в любом месте кода. Часто такой функционал реализуют с помощью аспектов, т.к. во-первых - это можно реализовать с помощью аспектов, а во-вторых - это красиво))), т.е. некий синтаксический сахар, улучшающий читаемость кода.
Но можно рассмотреть немного другую ситуацию. Предположим, есть код с математическими вычислениям. Или любой алгоритм. Или логика обработки данных. То, что хорошо реализуется в функциональном стиле - входные данные метода, результат, никаких внешних зависимостей. В нём нет внешних взаимодействий, сохранения в хранилище. Чистая логика. В этом случае логирование и метрики - это уже некая обработка полученного результата. Мы же не просто так выводим что-то в лог - это либо данные для разбора ошибки, либо отслеживание пользовательского пути, сбор статистики, отслеживание времени выполнения кода... Т.е. есть отдельная логика по месту и составу того, что мы логируем. Опять же контекст логирования часто требует инициализации, что добавляет ненужные зависимости в нашу логику. Поэтому такой код лучше поместить на уровень выше.
Итого: бизнес функционал и логирование/метрики - да, "чистая" логика - нет.

#logging #metrics #interview_question #code_design #solid #dev_compromises

Всем привет!

Я периодически провожу код-ревью. И лично у меня ранее был страх - а вдруг я не замечу что-то важное. Смотришь PR, явных ошибок не видно, но апрув "не ставится". А когда Pull Request (PR) очень большой - даже не хочется его открывать, т.к. примерно понимаешь, какой объем информации придется "загрузить в голову", чтобы точно можно было сказать - да, здесь ошибок нет. Крайний случай слишком тщательного подхода к код-ревью, встреченный мной на практике - когда разработчик просит освободить его от других задач, чтобы он мог полностью погрузиться в ревью: прочитать аналитику, изучить архитектуру кода...

Но как я уже писал в https://t.me/javaKotlinDevOps/146 - скорость прохождения код-ревью очень важна для быстрого выявления ошибок и предотвращения merge конфликтов. Что же тут можно сделать?

Во-первых я предлагаю рассматривать код-ревью как еще одну из сетей для поиска проблем в коде. Есть и другие сети - валидация в IDEA, модульные и интеграционные тесты, CI pipeline, SonarQube и другие инструменты статистического анализа, включая сканирования на уязвимости. Это если говорить о тех, что работают до код-ревью. После - регресс, тесты нового функционала, нагрузочное тестирование, демо (приемо-сдаточные испытания), канареечное развертывание, chaos engineering... Т.е. код-ревью - полезно, но это не последний рубеж обороны.

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

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

#code_review

Всем привет!

Не ошибусь, если предположу, что многие Java разработчики знают об Sun code style conventions https://www.oracle.com/java/technologies/javase/codeconventions-contents.html
Их автоматическая проверка реализована в Checkstyle https://checkstyle.org/styleguides/sun-code-conventions-19990420/CodeConvTOC.doc.html

Но это еще не все, что предлагают себе и нам разработчики Java.

Во-первых Sun уже давно нет, есть Oracle, который его купил.
И есть более новая версия code style от Oracle https://cr.openjdk.org/~alundblad/styleguide/index-v6.html#toc-introduction (доступ по VPN)

А кроме того, у Oracle есть свои правила для проверки качества кода https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88487665
Ссылка выше ведет на раздел по обработке исключений, чтобы можно было оценить объем и глубину требований на конкретной фиче языка.
Что интересно - не все правила есть в SonarQube, по тем же исключениям увидел несколько новых для себя вещей.
Некоторые из них я полностью поддерживаю, они вроде как логичны и можно сказать очевидны https://wiki.sei.cmu.edu/confluence/display/java/ERR03-J.+Restore+prior+object+state+on+method+failure
Некоторые https://wiki.sei.cmu.edu/confluence/display/java/ERR06-J.+Do+not+throw+undeclared+checked+exceptions можно кратко суммировать так: да, я нашем языке есть дыры, но не надо их использовать, пожалуйста)))
Что хорошо - в конце страницы с описанием правила есть секция со ссылками на соответствующие правила SonarQube и прочих утилит статического анализа кода.

#java #code_static_analysis

Всем привет!

В последнее время все чаще вижу, как LLM в IDE используют для генерации каркаса приложения. Казалось бы - это же задача генератора, а не LLM. Какого-нибудь Spring Initializr (https://start.spring.io/) Почему он этого не делает, а ограничивается по сути только pom-ников или *.gradle?

Потому что это отдельный сервис, требующий поддержки. Который со временем - по мере развития библиотеки, фреймворка или платформы, для которой он предназначен - будет обрастать все более сложной логикой. Обновился фреймворк - нужно обновлять генератор. Появился смежный компонент - будут запросы на добавление интеграции с ним. И при этом генератор все равно будет ограничен в возможностях. Возьмем тот же Amplicode - контроллеры, обработчики ошибок и тесты для Spring приложения он генерировать умеет, что-то еще - нет. Со временем возможностей для генерации будет больше, но 100% кейсов не будет покрыто.

А LLM в теории может сгенерировать что угодно, главное "скормить" ей побольше типового кода. Ключевое слово здесь - типового. Т.е. какую-то сложную логику тоже можно сгенерировать в LLM, но если размер диалога будет в 10 раз больше сгенерированного кода, а время - сравнимо, есть ли в этом смысл? Да, модель нужно будет периодически "подкармливать", но видится, что эту процедуру можно автоматизировать взяв за исходные данные открытый код из того же github.

Есть еще нюанс. LLM - это аналог MP3 128 kBit Joint Stereo ))) Сжатие с потерями. Это если что моя оценка вида "пальцем в небо", очень может быть степень сжатия больше. Распаковка - генерация кода - тоже приведет к потерям. Как проверить, что потерь нет - компиляция, тесты, а главное - предварительная оценка того, насколько типовой код нужен. В итоге для простых задач мы получаем универсальный генератор. И это круто!

P.S. Может показаться, что я наехал на Spring Initializr. Нет, штука полезная. Фактически Spring задали стандарт на рынке - все конкуренты Spring сделали свои инициализаторы: https://code.quarkus.io/ и https://start.microprofile.io/ И в Enterprise я видел попытки сделать свои инициализаторы разной степени успешности.

#llm #ai #code_generation

Качественное ли у вас API? А чем докажете?)

Как мы проверяем код на качество? SonarQube, покрытие кода тестами. Если говорить о code style - CheckStyle-ом. Если говорить об уязвимостях - проверка по базам уязвимостей (разные тулы), Checkmarx.

А можно ли как-то проверить API на соответствие лучшим практикам? В частности, OpenAPI как самый типовой на данный момент вариант.
Да - для этого есть Spectral linter https://meta.stoplight.io/docs/spectral/a630feff97e3a-concepts

У него три основных достоинства:
1) это linter и его можно включить в CI pipeline

2) у него есть наборы предустановленных правил, в частности:
а) OpenAPI rules https://meta.stoplight.io/docs/spectral/4dec24461f3af-open-api-rules
б) URL rules https://apistylebook.stoplight.io/docs/url-guidelines - использование kebab-case, не использование get в URL...
в) OWASP rules https://apistylebook.stoplight.io/docs/owasp-top-10 - безопасность, например, использование uuid вместо чисел в идентификаторах
...

3) возможность добавлять свои правила https://meta.stoplight.io/docs/spectral/01baf06bdd05a-create-a-ruleset в том числе наследуясь от существующих

Ну и отдельно отмечу, что есть плагин для IDEA https://plugins.jetbrains.com/plugin/25989-spectral-linter

Итого - штука полезная, настоятельно рекомендую попробовать.

#api #arch #code_quality

code style бывает разный

Меня сложно чем-то удивить в области разработки. Но недавно, изучая разные code style, я решил посмотреть, какие бывают варианты расположения фигурной скобки. Изначально в моём понимании их два - на одной строке с управляющей конструкцией и с новой строки. Но жизнь оказалась богаче любых ожиданий:
https://en.m.wikipedia.org/wiki/Indentation_style
Девять! Девять, Карл!

Хотя в Java наиболее распространён один, пришедший из C, от Кернигана и Ричи.

Вывод будет такой. Не важно сколько разных code style существует, важно, чтобы в проекте использовался один. А это значит .editorconfig https://editorconfig.org/ и автоматическое форматирование в Actions on Save в IDEA.

#code_style #idea

Нужно ли читать чужой код?
Я не про код-ревью, а про процесс разработки.

С одной стороны я всегда топлю за то, что если есть проблемы с внешней библиотекой - не понятно, почему метод возвращает такой результат или ошибку - залезь в ее код, и посмотри.
Но Стив Макконнелл (да, снова Совершенный код) подсвечивает один интересный момент - разницу между синтаксической и семантической инкапсуляцией внутренностей класса.
С синтаксической все просто - объявил по максимуму все private и профит.
А вот семантическая, а точнее ее нарушение - это наши догадки насчет внутреннего устройства чужого класса, влияющие на наш код.
А догадки эти обычно возникают после чтения кода...
Тут еще хорошая аналогия с работой разработчика и тестировщика. Для тестировщика код - это черный ящик. А для разработчика - нет. Или все же да?)

Я для себя решил так. Если найдено неочевидное\неправильное поведение внешней библиотеки, ее автор доступен и готов к сотрудничеству - стоит по советам Стива зайти к нему и попросить поправить.
А если хотя бы одно из этих условий не выполняется - можно и нужно залезть в код. И возможно даже завязаться на его внутренние детали реализации. Да, да, я про костыль.
Но с обязательными условиями:
1) proxy слой
2) с подробным комментарием что делается и почему.
И с заведением техдолга если все же есть возможность поправить эту проблему прямым образом.

#book_review #code

Два подхода к написанию функций (методов).

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

Т.е

public void processUser(User user) {
    if (user != null) {
        if (user.getAge() >= 18) {
            if (validateEmail(user.getEmail()) != null && user.getEmail().contains("@")) {
                if (user.isVerified()) {
                    performUserRegistration(user);
                } 
            } 
        }
  }    
}

vs

public void processUser(User user) {
    if (user == null) return;
    
    if (user.getAge() < 18) return;
    
    if (user.getEmail() == null || !user.getEmail().contains("@")) return;
  
    if (!user.isVerified()) return;
  
    performUserRegistration(user);
}

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

В Совершенном коде автор тоже за второй вариант. Присоединяюсь и считаю, что второй вариант - это база.
Возможно есть кейсы, когда он не применим, но я пока придумать не могу.
Даже на 2 условиях. Даже если недостаточно простого return.

#book_review #code

Снова минутка цитат на канале:

«Термин „структурное программирование“ был введён в 1969 году.
С тех пор термин „структурный“ применялся к любой деятельности в области ПО, включая структурный анализ, структурный дизайн и структурное валяние дурака».
Стив Макконнелл «Совершенный код».

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

Я уже писал про структурный дизайн - https://t.me/javaKotlinDevOps/400.
Напомню, там была речь о том, что не обязательно заводить кучу объектов и интерфейсов, если процесс простой и раскладывается на три части - подготовка данных, преобразования и запись.
Тогда его можно реализовать в виде сервиса, "чистой" модели и репозитория. К слову: ETL = Extract -> Transform -> Load - о том же.

Так вот - структурное программирование тоже раскладывает любой процесс на три части, только уже на уровне метода: последовательный код, условия и циклы.
Казалось бы - ну да, очевидно это так. Но есть нюанс - тут речь идет о проблеме множественных выходов из метода.
Структурное программирование появилось как ответ на широкое распространение оператора go to и призвано бороться с ним.
Опять вопрос - go to мы победили, тогда зачем эти "предания старины глубокой"?
Нет, не победили. Точнее победили, но не до конца.
Т.к. у нас есть множественный return и throw. А если копнуть глубже, и расширить проблему на любые прыжки по коду - break и continue.
Надо ли от них отказываться, учитывая что парадигма то вроде полезная?
На мой взгляд - нет.
Охранные выражения (guard pattern) - однозначно полезны.
throw для сигнализации об ошибке - тоже ок, они для этого создавались.
С рядом условий:
1) unchecked
2) не надо просто для передачи управления
3) должен быть определен слой\процесс перехвата исключений на уровне сервиса
break и continue - если без них код сложнее читать - тоже допустимы. Прыжки то небольшие, в рамках цикла. Хотя я ими пользуюсь редко, кейсов мало.

Т.е. снова приходим к искусству компромиссов.
Всякий раз, перед написанием такого оператора стоит подумать - нет ли других вариантов? Не усложняется ли он чтение и понимание кода?
Суть структурного программирования - код каждой операции должен просто читаться. Нет "спагетти-коду"!

#book_review #java #code #structure_programming #structure_xxx #principles

Сколько багов в час делает разработчик?

И снова цитата из Совершенного кода:

Исследования, проведенные в Институте
разработки П0 (Software Engincering Institute), показали, что разработчики допускают в среднем
от 1 до З дефектов в час при проектировании и от 5 до 8 дефектов в час при кодировании
(Humphrey, 1997). Ясно, что устранение дефектов — обязательное условие эффективного конструирования.

Хоть ты код не пиши!)))

А если серьёзно - рулит код-ревью и тесты. Именно в таком порядке.

Типичный концер процент нахождения багов в ПО при использовании различных практик из той же книги:

Неформальный обзор дизайна (тех. проекта) 35% 
Формальные инспекции дизайна (тех. проекта) 55% 
Моделирование и прототипирование 65% 
Неформальный обзор кода (code review) 25% 
Самостоятельная проверка код 35%
Формальные инспекции кода 60% 
Юнит-тестирование 30% 
Регрессионное тестирование 25% 
Тестирование новой функции (компонента) 30% 
Интеграционное тестирование 35% 
Системное тестирование 40% 
Бета-тестирование (<10 тестеров) 35% 
Бета-тестирование (>1000 тестеров) 75%

Что бросается в глаза:
1) очень высокая эффективность ревью кода. Что понятно, т.к. получаем shift left и, главное, при код-ревью мы не просто находим баг, но ещё и его причину, в отличие от тестирования. Т.е. не тратим время на отладку.
2) формальные ревью сильно эффективнее неформальных. Немного странно на первый взгляд. Но объяснение есть - формальное = выделено время + есть чек-лист + нужен отчёт. Т.е. его сложно провести "формально" (да, велик и могуч русский язык))) Т.е. правильно было бы назвать его глубокое код-ревью
3) Хорошая эффективность у самостоятельного ревью, а его провести проще всего. Например, при подготовке pull request. И снова понятно почему - своей код ты уже знаешь, главное чтобы между кодированием и ревью прошло время, чтобы сменился фокус.
4) низкая эффективность unit тестов, сравнимая с регрессом и тестированием нового функционала. Это странно. Но если подумать - TDD тогда ещё только появилось, да и сейчас его проникновение оставляет желать лучшего. А без него UT - это в основном регресс. И немного тестирование нового функционала, но методом "прозрачного" ящика.
5) высокие цифры у прототипирования и бета-тестах на большом числе людей. Пр сути они связаны - это проверка гипотезы и реализации. Провести сложно, но эффект похоже есть.

Ну и банальная вещь - ни один метод сам по себе не даёт 100%. Только в комплекте.

#code_review #quality_assurance

Чем заменить сложный if?

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

Но ближе к коду)
Вот есть у нас, предположим, такое условие:

int getDaysInMonth(int month) {
    if (month == 1) return 31;
    else if (month == 2) return 28;
    else if (month == 3) return 31;
    // ... ещё 9 веток
}

Сложно, т.к. много кода. Но вот так намного проще:

private static final int[] DAYS_IN_MONTH = {
    31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
};

int getDaysInMonth(int month) {
    return DAYS_IN_MONTH[month]; 
}

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

public int resolveMonth(LocalDate date) {
  return date.getMonthValue();
}

int getDaysInMonth(LocalDate date) {
    return DAYS_IN_MONTH[resolveMonth(date)];
}

Еше кейс. Предположим, у нас целевое значение зависит от положения точки в диапазоне, тогда получаем табличный метод со ступенчатым доступом:

@Getter
@RequiredArgsConstructor
enum Zone {
    NIGHT     ("Ночная",       2.15),
    SEMI_PEAK ("Полупиковая",  4.30),
    PEAK      ("Пиковая",      6.45);

    private final String label;
    private final double ratePerKwh;
}

private static final int[]  HOUR_THRESHOLDS = { 6, 9, 16, 20, 22, 23 };

private static final Zone[] TARIFF_TABLE = { Zone.NIGHT, Zone.PEAK, Zone.SEMI_PEAK, Zone.PEAK, Zone.SEMI_PEAK, Zone.NIGHT };

private static Zone lookupZone(int hour) {
    for (int i = 0; i < HOUR_THRESHOLDS.length; i++) {
        if (hour <= HOUR_THRESHOLDS[i]) {
            return TARIFF_TABLE[i];
        }
    }
}

public static double getZonePrice(int hour) {
    var zone = lookupZone(hour);
    return zone.getRatePerKwh();
}

Ну и возвращать можно не только значение, но и лямбду:

private static final Map<String, Consumer<String>> HANDLERS = Map.of(
    "ERROR",   msg -> System.err.println("[ERROR] " + msg),
    "WARNING", msg -> System.out.println("[WARN]  " + msg),
    "INFO",    msg -> System.out.println("[INFO]  " + msg),
    "DEBUG",   msg -> { /* no-op in prod */ }
);

public static void dispatch(String level, String message) {
    HANDLERS.getOrDefault(level, m -> System.out.println("[UNKNOWN] " + m))
      .accept(message);
}

Важный плюс такого решения - "таблица" легко выносится в файл при необходимости настройки.
Или любой внешний источник.
Таблица может быть мапой, а не только массивом или списком.

P.S. Да, это снова Совершенный код)

#code_review #code

Ты что такое или как AI ревьювит код?

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

Первый. Модель постоянно берет куски нового кода и запускает их с каким-то параметрами, например, с краевыми случаями.
Для начала куски метода, потом методы целиком.
Это явно плюс в карму AI, т.к. живому человеку это делать долго и дорого.
Кто-то так делал вообще?)
Надо отметить, что в Java это еще и сложнее сделать, чем в том же Python или JS, хотя подвижки есть - https://t.me/javaKotlinDevOps/412
Тут видится хорошая область, где AI ревьювер может дополнить человека.

Для демонстрации второго момента приведу кусок кода - это лог выполнения скрипта, который агент прогонял на этапе ревью. Код python, но он простой:

{"command":"python -c \"
import pandas as pd
import numpy as np

# Probe: does max_pos + 1 in rng.integers allow the window to exceed bounds?
# n = 10, duration = 3, max_pos = 7
# rng.integers(0, max_pos + 1) -> [0, 7] inclusive
# pos=7: index[7], window is [index[7], index[7]+3min)
# If index has exactly 10 rows (0..9), this is index[7], index[8], index[9] -> 3 rows OK
# pos=8: This would only happen if max_pos=8, which requires n-duration=8, so n=11 - fine
print('Boundary logic: max_pos = n - duration_minutes')
print('rng.integers(0, max_pos+1) is [0, max_pos] inclusive')
print('At max_pos, window covers indices [max_pos..max_pos+duration-1], which is valid')
print()

\"
","description":"Verify off-by-one and boundary correctness in random placement logic"},

На первый взгляд бред какой-то. Код ничего не делает, только печатает очевидные вещи, частично как комментарии, частично через print.
Но если подумать - похоже модель проверяет логику на отсутствие ошибки "ошибки на единицу".
Проговаривая ее для себя. Ну и возможно для меня, но видится в первую очередь для себя.
Т.к. это все попадает в историю диалога с LLM и далее в ее контекст, влияющие на результат выполнения последующих запросов.
С таким проговариванием человек справляется успешно, но радует, что модель тоже, хоть и вот так, странненько.

P.S. Ошибка между прочим известная, не у каждой ошибки своя статья в wikipedia есть https://ru.wikipedia.org/wiki/Ошибка_на_единицу

P.P.S. Но импорты явно лишние)

P.P.P.S. Риски самому перестать думать возрастают)))

#ai #code-review