2022-10-30 16:17:29
Не писать код «про запас»«Не нашла где этот метод используется. Написан/сохранен на будущее?» - такой комментарий однажды я получил в code review. И эта формулировка прилипла, как банный лист к На протяжении нескольких лет я оставлял такой комментарий в ревью, если замечал, что какой-то код не используется. Комментарий должен быть радикально другой: «этот код не используется, поэтому его нужно удалить».
Код - это обязательство, а не актив. Любой код требует сопровождения. И речь не только про актуализацию и расширение для новых фичей. Есть очень большая статья расходов, которую обычно недооценивают - чтение, изучение, осознание. Причем несет эти расходы вся команда и постоянно.
Устаревший код нужно удалять.
Git все помнит, при необходимости можно будет восстановить нужный кусок. То же самое применимо и к закоментированному коду, если он не часть документации.
Переусложнение - другая форма «кодоконсервов на будущее». Интересно наблюдать, как разработчики отстаивают избыточное усложнение простых вещей.
С человеческой точки зрения - это можно понять. Выглядит красиво, время уже потратили, жалко выбрасывать. На мой взгляд, к подобному коду нужно относиться радикально - также как и к тому, что захламляет вашу квартиру/рабочий стол/жесткий диск. Удалять не жалея.
С практической точки зрения, главный аргумент переусложнения звучит так: «в будущем за счет этого усложнения код будет проще расширяться». Аргумент так себе, разработчики очень плохо предсказывают будущее. Любые трудозатраты, которых можно было избежать, любой код, который можно было не писать - это производственный мусор и не завершенная работа. Код должен легко расширяться
за счет простоты и элегантности, а не за счет переусложнения и введения концепций сверх необходимого. KISS! Рефакторинг, а не оверинжениринг, должен быть нормой ежедневной работы.
Как отличить переусложнение от необходимости? Главный критерий - своевременность. Если решение легко изменить в будущем, то стоит остановиться на простом варианте. Если решение сложно изменить в будущем, а вероятность изменения высока - лучше перезаложиться.
Схожая проблема -
код, который не несет смысловой нагрузки.
Например, это проверка аргумента в методах, которые не используют его, а передают дальше по стеку вызовов. Подобный код можно удалить без изменения итоговых характеристик системы. И его
нужно удалять, ведь помимо того, что он мешает чтению и развивает “банерную слепоту”, через него часто еще и
утекают детали реализации. Проверка аргументов - задача их непосредственного потребителя.
Особенно ярко описанные проблемы встают, когда приходит
новый разработчик:
- закоментированный не актуальный код отвлекает и сбивает с толку;
- приходится тратить время на обучение новым концепциям, которые заменяют простые вещи, но не оправдывают себя в 80% случаев;
- вместо радости от первой зарелиженной фичи - нудное изучение тысяч строк кода, который ни на что не влияет в продукте.
Такой себе онбординг…
Есть и
исключения, когда не используемому коду есть место:
1. Проверить, используется ли код, сложно. Например, вы разрабатываете библиотеку для широкого круга потребителей. Но и в этом случае стоит относится к библиотеке как к продукту: собирать обратную связь от пользователей и выпиливать не используемые фичи. Да, да, выпиливать неиспользуемые фичи из продукта я тоже считаю нормой.
2. Задачи или релизы разделены так, что неиспользуемый код «оживет» в дальнейшем. Ситуация странная, скорее всего имеет место неверная декомпозиция. Но иногда такое случается. Пример хорошего разделения - зависимость от контракта выраженного в коде: один разработчик реализует хранилище и пишет в него данные, другой разработчик читает их; контракт проговаривается и фиксируется в коде заранее. Плохой вариант разделения - написать методы/классы, которые «скорее всего» понадобятся другой команде после релиза.
К счастью, можно настроить linter/IDE/CI так, чтобы исключить вероятность попадания в master не используемого кода. И не стоит принимать аргумент «это понадобится в будущем» - не понадобится с вероятностью 99%.
95 viewsAlexander Lozhkin, edited 13:17
Т
