Код без тестов — легаси +21


AliExpress RU&CIS

Если вы работаете в IT, то о легаси вы слышите часто — обычно с множеством негативных коннотаций. Понятно, что это не «хороший код», но какой? Может старый, может не поддерживаемый или не обновляемый, а может просто чужой? Есть ли «полноценное» определение «легаси», на которое можно ссылаться? А когда разберемся — что нам делать с легаси? Попробуем разобраться. Спойлер: выводы неочевидны.

Автор — Николас Карло, веб-разработчик в Busbud (Монреаль, Канада). Специализируется на легаси. В свободное время организует митап  Software Crafters и помогает с конференциями SoCraTes Canada и The Legacy of SoCraTes.

Данная статья была скомпилирована (и отредактирована) из двух статей Николаса: «What is Legacy Code? Is it code without tests?» и «The key points of Working Effectively with Legacy Code». Показалось логичным рассказать о том, что такое легаси, а потом — как с ним работать.

Что такое «легаси»?

Возможно, если вы задавались этим вопросом, то встречали определение от Майкла Физерса. Майкл выпустил книгу «Working Effectively with Legacy Code» в 2004 году, но она до сих пор актуальна. Комикс это отлично иллюстрирует.

В своей книге Майкл пишет своё определение:

«Для меня легаси — это просто код без тестов».

Почему Физерс так считает? Потому что по его многолетнему опыту без тестов обычно трудно узнать всё, что код умеет. Если тестов нет, то для понимания, что код делает, вам нужно внимательно его прочитать, воспроизвести программу в своей голове и представить все возможные сценарии. Потом вы поменяете код и нужно снова представить все сценарии. Или проверить их вручную, но всегда есть шанс что-то сломать.

Это хорошее определение: чаще всего тесты отсутствуют, так что это хорошее начало. Но это ещё не всё — есть нюансы.

Код с тестами также может быть легаси. Если вы читаете тесты, но не можете понять, что должен делать код — они отстой. Плохие тесты только мешают: тестируемый код так же трудно отрефакторить, как если бы у него не было тестов, а может даже и сложнее!

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

Перейдём к моему определению легаси.

Легаси — это ценный код, который вы боитесь менять.

Например, мы ищем первопричину ошибки или выясняете, куда вставить свою функцию. Мы хотим поменять код, но это трудно, потому что непонятно как не нарушить существующее поведение. Готово — у нас легаси!

Но есть нюансы.

Мы переоцениваем сложность незнакомого кода. Поэтому мы думаем, что код, который писали не мы — устаревший. Это работает и с нашими прошлыми проектами, когда мы не можем понять, что закладывали и имели в виду, когда писали эту мешанину на экране.

Хорошие тесты помогают легко менять незнакомый код. А плохие тесты не помогают. Отсюда и определение Физерса. 

С легаси помогает время. Парадоксально: обычно время превращает любой код в легаси, но чтобы его понять нам также помогает время. Если вы начали работать над легаси и это трудно — подождите. Да, большая часть кода ужасна, но вы привыкнете и лучше поймете его причуды и особенности.

Легаси не виновато в том, что оно такое. Большая часть кода ужасна, потому что это результат работы многих людей в течение долгого времени с противоречивыми требованиями и под давлением дедлайнов. Это Рецепт Устаревшего Кода™. Когда мало времени и недостаточно знаний — рождаются костыли (ну вы знаете). В конце концов, мы достигнем состояния, когда каждое движение приводит к ошибке, а реализация любой функции занимает целую вечность.

А теперь один из важнейших нюансов.

Легаси — это код, который мы изо всех сил пытаемся понять, чтобы поменять.

Легаси — это личная точка зрения. Устаревший код может стать проблемой для каждого разработчика команды. Какой-то код может показаться сложным, потому что мы его ещё не поняли, а какой-то понимаем, но всё равно чувствуем себя некомфортно, когда рефакторим. Но субъективное ощущение «легаси» зависит от нашего понимания кода, и наших чувств по поводу его изменения. Часто люди этого не понимают.

В итоге мы получаем, что легаси это:

  • код без тестов;

  • который мы пытаемся понять, чтобы отрефакторить;

  • но боимся.

Что же делать?

Как же эффективно работать с легаси?

Легаси — код, который мы пытаемся понять, чтобы отрефакторить. Задача рефакторинга в том, чтобы сохранить существующее поведение кода. Как без тестов мы будем уверены, что ничего не сломали? Нам нужна обратная связь. Автоматизированная обратная связь — ещё лучше.

Добавить тесты, а затем внести изменения

Логично, что если добавить тесты, они помогут его «прощупать» и он перестанет быть устаревшим. Поэтому первое, что нужно сделать — написать тесты. Только тогда мы будем в безопасности, чтобы рефакторить код.

Но чтобы запустить тесты, мы должны поменять код. Возникает парадокс легаси. Мы обречены? Нет. Поменяем как можно меньше кода для тестов:

  • Определим точки изменения — «швы».

  • Разорвём зависимости.

  • Напишем тесты.

  • Внесём изменения.

  • Отрефакторим.

Первые два пункта самые сложные, а как только доберёмся до тестов, мы знаем, что делать.

Найти «швы» для разрыва зависимостей

Обычно когда мы добавляем тесты к легаси возникает «проблема зависимостей»: код, который мы хотим протестировать, не может работать, потому что ему нужно что-то сложное для тестирования. Иногда это соединение с базой данных, иногда вызов на сторонний сервер, а иногда — параметр, который сложно создать. А чаще всё и сразу.

Чтобы протестировать код, нужно разбить эти зависимости в тестах. Для этого необходимо выявить «швы».

«Шов» — место, где можно изменить поведение программы, не меняя код.

«Швы» бывают разные. Если это объектно-ориентированный ЯП, то обычно это объект, например, в JavaScript.

export class DatabaseConnector {
  // A lot of code…

  connect() {
    // Perform some calls to connect to the DB.
  }
}

Допустим, метод connect() вызывает проблемы, когда мы пытаемся поместить код в тесты. Получается, что весь класс — это «шов», который можно поменять. Можно расширить этот класс в тестах, чтобы предотвратить его подключение к реальной БД.

class FakeDatabaseConnector extends DatabaseConnector {
  connect() {
    // Override the problematic calls to the DB
    console.log("Connect to the DB")
  }
}

Есть и другие виды швов. Если язык позволяет изменять поведение кода без изменения исходного кода, у нас есть точка входа в написание тестов. Кстати о тестах…

Напишем unit-тесты

Дискуссии о лучших практиках тестирования обычно перерастают в холивары. Применять принцип пирамиды тестов, и писать максимум unit-тестов? Или использовать «Кубок тестирования» и писать в основном интеграционные?

Почему советы такие противоречивые? Потому что у них нет единого определения того, что такое «unit». Одни люди говорят об «интеграционных тестах» и тестируют всю библиотеку, а другие тестируют каждый класс по отдельности.

Чтобы избежать путаницы, Майкл даёт четкое определение того, что такое НЕ unit-тест:

  • он не работает быстро (< 100ms / test);

  • он взаимодействует с инфраструктурой, например, базой данных, сетью, файловой системой, переменными;

Напишите максимум тестов, которые обладают этими 2 качествами, при этом неважно, как вы их назовёте.

Иногда трудно написать такие тесты, потому что не понятно, что код должен делать. Тогда используйте специальную технику.

Тесты для определения характеристик

Это тесты, которые формализуют фактическое поведение части кода.

Вместо того чтобы писать комплексные модульные тесты, мы фиксируем текущее поведение кода — делаем снимок того, что он делает. Тест гарантирует, что это поведение не изменится!

Это мощная техника, потому что:

  • В большинстве систем то, что код делает важнее того, что он должен делать.

  • Мы можем быстро покрыть легаси с помощью этих тестов. Так мы подстрахуемся для рефакторинга.

Этот метод также называют «Approval Testing» («тестированием одобрения»), «Snapshot Testing» или «Golden Master».

Но обычно на всё это очень мало времени.

Когда совсем нет времени на рефакторинг

Несколько советов, если предыдущие не подходят.

Большие куски кода обладают «гравитацией» и привлекают ещё больше кода. «Теория разбитых окон» в действии: небольшой беспорядок влечёт за собой беспорядок серьёзнее. Если класс уже содержит 2000 строк, то какая разница, что вы добавите еще 3 if оператора и будете поддерживать класс длиной в 2010 строк?

Это всего лишь 3 if: тяжело себя убедить, что нужно потратить на них 2 дня, хотя и должны. Что делать, если действительно нет времени писать тесты для этого класса? Используйте техники Sprout (прорастание), Wrap (обёртывание) и скретч-рефакторинг.

Sprout

Напишите код в другом месте, сделайте один тест, определите, где вы должны вызвать этот код из существующего кода (точка вставки), и вызовите свой код из легаси.

Рассмотрим на примере:

class TransactionGate {
  // … a lot of code

  postEntries(entries) {
    for (let entry of entries) {
      entry.postDate()
    }

    // … a lot of code
    transactionBundle.getListManager().add(entries)
  }

  // … a lot of code
}

Допустим, нам нужно убрать дубли файла entries, но postEntries() трудно проверить — нет на это времени. Мы можем «прорастить» код где-то ещё, например, в новом методе uniqueEntries(). Этот новый метод легко протестировать, потому что он изолирован. Затем вставим вызов этого метода в существующий, не проверенный код. 

class TransactionGate {
  // … a lot of code

  uniqueEntries(entries) {
    // Some clever logic to dedupe entries, fully tested!
  }

  postEntries(entries) {
    const uniqueEntries = this.uniqueEntries(entries)

    for (let entry of uniqueEntries) {
      entry.postDate()
    }

    // … a lot of code
    transactionBundle.getListManager().add(uniqueEntries)
  }

  // … a lot of code
}

Минимальные изменения, минимальный риск. Можете «вырастить» один метод, целый класс или что-то ещё, что изолирует новый код.

Wrap

Можно «обернуть» изменение, если оно должно произойти до или после существующего кода.

  • Переименуем старый метод, который хотим обернуть.

  • Создадим новый с тем же именем и подписью, что и старый.

  • Вызовем старый метод из нового.

  • Поместим новую логику до/после вызова другого метода.

Эту новую логику можно проверить, потому что старый метод — это «шов», который можно изменить в тестах. Помните предыдущий код?

class TransactionGate {
  // … a lot of code

  postEntries(entries) {
    for (let entry of entries) {
      entry.postDate()
    }

    // … a lot of code

    transactionBundle.getListManager().add(entries)
  }

  // … a lot of code
}

Ещё один способ решить эту проблему — это обернуть её, поэтому мы переходим к postEntries(), списку записей, из которых мы удалили дубли.

class TransactionGate {
  // … a lot of code

  postEntries(entries) {
    // Some clever logic to retrieve unique entries
    this.postEntriesThatAreUnique(uniqueEntries)
  }

  postEntriesThatAreUnique(entries) {
    for (let entry of entries) {
      entry.postDate()
    }

    // … a lot of code

    transactionBundle.getListManager().add(entries)
  }

  // … a lot of code
}

В тестах мы бы изменили проблему postEntriesThatAreUnique(), чтобы проверить, работает ли логика удаления дубликатов. Разница может быть ещё больше.

class TransactionGate {
  // … a lot of code

+  postEntries(entries) {
+    // Some clever logic to retrieve unique entries
+    this.postEntriesThatAreUnique(uniqueEntries)
+  }

+  postEntriesThatAreUnique(entries) {
-  postEntries(entries) {
    for (let entry of entries) {
      entry.postDate()
    }

    // … a lot of code

    transactionBundle.getListManager().add(entries)
  }

  // … a lot of code
}

Эти методы не идеальны, и у них есть недостатки. Но это полезные инструменты при работе с легаси. А при необходимости можно даже немного нарушить правила.

Скретч-рефакторинг

Сложно работать с кодом, который мы не писали, без тестов и с плохой документацией. Чтобы «выбраться» нам нужно разбить зависимости, написать тесты. Но  с чего вообще начинать, когда код непонятен? Хорошая техника — скретч-рефакторинг.

Его цель в том, чтобы ознакомиться с кодом, а не менять. Мы «играем» с кодом столько, сколько захотим: извлекаем функции, упрощаем, переименовываем переменные. Как только сделаем, всё, что нам нужно — откатим всё обратно и начнём с правильных тестов.

Выводы

Легаси будет везде, где бы вы ни работали, в каждой кодовой базе. Можно сопротивляться и чувствовать себя плохо, когда вы застряли в нём. А можно рассматривать это как возможность. Работа со старым кодом это очень ценный навык, его надо изучать теоретически (почитайте книгу «Working Effectively with Legacy Code») и практиковать в ежедневных задачах.


Похожие и интересные статьи:

Больше новостей про разработку в Додо Пицце я пишу в канале Dodo Pizza Mobile. Также подписывайтесь на чат Dodo Engineering, если хотите обсудить эту и другие наши статьи и подходы, а также на канал Dodo Engineering, где мы постим всё, что с нами интересного происходит.

А если хочешь присоединиться к нам в Dodo Engineering, то будем рады  — сейчас у нас открыты вакансии iOS-разработчиков (а ещё для Android, frontend, SRE и других).




Комментарии (23):

  1. flyer2001
    /#22737382 / +1

    Спасибо, на злобу дня тема. А определение «сложно понять, легко сломать» — лучшее по eмкости)))

    • amarao
      /#22737450 / +2

      Сотовый телефон в кармане — это легаси? Сломать легко, понять сложно.

      • merhalak
        /#22738116 / +1

        А то, столько легаси по обратную сторону мобильной связи…

        • amarao
          /#22739292

          Замените телефон на любой shiny new гаджет, который легко сломать, а понять сложно.


          Короче, утверждение "сложно понять" не катит, так же как и "легко сломать".

  2. amarao
    /#22737448 / +5

    "Легаси" и "легаси код" — это разные вещи.


    Например, "легаси" можно считать комплект проблем, которые решают люди. Допустим, у них была проблема с передачей файлов больше 1.44 мегабайта, и они изобрели свою оверлейную файловую систему с поддержкой оффлайн-томов и автоматическим переупорядочиванием.


    … Шли годы, и кто-то поверх написал на баше скрипт, который брал файл с файловой системы, нарезал в эту оверлейную файловую систему и запускал старый код.


    … Шли годы, и пришло время потрогать это. Человек, который откроет код не стой стороны увидит АД с переупорядочиванием блочных устройств (!) при чтении файла.


    Это — легаси. Вне зависимости от того, сколько там тестов и документации.


    Легаси — это решение задач, которых нет, либо решение задач методами, которые больше не признаются за решение. А код, который реализует это решение — легаси код. И проблема не в "легаси коде", а в "легаси задачах", о которых никто даже слышать больше не хочет.


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

  3. fkthat
    /#22737468

    По-моему, переделка легаси и рефакторинг это довольно разные вещи. Буквальное значение слова "legacy" в английском это "унаследованный". Т.е. то что не изначально твое, а досталось тебе от кого-то. Это вовсе не обязательно что-то плохое. Оно может быть очень даже хорошим, просто морально устаревшим. А рефакторинг он именно для "плохого" и не подразумевает каких-то переделок архитектуры или обновления а то и замены платформы.

    • akaDuality
      /#22737478

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


      В моем мире даже удаление базы данных — это рефакторинг

      • fkthat
        /#22737624

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

      • Lart5
        /#22739878

        ИМХО, рефакторинг это когда горел дедлайн и в код натыкали костылей, что бы все работало, а потом появилось время эти костыли выпилить и сделать все нормально. А удаление базы данных — это уже улучшение программы, пусть конечный пользователь этого и не видит, но внутренняя логика работы программы изменяется.

  4. iiwabor
    /#22737590

    ИМХО, легаси — это например модули на COBOL в банке, со всеми вытекающими и COBOLa последствиями. А то что можно решить рефакторингом или добавлением тестов, это не совсем легаси, это просто не очень качественный код.

  5. roboter
    /#22737696

    По коду всётаки всегда можно понять что он делает, главная проблема в объёме этого кода, и тут уже не важно покрыт он тестами или нет.

    • amarao
      /#22739314

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


      С практической точки зрения, вот вам чуть-чуть питона, enjoy.


      def business_dispatcher(*args, **kwargs):
         for a in args:
            for k,v in kwargs:
                yield from a(k)(v)

      • roboter
        /#22741676

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

  6. eldog
    /#22738264 / +1

    Был у меня давным давно, в далёкой галактике, проект.

    Десктопное приложение, точнее, чудовищный рой из запускающих друг друга процессов, рисующих в окно главной программы. Написано на смеси .net, C/C++ и древнего языка TFM с использованием между делом COM. Всё это работало с локальной базой данных древнего производителя, обращалось по сети в кучу мест, ходило в безумное количество файловых шар в сети и запускало-нуждалось в куче внешних компонентов.

    Задача: заменить локальную базу данных на центральный сервис.
    Решение (принимал менеджмент, не я). Я пришёл выполнять.

    Поскольку мы в принципе не можем проанализировать, понять и переработать весь этот код и не должны его сломать, а запросы в бд в коде везде, лезут из всех компонентов, то был написан REST-сервис, принимающий на вход sql-запросы древней базы данных и транслирующий этот диалект в T-SQL. На стороне же клиентов была написана на c++ shim-dll, имитирующая интерфейс бинарной 3rd party библиотеки для доступа к старинной базе. Осложнялось всё это тем, что древний производитель базы в своё время принципиально не документировал эту библиотеку. Вместо этого предложил некий скриптованный вариант C, в котором условные вызовы запросов проходили через специальный компилятор, который наружу выдавал C, где эти запросы были заменены на серии вызовов методов этой бинарной dll, не всегда тривиальных. Например, работа с курсорами, это куча вызовов разных методов и понятно, что библиотека хранит в памяти состояние. Догадаться, что там делается, можно, мы догадались, но официального подтверждения и гарантий работы нет и в принципе не будет, не ваше собачье дело это понимать, пишите скрипты.

    Потом как-то девочка в проект пришла, IT-оптимизатор. Со взором горящим. Говорит, давайте покроем это всё тестами. Давай, милая — говорим. Только вот цена такого покрытия это цена полной переработки архитектуры. Чем мы бы и рады заняться, но начальство, понимая, что это человеко-годы работы сениоров, запрещает. Расстроилась и через месяц ушла.

    • akaDuality
      /#22738470

      Я что-то вывода не понял. Т.е. вы меняете систему опираясь на чутье? Что мешает фиксировать внешнее видимое поведение системы? Как вы копите знание про систему и ее поведение?


      Тесты не пишутся за неделю, это часть процесса.

      • eldog
        /#22738504

        Ну тестеры-то есть, с некоторым ограниченным бизнес-знанием. Сидят, кнопки давят.

        Про тесты я как бы немного в курсе :-)

  7. freakru
    /#22739200

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

    • dipsy
      /#22741232

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

      Всё очень грустно.

  8. vvbob
    /#22741644

    Хуже всего когда тесты пишутся для галочки, только для того, что-бы увеличить покрытие. Открываешь такой проект, смотришь тесты — радуешься, их просто огромное количество, все классно, разобраться будет просто!
    Начинаешь разбираться, а оказывается что тесты там по факту проверяют сколько и в каком порядке модули дергают вызовы из других модулей. Понимания эти тесты не добавляют, надежности так-же, зато работы добавляют — любое изменение внутренней логики работы приходится отражать в этих тестах, при том что именно «что делает код» никак не проверяется, проверяется «как код это делает», что в общем-то и не особо ценно.

    • akaDuality
      /#22741672

      Согласен, «как» код это делает я и в коде вижу, важнее понять зачем и для какой случая. Я встречал термин «тавтология» теста, по нему можно нагуглить

  9. byman
    /#22744896

    Как аппаратчик, я тоже встречаю очень часто слово legacy в англоязычной документации. Для себя я перевожу его как что-то «устаревшее, архаичное». Ну, типа, на всякий случай, шнурок для слива в вашем умном туалете. Недавно я написал полезную для компании программу и, почитав сейчас комментарии к статье, понял, что написал сразу легаси код. Никто из профи-программеров в компании так и не может понять написанного, про рефакторинг и разговора нет. Хотя тесты были еще до рождения кода. Наверное, это потому, что написан код на самом простом Си. Архаизм, другими словами :)