Это не я! История одного рефакторинга
Кто и как пережил весну 2020-го, обсуждать можно бесконечно. Меня, например, карантин застал почти сразу после переезда в другой город, и на несколько месяцев запер в однушке с котом. Думаю, мы оба многое переосмыслили.
Началось с того, что экстренно потребовалось собрать форк приложения Aword, для которого уже больше года не выходило новых версий. По сути, сильно урезанную версию основного приложения Skyeng: в форке должны были быть тренировки, фид с контентом (сториз, видео и статьи), словарь и аудирование.
С многомодульностью такие задачи решаются очень просто, и форк мы собрали в течение буквально нескольких часов. Успели порадоваться тому, что не стали когда-то тянуть с рефакторингом проекта. Записали в ноушн возникший по ходу дела техдолг.
Но одному модулю было нехорошо.
Аудирование.
В основном приложении его нет, поэтому мы оттягивали момент рефакторинга достаточно долго. И написанный в незапамятные времена модуль listening никто не трогал, не подключал и не тестировал уже год. За это время все остальные модули проекта, в том числе те, от которых зависел listening, успели сильно измениться.
Вот что предстояло воскресить.
Аудирование — вполне самостоятельная и не то чтобы маленькая фича: список аудиофайлов с пагинацией, экран с плеером и субтитрами (и сервис для плеера), экраны для упражнений и тестов после прослушивания, свои, отдельные от остального приложения, настройки, фильтры по тегам и категориям…
— Мы никак не можем обойтись без листенинга в этом приложении?
— Никак.
— Его давно никто не собирал… Доков нет. Растительности нет. Населена роботами.
— Ага.
— А кто его последний трогал?
— Я и трогала. Был прошлой весной один баг в субтитрах…
Так я подписалась на то, чтобы по самые уши влезть в гору заброшенного кода.
Типичный диалог того времени выглядел так:
— Как ты себя чувствуешь?
— Я рефакторю листенинг вторую неделю. Не очень. А кто его писал, кстати?
— Это не я!
И вот почему.
Документация и тесты
Документации в модуль аудирования не завезли — в отличие от остального проекта. Единственным видом комментариев был копирайт. И благодаря ему я знала, кого хочу убить раньше, чем до меня доберутся те, кто вынужден поддерживать мой код на прошлых местах работы)
Тесткейсы. Чаще всего у нас они имеют названия, лаконично обрисовывающие сценарий использования. Если сущности неудачно названы, javadoc отсутствует или код не поддаётся беглому прочтению — они приходят на помощь, что делает тесты ничуть не менее важной частью доков, нежели комментарии или вики. В данном случае мне этого остро не хватало с самого начала.
И я подумала, что покрывать код тестами важно не только для проверки, что ничего не сломалось, но и для того, чтобы иметь дополнительный источник знаний о функциональности модуля. Особенно, если планируется его выключить, какое-то время не трогать и даже не гонять в нём тесты.
Переписать это наскоком было невозможно, и я начала с того, что закомментировала кучу неработающего кода.
Код в модуле
Представлял собой жёсткое легаси, которое опиралось на давно выпиленные из проекта базовые классы. Вот такие, например:
public abstract class LceActivityWithTracking , P extends LcePresenter, V>>
extends LceActivity Честно сказать, я далеко не сразу поняла, что принимает описанный выше класс, и в какой последовательности. В какой-то период я тоже стремилась использовать дженерики буквально повсюду, усмотрев в них хороший способ фиксировать важные ограничения. Но теперь знаю, что дженерики такой сложности и вложенности хороши лишь тогда, когда их пишешь. А не читаешь.
Скорее всего, написанный код был крайне очевидным для его автора. Для меня он очевидным не был. Зато навёл на не менее очевидную мысль, что лучше бы не забивать на доки, даже если всё кажется очевидным. Возможно, стоит даже писать javadoc ко всему, что вызвало вопросы на код-ревью —, но это плавный и непрерывный процесс, который сложно исправить одним волевым усилием. Особенно если надо на какое-то время срочно законсервировать модуль.
Или вот, например. Маленький класс, в котором всего было около пятидесяти строчек:
public class GetSubtitlesUseCase extends SerialUseCase, SubtitlesIds> Всё вроде хорошо, правда? Но SerialUseCase скрывал под собой ещё четыре уровня наследования, и все они были выпилены не меньше полугода назад за полной невостребованностью. Пришлось раскапывать.
Примерно везде меня ждали пагинация, кэширование, lce-паттерны и утилитные методы, спрятанные под капотом ныне несуществующих классов.
К счастью, я достаточно активно ввязалась в рефакторинг проекта, и многие из попавшихся базовых классов выпиливала и переписывала тоже я — это принесло некоторое представление о том, как они были устроены.
И это подтолкнуло к выводу: если есть вероятность того, что выключенный из проекта код долго не будет пересобираться, проверяться и использоваться — лучше сразу положить в него пакет с задействованными в нём базовыми классами. Их может внезапно не оказаться после рефакторинга. А может не оказаться ни документации, ни людей, которые помнят, что там в принципе было)
Переписывая экраны
На моё счастье, модуль очень слабо зависел от основного проекта в плане информации — всего две входящие зависимости. Поскольку он не нуждался практически ни в чём от окружающего мира, было достаточно легко восстановить всё, связанное с dagger.
С навигацией было немного посложнее. Подход с single-activity ещё не успел войти в моду, и на отдельных активити были написаны даже самые элементарные экраны, вроде голосовалки, понравился ли учебный материал.
Потом я стала потихоньку раскомменчивать и переписывать экраны на kotlin, сверяясь с версией с прода и картой функциональности, а иногда и просто выкидывать старый код. Сначала — главная с выбором материала. Потом — экраны попроще. Потом — посложнее. Некоторые классы просто остались болтаться, как лишние детали после пересборки.
Чтобы не сойти с ума, мы созванивались с тимлидом и дробили задачи на более мелкие с более понятным definition of done. Интересные задачи приносили фан сами по себе, с неинтересными и нудными я придумывала челленджи. Жизнь на тот момент так или иначе состояла в основном из четырёх стен, кота и кода. А большую часть времени в четырех стенах естественным образом занимала работа: как деятельность с наибольшей и гарантированной эмоциональной отдачей.
В конце концов модуль, доведённый до примерно рабочего состояния, влился в мастер, мы выдохнули…
И устроили совершенно безумнейший костюмированный созвон, где шапочка из фольги была, пожалуй, самым скромным атрибутом.
Мы делились страшными и странными историями из жизни и профессионального опыта. А затем я сделала для ребят внутренний доклад про этот опыт. Ну, как доклад. Мне просто хотелось пожаловаться и не держать это в себе.
Ну, а этот пост пишу, чтобы еще раз вынести в паблик выводы, которые набились в процессе прыганья по граблям: возможно, когда-то вы будете консервировать что-то в приложении. Потратьте время и причешите исходники, чтобы потом не думать, что это за неиспользуемый класс с кучей кода. Потратьте время на тесты и документацию, подчистите код, положите рядышком копии базовых классов.
И тогда тот, кто столкнётся с законсервированным модулем, будет чуть меньше хотеть отыскать ваш адрес и прийти к вам с топором. Или добыть ваши волосы для ритуала вуду… так, что-то меня понесло.
p.s. И да, чувак, которому не повезёт заглянуть в этот модуль следующим. Listening писала не я. Я его рефакторила ;)
