суббота, 16 мая 2009 г.

100% Coverage Driven Design (CDD) - борьба с тухлым кодом

Всем известно, что для качественного ПО покрытие должно быть более 70%. Я не знаю, кто это придумал и причем тут качество, но мне не понятно какая польза от этой цифры. Ещё смешнее слышать, как менеджеры привязывают покрытие к качеству. Если рассуждать в плоскости менеджеров, что и на мелководье и рак рыба, то этого качества конечно же для успокоения самолюбия менеджера достаточно. Но истинные войны света должны понимать, что на этом качество не останавливается... Качество кода - это целая культура разработки, в которую вовлечены клиенты, разработчики, тестировщики и много других людей. Но об этом я уже писал неоднократно, а сегодня остановлюсь на новом подходе. Подходе дающим ещё один мощный инструмент разработчику (ни менеджеру, ни тестировщику) для написания качественного кода.

Я же хочу раскрыть другую замечательную вещь, которая повысит качество кода во много раз. Один нюанс - вам придётся делать 100% покрытие :)

Эту идея пришла во время парного программирования с Кириллом Медведевым в рамках одной из стади-групп.

Эту разработку я называл "100% Coverage Driven Design" (или сокращённо CDD). Суть этого подхода есть сложение TDD, общего подхода написания тестов (ключевая фраза здесь - TestLast) и инструмента покрытия кодом.

Напомню цикл TDD:
1. Пишем тест (формализуем ожидания от будущего кода)
2. Реализуем код
3. Рефакторим

В "100% Coverage Driven Design" цикл такой:
1. Тест
2. Код
3. Рефакторим
4. Добиваемся 100% покрытия

Какие бонусы даёт такой подход
1. Все бонусы TDD
2. Вы всегда контролируете, что ваш код используется и нету лишнего, мертвого, того кода, который будет тухнуть из-за дня в день. Покрытие будет следить, как только что-то лишнее появляется и не используется - сразу выкидываем.

Возникает вопрос. А что делать с тестами, которые устаревают. Ведь их с помощью покрытия нельзя выявить, а тем более сказать что код, который покрывается устаревшими тестами уже начинает потухать. А решение простое - смирение. Если тест устарел, то первым делом он начинает из зеленого превращаться в красное. И вы тут же решаете, что с ним делать. В красное он может превратиться по множеству причин. Но если одна из причин - устаревший и ненужный тест. Мы его удаляем. Запускаем Code Сoverage и проверяем, где произошли смещения от точки 100% покрытия.

Покажу на маленьком примере. С помощью TDD цикла создали библиотечку. Проверили её покрытие и выяснилось не 100%. Начинаем разбираться и видим, что появился тухлый код. Сначала комментирую тухляк, а потом лопатой все убираю.


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



Мне приходиться напрячь мозги и понять, что components объект никогда и не используется! Такое открытие позволяет мне удалить столь запутанную тухлятину тоже.



Результат не заставляет ждать. Идеально чистый код! Я с уверенностью продолжаю дальнейшую разработку.

Теперь немного GUI-шного, про разработку через TestLast и как техника рефакторинга приводит ухудшению Code Coverage.

1. Конечно же, когда добавляется новый функционал, то используем TestFirst:

public void MainForm()
{
     SetUpClass.PrepareTestFile();

     var form = new MainForm();
     var mocks = new MockRepository();

     var dialog = (OpenFileDialog)mocks.CreateMock(typeof(OpenFileDialog));
     Expect.Call(dialog.FileName).Return("filename.ew");
     Expect.Call(dialog.ShowDialog()).Return(DialogResult.OK);
     mocks.ReplayAll();

     form.openFileDialog = dialog;
     form.MenuItem_Open(null, null);

     mocks.VerifyAll();
     Assert.AreEqual(2, form.dicGridControl.Count);
}

2. Реализуем функцию

public void MenuItem_Open(object sender, EventArgs e)
{
     if (openFileDialog.ShowDialog() == DialogResult.OK)
     {
         Dic dic = new Dic();
         dic.Load(openFileDialog.FileName);
         dicGridControl.Bind(dic.ToList());
         }
}

Покрытие: 100%

3. Проводим рефакторинг Replace Nested Conditional with Guard Clauses (кстати, мой самый любимый)

public void MenuItem_Open(object sender, EventArgs e)
{
     if (openFileDialog.ShowDialog() != DialogResult.OK)
         return;


     Dic dic = new Dic();
     dic.Load(openFileDialog.FileName);
     dicGridControl.Bind(dic.ToList());
}

Покрытие: 97%.

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

В общем снизили покрытие. Что делать. Я останусь верным принципам рефакторинга и оставлю проверку входного условия. Но я должен дотянуть покрытие до 100%. Тут на помощь приходит старая практика Test Last. Если раньше, эта техника была вызвана сообращениями разработки тестов для уже разработанного кода, сейчас эта техника изменит своё предназначение. Используя эту технику я подтяну покрытие до 100%.

После мелких рефакторингов я добавил второй тест, используя технику TestLast:

[Test]
public void MainFormCancel()
{
     Expect.Call(dialog.ShowDialog()).Return(DialogResult.Cancel);
     mocks.ReplayAll();

     form.MenuItem_Open(null, null);

     mocks.VerifyAll();
     Assert.AreEqual(0, form.dicGridControl.Count);
}

Результат: покрытие 100%

В данном случае проведение рефакторинга не очень оправдано - так как и с ним и без него код выглядит очень простым и его можно сразу понять. Я преследовал цель показать , как рефакторинг может привести к снижению покрытия кода, даже если он прошёл без изменения функциональности.

Другой нюанс, если я пойду дальше и подстрахуюсь от проблем с файловой системой:

public void MenuItem_Open(object sender, EventArgs e)
{
     if (openFileDialog.ShowDialog() != DialogResult.OK)
          return;

     try
     {
          Dic dic = new Dic();
          dic.Load(openFileDialog.FileName);
          dicGridControl.Bind(dic.ToList());
     }
     catch (Exception ex)
     {
          MessageBox.Show("Error: Could not read file from disk. Original error: " + ex.Message);
     }

}

Этот кусочек кода пишется даже без теста легко. Я нарушил правило TDD и добавил строчку кода без теста. И кара настигла меня моментально - покрытие упало, плюс протестировать статический метод MessageBox.Show - это целая проблема (ни моки, ни что либо другое здесь не поможет - в морг). А представьте сколько проблем мы делаем, когда разрабатываем более менее серьезное приложение?

Что хотелось бы иметь в инструментах покрытия:
1. Механизм разметки - какие функции все же не стоит покрывать. Например, автогенерацию или какие-то ничего незначащие хитрые кейсы.
2. Историю покрытия, чтобы наблюдать диффы. Это понадобилось бы для работы над легаси-системами. В таких системах, если мы начинаем разрабатывать по TDD большая часть кода будем мешать нашему анализу. А анализ диффов позволил бы использовать этот подход даже там.

Вывод: Данную методику изобрёл буквально вчера. За вчера её опробовал, очумел от эффективности и продолжаю праведно работать по ней. Методика 100% покрытия стала для меня точно так же как запускать юнит тесты. Стала родной и без неё уже свет не мил. Теперь не то чтобы у меня тесты есть согласно TDD, но и код очищается от мертвого кода очень удобно и легко. Теперь я не только уверен в функционировании (этому мне помогает TDD), но и что код чистый (этому помогает 100% покрытие). В общем я доволен как маленький ребёнок от такой простои и в тоже время мощной игрушки. Сейчас нарабатываю опыт от применения, о чем буду делиться и буду ждать последователей, кто готов проверить предложенный метод.

4 комментария:

  1. Анонимный16 мая 2009 г., 19:26

    В реальном проекте без впадания в маразм невозможно, да и не нужно добиваться 100% покрытия кода => ваш метод работать не будет.

    И еще вопрос: при наличии "Механизм разметки - какие функции все же не стоит покрывать" смысл от code coverage теряется совсем. Разве нет?

    -- meowth

    ОтветитьУдалить
  2. ещё ряд идей, которые родились после обсуждения с Сергеем Назаренко

    1. 100% покрытие это лишь капля в море
    100% покрытие кода это не 100% покрытие функционала. покрыть все варианты использования приложения это миллиарды кода нужно. поэтому здесь здесь нужна команда тестировщиков. TDD и 100% покрытие решает задачу качественной реализации кода, а не качества продукта в целом

    2. в чем тогда разница 100% и 70%?
    такая же как между 100% и 99%
    99% - это означает есть возможность оставить тухлый код
    100% - полностью исключает тухлятину
    и это никак не связана с работой, которую делают тестировщики. но девелоперы должны стремиться использовать практики, чтобы их работа не воняла тухлятиной.

    3. вывод: статья про качество именно кода, а не приложения. вот именно этот акцент я и не заакцентировал

    4. не совсем понял логическую цепочку что 70% покрытие кода всего лишь успокаивает самолюбие манагера. можешь чуть пояснить?
    70% покрытие совершенно ничего не означает, 30% может содержать ошибки которые нужно часами отлавливать это раз.

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

    ОтветитьУдалить
  3. Анонимный17 мая 2009 г., 1:42

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

    Как-то вы меня сразу в теоретики записали, да еще и обзываете мои комменты болтовней. Я постараюсь не обижаться на ваше хамство, тем более что мои убеждения -- таки правильные, с ваших слов :)

    Теперь к теме.
    Посмотрите на эти Ваши слова: "но я должен дотянуть покрытие". Такая фраза --прямо как code smell, если вам так понятнее. Вы придумали себе проблему, а теперь успешно ее решаете ) Хорошо, что решили, но есть путь проще -- не придумывать саму проблему.

    -- meowth

    ОтветитьУдалить
  4. Привет.

    Я только начинаю вливаться в тему TDD и особой практики ещё не имею, но основываясь на тех знаниях, что уже приобрел - вижу в этом подходе (CDD) только один смысл - самоконтроль.

    Смотри. Если следовать основным парадигмам TDD, то в твоем коде (который ещё до рефакторинга) не могла появиться строка:
    if (openFileDialog.ShowDialog() == DialogResult.OK)
    Ведь если её удалить, то тест пройдет, а значит она не нужна. Т.е. эта строка была написана без теста - а это нехорошо =)

    Получается, что если ты сразу пишешь явную реализацию, как в этом случае, то ты рискуешь, а чтобы снизить риск - потом перепроверяешь, не уменьшилось ли покрытие.

    В общем, подход хороший и я надеюсь когда-нибудь его попробовать. Спасибо.

    Дмитрий Усынин.

    ОтветитьУдалить