Некоторое время назад мой хороший друг (у него, кстати, есть интересный канал о разном) во время беседы о рефакторинге упомянул вот это видео. Доклад был на CppCon'е, но он почти не о C++ и посвящён ситуации, в которой однажды каждый из нас окажется или уже был: "So you inherited a large code base...". Автор пытается ответить на вопрос: "И что делать-то?"
Вы можете посмотреть доклад, а потом почитать мои измышления, а можете не смотреть, потому что то, что хочу выделить, я подробно опишу ниже.
Автор начинает с того, что транслирует отличную мысль: "Assumption is the mother of all mess ups". И объясняет эту мысль отличным же примером.
Есть унитазы с автоматическим доводчиком ободка, когда достаточно его толкнуть, чтобы началось аккуратное и тихое движение вниз. Если вы ставите такой унитаз дома, то вам удобно и хорошо, но велика вероятность, что придя в гости, вы по привычке точно так же толкнёте ободок на чужом унитазе и получите звук оглушительного удара на весь дом и чувство ужасной неловкости. Помните: в чужом проекте весь ваш предыдущий опыт и основанные на нём предположения могут оказаться неверными.
Список опасных предположений, которые автор далее излагает можно (нужно) использовать не только во время процесса рефакторинга, а вообще всегда, например, просто вкручивая фичу в существующий код. Фича завязана на имеющийся код? Ты не можешь верить в имеющийся код, вот держи чек-лист:
Вы не можете предполагать, что документация соответствует действительности -- вы можете только надеяться. Вы не можете предполагать, что миграция будет простой -- вы можете только надеяться. Вы не можете предполагать, что существует лишь три кейса использования этого кода -- вы можете только надеяться. А надежды часто не оправдываются, так что будьте начеку и знайте, что вы в опасности.
Теперь про рефакторинг глобально. В моём мире рефакторинг -- это в первую очередь про плохую архитектуру. В проекте проблемы, потому что связи в нём не, хм, "нормальные", а представляют из себя старый логотип Slack. Все модули знают информацию, которую им знать не надо; штуки, которые должны быть вот тут, лежат непонятно где; в датафлоу чёрт ногу сломит. И для меня рефакторинг -- это про разлепление сросшегося и склеивание потерявшегося на уровне проекта. Автор высказывает мысли в духе "начинайте с простого" и "начинайте с простого рефакторинга в рамках модуля, это можно автоматизировать". Но вот это вот "в рамках модуля" -- это не то, что выносится в рамки отдельных задач, вдумчиво планируется, делается по всему проекту одноразово и пр. В моём мире это daily routine. Ты вкручиваешь фичу в модуль -- и такой: "О, тут в модуле шлак, я его вижу, исправлю-ка". Но, как всегда, нужен баланс, чтобы не впадать в крайности.
Девид транслирует правильную мысль -- "не отвлекайся во время рефакторинга". Если я рефакторю какую-то корневую бизнес-логику и меня заносит во второстепенный модуль, где обнаруживается шлак, то я не кидаюсь причёсывать этот модуль, пусть даже и кажется, что это просто, легко, исправлю, получу дофамин, вот моя маленькая победа на фоне неподъемного рефакторинга. Во-первых, такими мелкими исправлениями в других модулях замусоривается коммит с Большим Рефакторингом, в который другим людям ещё вникать на код-ревью. Не нужно усложнять им жизнь. Во-вторых, не нужно усложнять жизнь себе: чем больше изменений, тем больше ошибок, тем меньше уверенность, что код будет работать правильно. Не размазывайте изменения по всему проекту, сконцентрируйтесь на своей большой приоритетной задаче с корневой бизнес-логикой. Если очень жжётся, создайте себе отдельную задачу и вернитесь в этот модуль после рефакторинга, сделайте исправление отдельным коммитом.
Моя логика примерно следующая: я делаю изменения по поводу своей задачи в этом модуле и в затрагиваемом коде вижу шлак -- исправлю его прямо сейчас; я делаю изменения по поводу своей задачи в одном модуле, но использую вызовы функций из другого модуля, в котором обнаружила шлак -- не исправляю его, запоминаю-записываю, делаю отдельно следом.
Еще про технический долг. Чем дальше в лес, тем больше меня коробит от этой фразы. Все о нём говорят; приходят в комменты к пулл-реквесту и истерят, что эта глобальная переменная повышает технический долг, уберите; требуют, чтобы в цели команды был добавлен пункт "уменьшение технического долга"; автор видео вот говорит, что "если это увеличивает технический долг, то делайте revert". Но хоть кто-нибудь из вас знает, как его измерять? Лично вы можете сказать, сколько у вас технического долга в проекте? На сколько эта глобальная переменная повышает технический долг? А если мы знаем, что эта переменная делается в скрипте на 100 строк, который только читает файл и запускает обработчик? И мы знаем, что эта читалка не будет меняться во веки веков, потому что вся логика в обработчике? Чем тут мешает эта глобальная переменная? Почему она вам мешает? Что такое тот технический долг, который вырастет от этой глобальной переменной?
Подавляющее большинство штук, которые приносят в комментарии к пулл-реквестам с рефакторингом или большими новыми фичами тянут на байкшеддинг. (Это когда люди трятят кучу времени на обсуждение не важных деталей, а тех, которые они понимают.) Никто не хочет вникать в сложную логику большой фичи, но "нужно же что-то откомментировать, хоть вот глобальные переменные пну". Фу такими быть.
Ну, и вообще я офигеваю, дорогая редакция, от фразы "увеличило технический долг -- revert". Вы в своём уме? У вас клиенты, им нужна фича, они вам деньги дают, но я сейчас откачу это рабочее решение, потому что оно увеличило технический долг и буду ещё 2 недели пилить идеальное. На клиентов плевать, у нас технического долга меньше! (И мы всё ещё не умеем его измерять, да.)
Картинка в тему:
Да, это смешно. Да, сначала вызывает "аааа, штаааа". А потом ты думаешь, что: 1) клиент доволен сейчас тем, что сделали, 2) будет доволен потом тем, что ускорили. И всем хорошо. И что вы там про увеличение технического долга и revert говорили? Главное -- это радость клиента, благодаря которому у вашей компании есть деньги, а вовсе не чувство прекрасного конкретного разработчика.
И воспримите правильно и здраво: если быстро клеить фичи левой пяткой в погоне за их количеством, то в какой-то момент вы обнаружите себя в немасштабируемом, неподдерживаемом, полном багов проекте. Так что нужно балансировать между "клеить быстро и радовать прямо сейчас" и "делать нормально сейчас, чтобы клеить быстро и радовать через полгода" (здравый смысл никто не отменял).
Ещё в докладе безумно понравился вот этот слайд:
Это прям огнище, выдавать в зубы всем и каждому. Очень полезно, коротко, по делу, хорошо, себе сохранила.
То есть вроде понятные и очевидные вещи, ты так и поступаешь, но Девид хорошо изложил в короткой текстовой форме. И можно выдавать начинающим, которым описанное очевидным ещё не кажется. Ниже подробнее и по-русски.
Если нужно заменить кусок какой-то функциональности, то сначала я намечаю себе границы этой функциональности. Кто будет затронут изменениями -- функция, модуль, несколько модулей? Затем осознаю, что происходит -- рисую сиквенс диаграммы, осознаю, как движется поток данных, смотрю на интерфейс, ищу, кто, где и как использует код, который я собираюсь изменить.
После этого я огораживаю тестами всех кандидатов на изменения. Обычно на вопрос "Зачем нужны тесты?" отвечают "Чтобы найти баги". Но это не главное. Главное в тестах то, что они закрепляют поведение вашего кода. И если вы код меняете, то тесты -- гарант уверенности в том, что новый код будет работать так же, как старый. С тестами не страшно делать рефакторинг. Если тесты на старый код написаны хорошо и дают 100% покрытия, то вы сразу узнаете, что в новом коде что-то пошло не так. Это вселяет уверенность. (Можно, например, использовать эту штуку, чтобы быть в курсе процента покрытия тестами вашего кода.)
После того, как старый код полностью покрыт тестами, я делаю новую реализацию, сохраняя старый интерфейс. Это нужно для того, чтобы ничего не потерять, корректно протестировать новый вариант и быть уверенным в том, что новый код работает так же, как старый (а поведение старого кода -- это референс поведения). Напомню ещё раз: чем меньше площадь изменений, тем проще проверить и тем меньше вероятность ошибки.
И в самом конце, получив новую рабочую и проверенную реализацию, меняем интерфейс на новый-удобный-блестящий. Рефакторинг сделан, поздравляю.