Есть большая функция на Си, несколько сотен строк, которая делает кучу вещей с разными типами объектов. Нужно распилить ее на несколько функций-кусков, каждый из который будет обрабатывать свой тип объекта. Функция интенсивно работает с базой, но не напрямую, а через вызовы других функций, которые отвечают за бизнес логику.
Есть интеграционные тесты, которые позволяют выявить грубые ошибки. Если основное поведение сломано, то какой-то из тестов зафэйлится. Но покрытие далеко не 100 процентов, и есть шанс сломать какой-то второстепенный кондишен, который тестами не ловится.
Что я сделал.
1) Создал отдельный класс с конструктором повторяющим аргументы функции и методом Run, который содержит тело функции.
2) Все вызовы функций бизнес логики я завернул через интерфейс. Теперь функцию можно тестировать с помощью юнит тестов.
3) Начал писать юнит тесты, использую новый интерфейс и google mock.
4) По мере написание тестов из функции извлекаются фрагменты и выносятся в отдельные протектед методы класса.
В чем проблема.
1) Не смотря на то, что я стараюсь все делать максимально аккуратно, я таки умудрился сломать эту функцию, и меня спасли интеграционные тесты.
2) Функция имеет большую степень вложенности и для того, чтобы проверить какое-то условие внутри, приходится мокать вызовы нескольких функций бизнес логики. Это сильно замедляет написание тестов и увеличивает объем их кода.
В принципе, у меня пара тысяч строк такого кода, хотелось бы разобраться, с какой стороны подходить.
Есть у кого мысли по поводу?
Полистайте книгу Мартина Фаулера — Рефакторинг (если еще этого не сделали).
В книге формализованы некоторые методы рефакторинга, которые по идее можно более-менее безобидно применять.
Вам необходим, как я понял, рефакторинг "Extract method".
Для начала хорошо по максимуму избавиться от локальных временных переменных:
"Replace temp with Query", "Split temporary varible", "Replace method with method object".
Это помогает упростить код для дальнейшего выполнения рефакторинга "Extract method".
Несколько сотен строк это еще терпимо, могло бы быть и хуже
Я бы сделал как-то так:
1) Дописал бы ещё тестов если там совсем мешанина и если боитесь.
2) Переименовал бы все переменные и функции вызываемые из этой функции так, чтобы было все понятно.
3) Дописал бы комментариев чисто для себя если где-то сложно понять что происходит
4) Ну а дальше основной инструмент — extract method. Т.е. искал бы кусочки функциональности которые можно вынести в отдельную функцию и выносил бы их, не забывая их понятно называть.
5) Делаем extract method пока основная функция не станет размером не больше пары экранов.
6) Теперь доделать финальный рефакторинг станет уже намного проще, когда функция разобрана на понятные кусочки.
7) Разносим выделенные функции по соответствующим классам.
SON>2) Все вызовы функций бизнес логики я завернул через интерфейс. Теперь функцию можно тестировать с помощью юнит тестов.
Как ты назвал этот интерфейс?
SON>4) По мере написание тестов из функции извлекаются фрагменты и выносятся в отдельные протектед методы класса.
Эти протектед методы нужно переносить в другие классы, группируя их так, чтобы результирующие классы обладали высокой связностью. Хотя может оказаться, что первоначальное деление на методы было сделано неверно. Тогда надо инлайнить назад и выделять снова, принимая во внимание уже выделенные абстракции. В целом твоя задача — построить дизайн некоторой подсистемы, имея описание требований к ней в виде исходных интеграционных тестов этой громадной функции.
В принципе, я так все и делаю, практически дословно. Просто не стал дописывать про дальнейший дизайн системы.
MC>Несколько сотен строк это еще терпимо, могло бы быть и хуже
Есть и порядком хуже, но надо с чего-то же начинать
MC>Я бы сделал как-то так: MC>1) Дописал бы ещё тестов если там совсем мешанина и если боитесь.
Вот два интересных момена. Какой набор юнит тестов считать достаточным?
И еще... Я работаю в итеративном стиле:
1) пишу тест для фиксирования существующей функциональности
2) проверяю, что тест работает.
3) выделяю фрагмент покрытый тестом.
4) проверяю, что все ок.
4а) правлю, если не ок.
5) переход к 1) для следующего фрагмента.
Проблема с этим подходом в том, что написание тестов занимает 80 процентов времени, и они в итоге получаются не очень полезными после окончания рефакторинга, так как тестирование идет на мок-объектах. Это немного напрягает.
Здравствуйте, Ryadovoy, Вы писали:
R>Полистайте книгу Мартина Фаулера — Рефакторинг (если еще этого не сделали). R>В книге формализованы некоторые методы рефакторинга, которые по идее можно более-менее безобидно применять. R>Вам необходим, как я понял, рефакторинг "Extract method". R>Для начала хорошо по максимуму избавиться от локальных временных переменных: R>"Replace temp with Query", "Split temporary varible", "Replace method with method object". R>Это помогает упростить код для дальнейшего выполнения рефакторинга "Extract method".
"Replace method with method object" — это основа моего рефакторинга в данном случае. Использую ровно такой набор инструментов, что вы сказали, названия, правда, не для всех знаю
Если с инструментами более-менее понятно, то еще остается вопрос про тесты. Какая тактика тестирования? Чтобы не повторяться... Я в соседней ветке MozgC написал, что я использую.
Здравствуйте, -VaS-, Вы писали:
SON>>2) Все вызовы функций бизнес логики я завернул через интерфейс. Теперь функцию можно тестировать с помощью юнит тестов.
VS>Как ты назвал этот интерфейс?
На самом деле это интерфейс для реафакторинга группы функций, в нем около сотни методов. Название db::operations::IDbFunctions, например.
SON>>4) По мере написание тестов из функции извлекаются фрагменты и выносятся в отдельные протектед методы класса.
VS>Эти протектед методы нужно переносить в другие классы, группируя их так, чтобы результирующие классы обладали высокой связностью. Хотя может оказаться, что первоначальное деление на методы было сделано неверно. Тогда надо инлайнить назад и выделять снова, принимая во внимание уже выделенные абстракции. В целом твоя задача — построить дизайн некоторой подсистемы, имея описание требований к ней в виде исходных интеграционных тестов этой громадной функции.
Интеграционные тесты тоже я писал, как первый этап рефакторинга. Ну не важно. Вообще, good point! Подумаю над этим.
SON>2) Все вызовы функций бизнес логики я завернул через интерфейс. Теперь функцию можно тестировать с помощью юнит тестов. SON>3) Начал писать юнит тесты, использую новый интерфейс и google mock.
... SON>2) Функция имеет большую степень вложенности и для того, чтобы проверить какое-то условие внутри, приходится мокать вызовы нескольких функций бизнес логики. Это сильно замедляет написание тестов и увеличивает объем их кода.
Может подумать над тем, чтобы создать класс-заглушку на основании интерфейса Вашей бизнес-логики с каким-то базовым поведением
вместо того, чтобы в каждом тесте настраивать поведение интерфейса заново при помощи gmock объекта?
Со своего опыта: у меня был класс, описывающий диск файловой системы.
По мере написания тестов я выделил базовый объект со стандартным поведением и дальше лишь подстраивал поведение по мере надобности.
Ну а вообще IMHO: юнит-тесты не панацея (к сожалению) и покрыть 100% кода не всегда хорошая идея,
особенно если эти тесты потом лягут мертвым грузом и их придется сопровождать вместе с остальным кодом.
Здравствуйте, Son of Northern Darkness, Вы писали:
SON>В принципе, у меня пара тысяч строк такого кода, хотелось бы разобраться, с какой стороны подходить. SON>Есть у кого мысли по поводу?
Я в таких случаях делаю примерно так:
Первый шаг — сделать код максимально понятным.
Сначала нужно уменьшить уровень вложенности. Потом сгруппировать код относительно объектов с которыми идет работа.
Здесь хороший результат даёт тупое дублирование кода. Кое где возможно придется инлайнить даже внешние функции.
Дальше нужно переименовать все переменные, параметры, что бы отражали основные обяханности.
Второй шаг — изолировать код от внешнего мусора. Т.е. нужно обрезать все возможные депенденсы. Эт лучше делать через параметры . Эта часть работы наиболее важная, т.к. потом придется приседать именно вокгруг депенденсов. Именно эта часть определяет, как потом можно писать моки. Кое какие тесты можно писать уже после обрезания депенденсов.
И только после этих шагов можно думать про разбиение метода на части . И здесь стоит начать с малого и двигаться мелкими шажочками.
После первых двух шагов в коде обязательно будут участки сильно похожие или даже одинаковые. Их нужно аккуратно вырезать во отдельные методы. Лучше именно в методы. Дальше метод преобразовывается в класс, интерфейс унифицируется. Естесвенно руководствоваться нужно обязанностями, а не только степенью сходтсва .
Четвертый шаг — основные тесты и моки.
Пятый шаг — устранение дублирования.
Естественно, шаги сильно условные. Т.к. границы не всегда четкие и иногда проще выделить отдельную сущность сразу и только потом продолжать все остальное.
Здравствуйте, Ryadovoy, Вы писали:
SON>>2) Все вызовы функций бизнес логики я завернул через интерфейс. Теперь функцию можно тестировать с помощью юнит тестов. SON>>3) Начал писать юнит тесты, использую новый интерфейс и google mock. R>... SON>>2) Функция имеет большую степень вложенности и для того, чтобы проверить какое-то условие внутри, приходится мокать вызовы нескольких функций бизнес логики. Это сильно замедляет написание тестов и увеличивает объем их кода.
R>Может подумать над тем, чтобы создать класс-заглушку на основании интерфейса Вашей бизнес-логики с каким-то базовым поведением R>вместо того, чтобы в каждом тесте настраивать поведение интерфейса заново при помощи gmock объекта?
R>Со своего опыта: у меня был класс, описывающий диск файловой системы. R>По мере написания тестов я выделил базовый объект со стандартным поведением и дальше лишь подстраивал поведение по мере надобности.
Ну то есть, это по сути реализация mock object врукопашную? Хотелось бы этого избежать, написание тестов и так выходит очень затратным по времени. Внимательно почитал, что народ ответил. Видимо, я делаю примерно то же самое, руководствуюсь common sense... Серебряной пули нет Еще очень разочаровал Visual Assist X, даже method extraction не работает как надо. В Java и C#, с этим проще, конечно.
SON>Ну то есть, это по сути реализация mock object врукопашную? Хотелось бы этого избежать, написание тестов и так выходит очень затратным по времени.
Я советовал создавать не Mock объект, а по возможности создавать Stub вместо Mock. Это должно ускорить процесс, т.к. stub объект создается один раз и затем повторно используется.
Mock-же приходится настраивать для каждого теста по отдельности (нужно это или нет).
Если у Вас есть интерфейс на виртуальных функциях, то вы можете унаследовать от него Stub, а затем создавать Mock с перегрузкой лишь необходимых методов, унаследовав его от Stub.
class IMyObject
{
virtual void f1() = 0;
virtual void f2() = 0;
...
virtual void f100() = 0;
};
class CMyObjectStub : public IMyObject
{
virtual void f1(){}
virtual void f2(){}
...
virtual void f100(){}
};
class CMyObjectMockForTest2 : public CMyObjectStub
{
MOCK_METHOD0(f2, void ()); //Переопределяем только то, что реально нужно для проверки, метод f1() реализуется Stub обхектом.
};
Еще я бы советовал всегда помнить что именно тестируется в конкретном тесте (для этого называйте тесты понятными именами),
не тестируйте сторонний код, делайте проверки только тестируемого кода, то есть тестируйте только то, что написано в имени теста.
Еще очень разочаровал Visual Assist X, даже method extraction не работает как надо. В Java и C#, с этим проще, конечно.
