Comments 15
> Простите, накипело…
А у себя как решаете, нашли такого человека?
У нас стратегически другой подход: была сделана ставка на жесткий код-ревью. Это не значит 'взамен' рефакторингу. Но время на такой код-ревью существенно сокращает свободное.
У нас команды, например, к такому подходу пришли.
Последние года 3 работал в командах, где желание рефакторить если не всей команды, то значительной её части пресекалось со стороны бизнеса, начиная с тимлида. Ресурсы на рефакторинг хорошо если выделялись по остаточному принципу типа "сделаешь бизнес-таски быстрее — порефакторишь". На текущем проекте гораздо лучше, но, по-моему, баланса всё равно нет.
В последнее время заметил, что рефакторинг у нас выглядит формально как рефакторинг ради рефакторинга. В тасках на рефакторинг написано что-то вроде "отрефакторить компоненты/модули/подсистемы такие-то" без цели этого рефакторинга, без метрик, по которым можно понять удачный был рефакторинг или нет, без ограничений что нужно отрефакторить, что можно, а что нельзя трогать в рамках этой таски.
Ну и по результатам получается, что вроде рефакторинг проведён, но код большей частью становится просто другой. Для автора он лучше, но не все это видят, а некоторые считают, что неправильно концептуально сделан.
Встречались с такими проблемами?
Буквально на днях решал таск на "просто рефакторинг" одного модуля на питоне. Функциональность и апи в результате никак не изменились, но в целом стало гораздо читабельнее. "Просто рефакторинг" у нас нужен, когда какая-то задача была решена, но спустя пару дней ты сам не можешь понять что к чему. А хорошая читабельность кода, соответственно, ведёт к лучшей поддержке и возможности дальнейшего расширения.
Ну и да, разные люди считают, что их подход единственный наиболее правильный. Но даже один и тот же подход можно выразить так, что это будет эстетично и понятно, или так, что ногу сломать можно.
но в целом стало гораздо читабельнее
Это одна из возможных целей рефакторинга. Я бы сказал что такую задачу нужно формулировать как "Сделать код модуля N читабельней", а не "отрефакторить модуль N"
Можно было бы сформулировать, но исторически закрепилось такое название, никто не против.
А ещё, если заявлять читабельность как непосредственную цель, то возникает вопрос, а что считать объективной, вычисляемой метрикой читабельности? Количество применяемых сущностей/методов на 100 условных строк модуля, глубина стека вызовов умноженная на разбросанность вызываемых методов по модулю, "плотность кода", что бы это ни значило, или количество людей, которые согласны, что стало лучше?
А так написал "отрефакторить" и можно действовать на своё усмотрение. Сел, подумал и переписал всё с нуля, затронув несколько смежных модулей. Или наоборот, только пару строчек подправил, чтобы более идеоматически смотрелось.
Ну можно считать такой метрикой прохождение код-ревью у (всей?) команды. Вашими словами "количество людей, которые согласны, что стало лучше". (Человеко)читабельность ведь субъективная величина по определению.
Я к тому, что у "отрефакторить модуль N" могут быть и другие цели, навскидку:
- снизить количество ответственностей если не у самого модуля, то у составляющих его элементов
- передать инициализацию зависимостей от самого модуля к его клиентам, фабрике, DI-контейнеру и т. п. или наоборот
- минимизировать публичный интерфейс, сделать его абстрактным, усилить инкапсуляцию
- ускорить прохождение тестов (читай — упростить инициализацию тестов) без уменьшения покрытия
- перевести старый код на новые соглашения, стили, архитектурные и подобные принципы для унификации
- изменить модель на более подходящую предметной области
- …
и ещё десятки и, может, сотни целей рефакторинга, часть из которых вполне объективны и часто бинарны — достигнуто или нет.
когда какая-то задача была решена, но спустя пару дней ты сам не можешь понять что к чему.
а тесты были написаны?
Как ни странно, да.
Добавлю деталей, чтобы пояснить на примере. Модуль слушает клавиатуру и в зависимости от нажатых клавиш формирует пакет с командами. Было реализовано с возможностью расширения вширь (если пакет станет больше или изменится его структура) и с возможностью динамической подмены биндингов (то бишь, чтобы те же команды отправлялись с других клавиш). И вот это было реализован через ж кучу отдельных списков, каждый из которых отвечал строго за свою функцию (список с используемыми кнопками, список с названиями команд, мапа с соответствием между командами и индексом числа в пакете...). Любой из этих списков можно было подменять в отдельности просто подставив новый, но этот новый нужно было проверить на соответствие по длине всем остальным спискам. После подстановок эти списки динамически превращались в одну волшебную мапу, с помощью которой конечная функция "нажми на кнопку — получишь результат" выглядела достаточно просто.
Ревью этот подход прошёл, но быстро выяснилось, что надо добавить формирование ещё одного аналогичного пакета там же, внося изменения в списки и вручную следя за тем, чтобы всё всему соответствовало. Было сложно, блин, изменить значения по умолчанию. Приняли решение выпилить кучу этих списков из внутренностей, оставив две явно задаваемые волшебные мапы, пусть и усложнив процесс подмены.
мы получаем какой-то чудовищный перекос в сторону «наведения порядка» в уже написанном коде вместо того, чтобы двигать проект в светлое будущее
Если в компании первое и второе противопоставляется — нужно бежать из такой компании
Рефакторим параллельно с разработкой: наш опыт и два чек-листа