Многие разработчики в нашей команде избегают рефакторинга. Обсуждение этой ситуации выявило следующие причины.

Почему многие не делают рефакторинг

  • страшно поломать существующую функциональность

  • трудоемко: если код, который ты рефакторишь, меняется, то мержить (многократно) его из основной ветки становится болью

  • есть риск не успеть к дедлайну задачи

При этом рефакторинг - практически неотъемлемая часть процесса разработки. Его необходимость связана со следующими предпосылками:

  • начальные требования практически никогда не бывают полными и разработка ведется в соответствии с определенными предположениями, некоторые из которых впоследствии оказываются не верными или не точными

  • часто требуется внести быстрые правки, чтобы заработало прямо "здесь и сейчас", без учета дальнейшей поддержки

Когда рефакторинг оправдан (необходим)

Я вижу следующие причины для изменения структуры кода:

  1. дублирование кода

    Для наличия дублированного кода есть мало причин, чаще всего от него нужно избавляться

  2. универсализация

    Чтобы облегчить понимание кода и уменьшить количество проблем при эксплуатации, часто можно создать абстракцию, объединяющую похожий код. Важно иметь как минимум 3 прототипа для этой абстракции, иначе она может оказаться не точной

    Например, мне нужно было сделать интеграцию платежного шлюза с GooglePay. В процессе выяснилось, что процесс оплаты ApplePay и SamsungPay практически идентичен и изменения в одном из них часто требуются и в двух других. В итоге я сделал оплату для абстрактного VendorPay со стратегиями для каждого метода оплаты

  3. код не понятен

    Под этим я понимаю ситуацию, когда ты несколько раз натыкаешься на непонятный кусок кода и каждый раз приходится докапываться до сути происходящего

  4. переусложненный класс/метод

    Для меня достаточно таких формальных признаков, как больше 50 строк в методе или больше 500 строк в классе, чтобы задуматься над рефакторингом

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

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

Как быть?

Чтобы минимизировать проблемы, связанные с обоснованным рефакторингом, стоит придерживаться следующих приниципов:

  • иметь тесты (unit и регресс) с хорошим покрытием

  • не смешивать пулл реквесты с рефакторингом и бизнес-логикой

  • мержить рефакторинг сразу, не дожидаясь бизнес-задачи

Мой подход к рефакторингу

По нашему процессу мы создаем feature-ветку от master'а для каждой задачи.

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

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

После разделения изменения на две ветки я прошу коллег проревьюить ветку рефакторинга, мержу ее в master и делаю rebase бизнес-ветки на master.

UPD: пришел к такому процессу отделения рефакторинга от бизнес-изменений:

  1. делаю изменения (в течение <=3 дней) в ветке feature, в ней и бизнес-изменения, и рефакторинг

  2. создаю ветку refactoring от master

  3. копирую все изменения feature -> refactoring, используя один из методов:

    • cherry-pick feature -> refactoring нужных комитов

    • применение патча к ветке refactoring, который можно создать, например, используя команду git diff master..feature > feature.diff

  4. убираю из ветки refactoring бизнес-изменения (вручную)

    • пункты 3. и 4. занимают у меня примерно 10% времени, потраченного в пункте 1

  5. ревью и мерж ветки refactoring

  6. обновляю ветку feature из master

Результат

Этот подход несет некоторые накладные расходы, но смягчает основные трудности при рефакторинге:

  • ревьюить и мержить рефакторинг не так страшно, потому что он, в основном, не несет функциональных изменений, либо эти функциональные изменения четко видны в небольшом пулл-реквесте

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

  • в случае когда сроки подпирают, можно прекратить делать рефакторинг после очередного двух-трех дневного цикла