Однажды в мое поле зрения попал проект заказной разработки, который команда безуспешно пыталась сдать в течение двух лет. Полностью сдано было около 5% проекта, однако дальнейшая сдача заканчивалась неудачами то на стадии приемки функционала заказчиком, то на стадии внутреннего тестирования, то разработчики срывали сроки при внесении изменений в код.
За время проекта два раза поменяли команду и заказчика.
Аудит процесса производства с точки зрения time2market (сроков от появления идеи до запуска в промышленное использование) показал, что задержки возникают на трех стадиях:
-
долго вносятся изменения в код
-
долго выполняется ручное тестирование
-
часто после тестирования код возвращается на доработку
Первопричиной низкого time2market являлось то, что команда не владела практикой coding kata, о которой я подробно расскажу в данной статье на примере задачи «Позолоченная роза«.
Задача является довольно известной в узких кругах. Правильного решения у нее нет. Ниже я предлагаю свое решение в формате пошагового тьюториала. Если этот тьюториал окажется интересным, то рекомендую дать его команде разработчиков для самостоятельного изучения и проработки.
После применения приемов, описанных в этой статье, оставшиеся 95% проекта описанная выше команда реализовала и сдала за 6 месяцев вместо прогнозируемых 48 лет, что эквивалентно сокращению time2market в 96 раз.
Позолоченная роза: условия задачи для coding kata
Привет и добро пожаловать в команду Gilded Rose. Как вы знаете, мы небольшая гостиница в отличном месте в известном городе, которой управляет дружелюбный трактирщик по имени Эллисон. Мы также покупаем и продаем только лучшие товары. К сожалению, качество наших товаров постоянно ухудшается по мере приближения срока реализации. У нас есть система, которая обновляет наш инвентарь для нас. Он был разработан серьезным типом по имени Лирой, который перешел к новым приключениям. Ваша задача — добавить новую функцию в нашу систему, чтобы мы могли начать продавать новую категорию товаров. Сначала введение в нашу систему:
-
Все предметы имеют значение «продать в течение» (sell_in), которое обозначает количество дней, в течение которых мы должны продать предмет.
-
Все предметы имеют значение качества (quality), которое указывает, насколько ценен предмет.
-
В конце каждого дня наша система снижает оба значения для каждого элемента.
Довольно просто, правда? Ну вот тут становится интересно:
-
Как только срок продажи истек, качество ухудшается в два раза быстрее.
-
Качество предмета никогда не бывает отрицательным
-
«Выдержанный бри» (Aged Brie) на самом деле тем лучше, чем старше он становится.
-
Качество предмета никогда не превышает 50.
-
«Sulfuras», будучи легендарным предметом, никогда не продается и не теряет качества.
-
«Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), также как выдержанный бри (Aged Brie), повышаются в качестве по мере приближения значения sell_in: качество повышается на 2, если осталось 10 дней или меньше, и на 3, если осталось 5 дней или меньше, но после концерта качество падает до 0.
Недавно мы заключили договор с поставщиком колдовских предметов. Для этого требуется обновление нашей системы:
-
Качество «Сотворенных» (Conjured) предметов ухудшается в два раза быстрее, чем у обычных предметов.
Не стесняйтесь вносить любые изменения в метод update_quality и добавлять любой новый код, пока все работает правильно. Однако не изменяйте класс Item или свойство items, так как они принадлежат гоблину в углу, который нападет на вас и убьет вас одним выстрелом, поскольку он не верит в совместное владение кодом (вы можете сделать метод update_quality и свойство items статическими, если хотите, мы вас прикроем).
Исходный код: https://github.com/emilybache/GildedRose-Refactoring-Kata
#gilded_rose.py class GildedRose(object): def __init__(self, items): self.items = items def update_quality(self): for item in self.items: if item.name != "Aged Brie" and item.name != "Backstage passes to a TAFKAL80ETC concert": if item.quality > 0: if item.name != "Sulfuras, Hand of Ragnaros": item.quality = item.quality - 1 else: if item.quality < 50: item.quality = item.quality + 1 if item.name == "Backstage passes to a TAFKAL80ETC concert": if item.sell_in < 11: if item.quality < 50: item.quality = item.quality + 1 if item.sell_in < 6: if item.quality < 50: item.quality = item.quality + 1 if item.name != "Sulfuras, Hand of Ragnaros": item.sell_in = item.sell_in - 1 if item.sell_in < 0: if item.name != "Aged Brie": if item.name != "Backstage passes to a TAFKAL80ETC concert": if item.quality > 0: if item.name != "Sulfuras, Hand of Ragnaros": item.quality = item.quality - 1 else: item.quality = item.quality - item.quality else: if item.quality < 50: item.quality = item.quality + 1 class Item: def __init__(self, name, sell_in, quality): self.name = name self.sell_in = sell_in self.quality = quality def __repr__(self): return "%s, %s, %s" % (self.name, self.sell_in, self.quality)#texttest_fixture.py from __future__ import print_function from gilded_rose import * if __name__ == "__main__": print ("OMGHAI!") items = [ Item(name="+5 Dexterity Vest", sell_in=10, quality=20), Item(name="Aged Brie", sell_in=2, quality=0), Item(name="Elixir of the Mongoose", sell_in=5, quality=7), Item(name="Sulfuras, Hand of Ragnaros", sell_in=0, quality=80), Item(name="Sulfuras, Hand of Ragnaros", sell_in=-1, quality=80), Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=15, quality=20), Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=49), Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=49), Item(name="Conjured Mana Cake", sell_in=3, quality=6), # <-- :O ] days = 2 import sys if len(sys.argv) > 1: days = int(sys.argv[1]) + 1 for day in range(days): print("-------- day %s --------" % day) print("name, sellIn, quality") for item in items: print(item) print("") GildedRose(items).update_quality()Проведите аудит процесса производства с точки зрения time2market
У Вас не будет доступа к команде, поэтому ограничьтесь только ревью артефактов:
-
требования
-
исходный код
-
тесты
Аудит процесса производства 1/4. Требования с точки зрения time2market
Изучите требования и запишите свое мнение о слабых и сильных сторонах требований с точки зрения time2market.
Для сравнения приведу свое субъективное мнение: существующие требования показались мне простыми и понятными, изучение требований заняло менее минуты. Возник вопрос как помечаются сотворенные предметы и я выдвинул гипотезу, что сотворенные предметы начинаются с последовательности «Conjured » перед названием предмета. Проверить гипотезу я не смог, поэтому принял ее на веру.
Аудит процесса производства 2/4. Проектирование и разработка с точки зрения time2market
Изучите код в файле gilded_rose.py и запишите свое мнение о слабых и сильных сторонах кода с точки зрения time2market.
#gilded_rose.py class GildedRose(object): ... def update_quality(self): for item in self.items: if item.name != "Aged Brie" and item.name != "Backstage passes to a TAFKAL80ETC concert": if item.quality > 0: if item.name != "Sulfuras, Hand of Ragnaros": item.quality = item.quality - 1 else: if item.quality < 50: item.quality = item.quality + 1 if item.name == "Backstage passes to a TAFKAL80ETC concert": if item.sell_in < 11: if item.quality < 50: item.quality = item.quality + 1 if item.sell_in < 6: if item.quality < 50: item.quality = item.quality + 1 if item.name != "Sulfuras, Hand of Ragnaros": item.sell_in = item.sell_in - 1 if item.sell_in < 0: if item.name != "Aged Brie": if item.name != "Backstage passes to a TAFKAL80ETC concert": if item.quality > 0: if item.name != "Sulfuras, Hand of Ragnaros": item.quality = item.quality - 1 else: item.quality = item.quality - item.quality else: if item.quality < 50: item.quality = item.quality + 1 ...Для сравнения приведу свое субъективное мнение: в методе update_quality я обнаружил менее 28 строк кода. Код показался мне запутанным. Любопытно, что слова «путать» и «путь» в русском языке похожи (хотя имеют разную этимологию).
Для определения количества путей в коде посчитайте цикломатическую сложность кода, используя пакет radon (https://radon.readthedocs.io/en/latest/commandline.html) и запишите свое мнение о сложности кода.
$ pip install radon $ radon cc -s gilded_rose.py gilded_rose.py M 8:4 GildedRose.update_quality - C (19) C 3:0 GildedRose - C (11) C 39:0 Item - A (2) M 5:4 GildedRose.__init__ - A (1) M 40:4 Item.__init__ - A (1) M 45:4 Item.__repr__ - A (1)Для сравнения приведу свое мнение: метод update_quality имеет цикломатическую сложность 19 единиц, то есть существует 19 путей, по которым может исполниться данный код. В мою голову относительно легко помещается 5 путей и этот код для меня сложный.
Важно: метрика цикломатической сложности иногда дает ложноположительные срабатывания на понятном и простом коде. Игнорируйте такие случаи.
Запишите как много времени Вы потратили на изучение кода. У меня получилось около 20 минут.
Аудит процесса производства 3/4. Тестирования с точки зрения time2market
Изучите код в файле texttest_fixture.py и запишите свое мнение о слабых и сильных сторонах кода с точки зрения time2market.
#texttest_fixture.py from __future__ import print_function from gilded_rose import * if __name__ == "__main__": print ("OMGHAI!") items = [ Item(name="+5 Dexterity Vest", sell_in=10, quality=20), Item(name="Aged Brie", sell_in=2, quality=0), Item(name="Elixir of the Mongoose", sell_in=5, quality=7), Item(name="Sulfuras, Hand of Ragnaros", sell_in=0, quality=80), Item(name="Sulfuras, Hand of Ragnaros", sell_in=-1, quality=80), Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=15, quality=20), Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=49), Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=49), Item(name="Conjured Mana Cake", sell_in=3, quality=6), # <-- :O ] days = 2 import sys if len(sys.argv) > 1: days = int(sys.argv[1]) + 1 for day in range(days): print("-------- day %s --------" % day) print("name, sellIn, quality") for item in items: print(item) print("") GildedRose(items).update_quality() Для сравнения приведу свое субъективное мнение: тесты проекта автоматизированы, но не являются автоматическими — то есть требуют участия тестировщика. В файле с тестами зафиксированы входные данные, написана утилита для запуска тест-кейсов, но отсутствуют ожидаемые результаты запуска. Тестировщик запускает утилиту и сравнивает результаты ее работы с известными ему результатами. Эта ручная работа увеличивает time2market.
Запустите тест, изучите выходные данные и попробуйте разобраться корректно ли текущий вариант кода реализовывает требования.
$ python texttest_fixture.py OMGHAI! -------- day 0 -------- name, sellIn, quality +5 Dexterity Vest, 10, 20 Aged Brie, 2, 0 Elixir of the Mongoose, 5, 7 Sulfuras, Hand of Ragnaros, 0, 80 Sulfuras, Hand of Ragnaros, -1, 80 Backstage passes to a TAFKAL80ETC concert, 15, 20 Backstage passes to a TAFKAL80ETC concert, 10, 49 Backstage passes to a TAFKAL80ETC concert, 5, 49 Conjured Mana Cake, 3, 6 -------- day 1 -------- name, sellIn, quality +5 Dexterity Vest, 9, 19 Aged Brie, 1, 1 Elixir of the Mongoose, 4, 6 Sulfuras, Hand of Ragnaros, 0, 80 Sulfuras, Hand of Ragnaros, -1, 80 Backstage passes to a TAFKAL80ETC concert, 14, 21 Backstage passes to a TAFKAL80ETC concert, 9, 50 Backstage passes to a TAFKAL80ETC concert, 4, 50 Conjured Mana Cake, 2, 5Для сравнения мое субъективное мнение: по-умолчанию утилита имитирует 2 дня работы системы для инвентаря из 9 предметов и тестировщику в этом случае нужно проверить 54 параметра. Некоторые тестовые случаи требуют имитации большего количества дней работы системы: например, для проверки того, что качество эликсира мангуста не падает ниже нуля, нужно эмулировать 8 дней работы системы, что приведет к четырехкратному увеличению нагрузки на тестировщика. Был обнаружен недостаток в реализации поведения программы для сотворенного пирожного маны — его качество убывало по одной единице в день, а не по две единицы в день как было указано в требованиях.
Запишите как много времени Вы потратили на запуск тестов. У меня получилось около 10 минут.
Аудит процесса производства 4/4. Рекомендациии по итогам аудита с целью снижения time2market
Запишите рекомендации по итогу изучения.
Для сравнения привожу свои рекомендации:
-
Для сокращения time2market проектирования: провести рефакторинг кода update_quality в соответствии с принципами SOLID
-
Для сокращения time2market разработки: снизить цикломатическую сложность кода до 5.
-
Для сокращения time2market тестирования:
3.1 написать отдельные тест-кейсы на каждое из требований
3.2 исключить необходимость участия тестировщика
3.3 обеспечить покрытие кода тестами не менее чем 90%
Рефакторинг кода 1/4. Автоматизация тестов
При рефакторинге больше всего времени будет потрачено на регрессионное тестирование, поэтому начните с автоматизации тестов. Добейтесь того, чтобы тесты проверяли требования и отпала необходимость ручного труда.
Замените код в test_gilded_rose.py на код из фрагмента ниже.
#test_gilded_rose.py import pytest def test_item_has_sell_in_attribute(): """ Все предметы имеют значение "продать в течение" (sell_in), которое обозначает количество дней, в течение которых мы должны продать предмет. """ item = Item(name="Foo", sell_in=10, quality=1) assert item.sell_in == 10 def test_item_has_quality_attribute(): """ Все предметы имеют значение качества (quality), которое указывает, насколько ценен предмет. """ item = Item(name="Foo", sell_in=10, quality=1) assert item.quality == 1 def test_system_decreases_quality_every_day(): """ В конце каждого дня наша система снижает оба значения для каждого элемента. """ items = [Item(name="+5 Dexterity Vest", sell_in=10, quality=1),] GildedRose(items).update_quality() assert items[0].sell_in == 9 assert items[0].quality == 0Запустите тесты с измерением процента покрытия кода.
$ pip install pytest $ pip install coverage $ coverage run -m pytest test_gilded_rose.py && coverage report ========================================================================== test session starts ========================================================================== platform darwin -- Python 3.10.9, pytest-7.2.1, pluggy-1.0.0 rootdir: /Users/vmihaylov/PycharmProjects/GildedRose-Refactoring-Kata/python collected 3 items test_gilded_rose.py ... [100%] =========================================================================== 3 passed in 0.00s =========================================================================== Name Stmts Miss Cover ----------------------------------------- gilded_rose.py 36 18 50% test_gilded_rose.py 13 0 100% ----------------------------------------- TOTAL 49 18 63%Покрытие кода gilded_rose.py составило 50%.
Добавьте тесты на остальные требования.
#test_gilded_rose.py def test_when_sell_in_expired_quality_decreases_twice_as_fast(): """ Как только срок продажи истек, качество ухудшается в два раза быстрее. """ items = [Item(name="+5 Dexterity Vest", sell_in=0, quality=10),] GildedRose(items).update_quality() assert items[0].sell_in == -1 assert items[0].quality == 8 def test_quality_never_becomes_negative(): """ Качество предмета никогда не бывает отрицательным """ items = [Item(name="+5 Dexterity Vest", sell_in=1, quality=0),] GildedRose(items).update_quality() assert items[0].sell_in == 0 assert items[0].quality == 0 def test_aged_brie_becomes_better_after_sell_in(): """ «Выдержанный бри» (Aged Brie) на самом деле тем лучше, чем старше он становится. """ items = [Item(name="Aged Brie", sell_in=10, quality=1),] GildedRose(items).update_quality() assert items[0].sell_in == 9 assert items[0].quality == 2 def test_item_quality_never_exceeds_50(): """ Качество предмета никогда не превышает 50. """ items = [Item(name="Aged Brie", sell_in=10, quality=50),] GildedRose(items).update_quality() assert items[0].sell_in == 9 assert items[0].quality == 50 def test_sulfuras_never_sells_in_and_never_looses_quality(): """ "Sulfuras", будучи легендарным предметом, никогда не продается и не теряет качества. """ items = [Item(name="Sulfuras, Hand of Ragnaros", sell_in=10, quality=80),] GildedRose(items).update_quality() assert items[0].sell_in == 10 assert items[0].quality == 80 def test_backstage_passes_increase_quality_by_2_if_10_to_6_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert) повышаются в качестве по мере приближения значения sell_in: качество повышается на 2, если осталось 10 дней или меньше. """ items = [Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=6),] GildedRose(items).update_quality() assert items[0].sell_in == 9 assert items[0].quality == 8 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), повышаются в качестве по мере приближения значения sell_in: качество повышается на 3, если осталось 5 дней или меньше. """ items = [Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=7),] GildedRose(items).update_quality() assert items[0].sell_in == 5 assert items[0].quality == 10 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), после концерта качество падает до 0. """ items = [Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7),] GildedRose(items).update_quality() assert items[0].sell_in == -1 assert items[0].quality == 0Запустите тесты еще раз и измерьте покрытие кода gilded_rose.py тестами.
python % coverage run -m pytest test_gilded_rose.py && coverage report ========================================================================== test session starts ========================================================================== platform darwin -- Python 3.10.9, pytest-7.2.1, pluggy-1.0.0 rootdir: /Users/vmihaylov/PycharmProjects/GildedRose-Refactoring-Kata/python collected 10 items test_gilded_rose.py .......... [100%] ========================================================================== 10 passed in 0.01s =========================================================================== Name Stmts Miss Cover ----------------------------------------- gilded_rose.py 36 3 92% test_gilded_rose.py 53 4 92% ----------------------------------------- TOTAL 89 7 92%Покрытие кода тестами составило 92%.
Запишите сколько было потрачено времени на автоматизацию тестов. Для сравнения у меня ушло на это 40 минут.
Рефакторинг кода 2/4. Рефакторинг метода update_quality
По условиям задачи нельзя изменять код класса Item.
Для обхода этого ограничения создайте класс ItemEntity, реализующий следующие требования:
-
ItemEntity может быть создан из Item, при этом он копирует к себе поля Item
-
ItemEntity имеет атрибут conjured, устанавливаемый в True если название предмета начинается с «Conjured «
-
ItemEntity может быть преобразован в Item, при этом он копирует свои поля в Item
#test_gilded_rose.py ... def test_item_entity_copies_attributes_from_item(): """ ItemEntity может быть создан из Item, при этом он копирует к себе поля Item """ item = Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7) item_entity = ItemEntity(item=item) assert item_entity.name == item.name assert item_entity.sell_in == item.sell_in assert item_entity.quality == item.quality def test_item_entity_detects_conjured_item(): """ ItemEntity имеет атрибут conjured, устанавливаемый в True если название предмета начинается с "Conjured " """ item = Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7) item_entity = ItemEntity(item=item) assert item_entity.is_conjured def test_item_entity_converts_to_item(): """ ItemEntity может быть преобразован в Item, при этом он копирует свои поля в Item """ item = Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7) item_entity = ItemEntity(item=item) copied_item = item_entity.to_item() assert copied_item.name == "Backstage passes to a TAFKAL80ETC concert" assert copied_item.sell_in == 0 assert copied_item.quality == 7#gilded_rose.py ... class ItemEntity: def __init__(self, item): name = item.name self.name = name self.sell_in = item.sell_in self.quality = item.quality self.is_conjured = name.startswith("Conjured ")Перенесите код update_quality из класса GildedRose в класс ItemEntity
#gilded_rose.py class GildedRose(object): ... def update_quality(self): self.items = [ItemEntity(item=item).update_quality().to_item() for item in self.items] ... class ItemEntity: ... def update_quality(self): if self.name != "Aged Brie" and self.name != "Backstage passes to a TAFKAL80ETC concert": if self.quality > 0: if self.name != "Sulfuras, Hand of Ragnaros": self.quality = self.quality - 1 else: if self.quality < 50: self.quality = self.quality + 1 if self.name == "Backstage passes to a TAFKAL80ETC concert": if self.sell_in < 11: if self.quality < 50: self.quality = self.quality + 1 if self.sell_in < 6: if self.quality < 50: self.quality = self.quality + 1 if self.name != "Sulfuras, Hand of Ragnaros": self.sell_in = self.sell_in - 1 if self.sell_in < 0: if self.name != "Aged Brie": if self.name != "Backstage passes to a TAFKAL80ETC concert": if self.quality > 0: if self.name != "Sulfuras, Hand of Ragnaros": self.quality = self.quality - 1 else: self.quality = self.quality - self.quality else: if self.quality < 50: self.quality = self.quality + 1Измените тесты, чтобы они работали с экземпляром ItemEntity вместо GuildedRose
#test_gilded_rose.py ... def test_system_decreases_quality_every_day(): """ В конце каждого дня наша система снижает оба значения для каждого элемента. """ item = ItemEntity(item=Item(name="+5 Dexterity Vest", sell_in=10, quality=1)) item.update_quality() assert item.sell_in == 9 assert item.quality == 0 def test_when_sell_in_expired_quality_decreases_twice_as_fast(): """ Как только срок продажи истек, качество ухудшается в два раза быстрее. """ item = ItemEntity(Item(name="+5 Dexterity Vest", sell_in=0, quality=10)) item.update_quality() assert item.sell_in == -1 assert item.quality == 8 def test_quality_never_becomes_negative(): """ Качество предмета никогда не бывает отрицательным """ item = ItemEntity(Item(name="+5 Dexterity Vest", sell_in=1, quality=0)) item.update_quality() assert item.sell_in == 0 assert item.quality == 0 def test_aged_brie_becomes_better_after_sell_in(): """ «Выдержанный бри» (Aged Brie) на самом деле тем лучше, чем старше он становится. """ item = ItemEntity(Item(name="Aged Brie", sell_in=10, quality=1)) item.update_quality() assert item.sell_in == 9 assert item.quality == 2 def test_item_quality_never_exceeds_50(): """ Качество предмета никогда не превышает 50. """ item = ItemEntity(Item(name="Aged Brie", sell_in=10, quality=50)) item.update_quality() assert item.sell_in == 9 assert item.quality == 50 def test_sulfuras_never_sells_in_and_never_looses_quality(): """ "Sulfuras", будучи легендарным предметом, никогда не продается и не теряет качества. """ item = ItemEntity(Item(name="Sulfuras, Hand of Ragnaros", sell_in=10, quality=80)) item.update_quality() assert item.sell_in == 10 assert item.quality == 80 def test_backstage_passes_increase_quality_by_2_if_10_to_6_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert) повышаются в качестве по мере приближения значения sell_in: качество повышается на 2, если осталось 10 дней или меньше. """ item = ItemEntity(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=6)) item.update_quality() assert item.sell_in == 9 assert item.quality == 8 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), повышаются в качестве по мере приближения значения sell_in: качество повышается на 3, если осталось 5 дней или меньше. """ item = ItemEntity(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=7)) item.update_quality() assert item.sell_in == 5 assert item.quality == 10 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), после концерта качество падает до 0. """ item = ItemEntity(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7)) item.update_quality() assert item.sell_in == -1 assert item.quality == 0 ...Обратите внимание на повторяющуюся if-конструкцию ниже в методе update_quality и вынесите ее в отдельный метод increase_quality.
#gilded_rose.py class ItemEntity: ... def update_quality(self): ... if self.quality < 50: self.quality = self.quality + 1 ...#gilded_rose.py class ItemEntity: ... def increase_quality(self): if self.quality < 50: self.quality = self.quality + 1 ...Обратите внимание на повторяющуюся конструкцию уменьшения качества и вынесите ее в отдельный метод decrease_quality_if_not_sulfuras()
#gilded_rose.py class ItemEntity: ... def update_quality(self): ... if self.quality > 0: if self.name != "Sulfuras, Hand of Ragnaros": self.quality = self.quality - 1 ...#gilded_rose.py class ItemEntity: ... def decrease_quality_if_not_sulfuras(self): if self.quality > 0: if self.name != "Sulfuras, Hand of Ragnaros": self.quality = self.quality - 1 ...Название метода decrease_quality_if_not_sulfuras подсказывает нам, что его реализация должна отличаться для предметов Sulfuras. Реализуйте это поведение с помощью дочернего класса.
Создайте класс Sulfuras и перенесите в него логику уменьшения качества (в данном случае — эта логика состоит в не делании ничего, то есть в конструкции pass).
#gilded_rose.py ... class Sulfuras(ItemEntity): def decrease_quality(self): pass ...Создайте класс EntityFactory, который будет создавать правильный объект в зависимости от его наименования.
#gilded_rose.py ... class EntityFactory: def create(item): if item.name == "Sulfuras, Hand of Ragnaros": return Sulfuras(item=item) else: return ItemEntity(item=item) ...Поменяйте код для использования EntityFactory вместо прямого создания ItemEntity в коде GildedRose и в тестах.
#gilded_rose.py ... class GildedRose(object): ... def update_quality(self): self.items = [EntityFactory.create(item=item).update_quality().to_item() for item in self.items] ...#test_gilded_rose.py ... from gilded_rose import Item, GildedRose, ItemEntity, Sulfuras, EntityFactory ... def test_system_decreases_quality_every_day(): """ В конце каждого дня наша система снижает оба значения для каждого элемента. """ item = EntityFactory.create(item=Item(name="+5 Dexterity Vest", sell_in=10, quality=1)) item.update_quality() assert item.sell_in == 9 assert item.quality == 0 def test_when_sell_in_expired_quality_decreases_twice_as_fast(): """ Как только срок продажи истек, качество ухудшается в два раза быстрее. """ item = EntityFactory.create(Item(name="+5 Dexterity Vest", sell_in=0, quality=10)) item.update_quality() assert item.sell_in == -1 assert item.quality == 8 def test_quality_never_becomes_negative(): """ Качество предмета никогда не бывает отрицательным """ item = EntityFactory.create(Item(name="+5 Dexterity Vest", sell_in=1, quality=0)) item.update_quality() assert item.sell_in == 0 assert item.quality == 0 def test_aged_brie_becomes_better_after_sell_in(): """ «Выдержанный бри» (Aged Brie) на самом деле тем лучше, чем старше он становится. """ item = EntityFactory.create(Item(name="Aged Brie", sell_in=10, quality=1)) item.update_quality() assert item.sell_in == 9 assert item.quality == 2 def test_item_quality_never_exceeds_50(): """ Качество предмета никогда не превышает 50. """ item = EntityFactory.create(Item(name="Aged Brie", sell_in=10, quality=50)) item.update_quality() assert item.sell_in == 9 assert item.quality == 50 def test_sulfuras_never_sells_in_and_never_looses_quality(): """ "Sulfuras", будучи легендарным предметом, никогда не продается и не теряет качества. """ item = EntityFactory.create(Item(name="Sulfuras, Hand of Ragnaros", sell_in=10, quality=80)) item.update_quality() assert item.sell_in == 10 assert item.quality == 80 def test_backstage_passes_increase_quality_by_2_if_10_to_6_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert) повышаются в качестве по мере приближения значения sell_in: качество повышается на 2, если осталось 10 дней или меньше. """ item = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=6)) item.update_quality() assert item.sell_in == 9 assert item.quality == 8 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), повышаются в качестве по мере приближения значения sell_in: качество повышается на 3, если осталось 5 дней или меньше. """ item = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=7)) item.update_quality() assert item.sell_in == 5 assert item.quality == 10 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), после концерта качество падает до 0. """ item = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7)) item.update_quality() assert item.sell_in == -1 assert item.quality == 0 def test_sulfuras_produced_by_factory(): """ Предметы "Sulfuras, Hand of Ragnaros" производят сущность Sulfuras """ sulfuras = EntityFactory.create(Item(name="Sulfuras, Hand of Ragnaros", sell_in=10, quality=80)) assert isinstance(sulfuras, Sulfuras) ...Обратите внимание на конструкцию уменьшения срока годности sell_in и перенесите ее в отдельный метод decrease_sell_in.
#gilded_rose.py ... class ItemEntity: ... def update_quality(self): ... if self.name != "Sulfuras, Hand of Ragnaros": self.sell_in = self.sell_in - 1 ... Сделайте разные реализации decrease_sell_in для обычных предметов и Sulfuras.
#gilded_rose.py class ItemEntity: ... def decrease_sell_in(self): self.sell_in = self.sell_in - 1 ... ... class Sulfuras(ItemEntity): ... def decrease_sell_in(self): pass ...Добавьте тесты для классов «Aged Brie» и «Backstage passes to a TAFKAL80ETC concert», создайте эти классы и добавьте в фабрику логику для создания правильных объектов в зависимости от названия предмета.
#test_gilded_rose.py ... from gilded_rose import Item, GildedRose, ItemEntity, Sulfuras, EntityFactory, AgedBrie, BackstagePasses ... def test_aged_brie_produced_by_factory(): """ Предметы "Aged Brie" производят сущность AgedBrie """ aged_brie = EntityFactory.create(Item(name="Aged Brie", sell_in=10, quality=80)) assert isinstance(aged_brie, AgedBrie) def test_backstage_passes_produced_by_factory(): """ Предметы "Backstage passes to a TAFKAL80ETC concert" производят сущность BackstagePasses """ backstage_passes = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=80)) assert isinstance(backstage_passes, BackstagePasses) ...#gilded_rose.py ... class AgedBrie(ItemEntity): pass class BackstagePasses(ItemEntity): pass class EntityFactory: def create(item): if item.name == "Aged Brie": return AgedBrie(item=item) elif item.name == "Backstage passes to a TAFKAL80ETC concert": return BackstagePasses(item=item) elif item.name == "Sulfuras, Hand of Ragnaros": return Sulfuras(item=item) else: return ItemEntity(item=item) ...Обратите внимание на то, что код улучшения качества в update_quality очень похож на код increase_quality, но для Backstage passes он дополнительно выполняет улучшение качества для разных sell_in.
#gilded_rose.py ... class ItemEntity: ... def increase_quality(self): if self.quality < 50: self.quality = self.quality + 1 ... def update_quality(self): ... if self.quality < 50: self.quality = self.quality + 1 if self.name == "Backstage passes to a TAFKAL80ETC concert": if self.sell_in < 11: self.increase_quality() if self.sell_in < 6: self.increase_quality() ... Перенесите строки 19-25 выше в метод increase_quality класса BackstagePasses, а в теле update_quality оставьте вызов increase_quality.
#gilded_rose.py ... class BackstagePasses(ItemEntity): def increase_quality(self): super().increase_quality() if self.sell_in < 11: super().increase_quality() if self.sell_in < 6: super().increase_quality() class ItemEntity: ... def increase_quality(self): if self.quality < 50: self.quality = self.quality + 1 ... def update_quality(self): if self.name != "Aged Brie" and self.name != "Backstage passes to a TAFKAL80ETC concert": self.decrease_quality() else: self.increase_quality() self.decrease_sell_in() if self.sell_in < 0: if self.name != "Aged Brie": if self.name != "Backstage passes to a TAFKAL80ETC concert": self.decrease_quality() else: self.quality = self.quality - self.quality else: self.increase_quality() return self ...Обратите внимание на конструкцию уменьшения качества для Aged Brie и Backstage passes.
#gilded_rose.py ... class ItemEntity: ... def update_quality(self): if self.name != "Aged Brie" and self.name != "Backstage passes to a TAFKAL80ETC concert": self.decrease_quality() else: self.increase_quality() ...Создайте метод change_quality, который для Aged Brie и Backstage passes будет увеличивать качество, а для остальных сущностей — уменьшать.
#gilded_rose.py ... class ItemEntity: ... def change_quality(self): self.decrease_quality() ... def update_quality(self): self.change_quality() ... ... class AgedBrie(ItemEntity): ... def change_quality(self): self.increase_quality() ... class BackstagePasses(ItemEntity): ... def change_quality(self): self.increase_quality() ...Обратите внимание на фрагмент кода с изменением качества после окончания срока годности. Перенесите его в отдельный метод change_quality_after_sell_in и сделайте отдельные реализации для Aged Brie и Backstage Passes.
#gilded_rose.py ... class ItemEntity: ... def update_quality(self): ... if self.sell_in < 0: if self.name != "Aged Brie": if self.name != "Backstage passes to a TAFKAL80ETC concert": self.decrease_quality() else: self.quality = self.quality - self.quality else: self.increase_quality() ... ...#gilded_rose.py ... class ItemEntity: ... def change_quality_after_sell_in(self): self.decrease_quality() def update_quality(self): self.change_quality() self.decrease_sell_in() if self.sell_in < 0: self.change_quality_after_sell_in() return self ... class AgedBrie(ItemEntity): ... def change_quality_after_sell_in(self): self.increase_quality() class BackstagePasses(ItemEntity): ... def change_quality_after_sell_in(self): self.quality = 0Рефакторинг кода 3/4. Проверьте цикломатическую сложность
$ radon cc -s gilded_rose.py gilded_rose.py C 56:0 EntityFactory - A (5) M 57:4 EntityFactory.create - A (4) C 1:0 GildedRose - A (3) C 74:0 BackstagePasses - A (3) M 81:4 BackstagePasses.increase_quality - A (3) M 6:4 GildedRose.update_quality - A (2) C 9:0 Item - A (2) C 18:0 ItemEntity - A (2) M 26:4 ItemEntity.increase_quality - A (2) M 30:4 ItemEntity.decrease_quality - A (2) M 43:4 ItemEntity.update_quality - A (2) C 67:0 AgedBrie - A (2) C 89:0 Sulfuras - A (2) M 3:4 GildedRose.__init__ - A (1) M 10:4 Item.__init__ - A (1) M 15:4 Item.__repr__ - A (1) M 19:4 ItemEntity.__init__ - A (1) M 34:4 ItemEntity.decrease_sell_in - A (1) M 37:4 ItemEntity.change_quality - A (1) M 40:4 ItemEntity.change_quality_after_sell_in - A (1) M 53:4 ItemEntity.to_item - A (1) M 68:4 AgedBrie.change_quality - A (1) M 71:4 AgedBrie.change_quality_after_sell_in - A (1) M 75:4 BackstagePasses.change_quality - A (1) M 78:4 BackstagePasses.change_quality_after_sell_in - A (1) M 90:4 Sulfuras.decrease_quality - A (1) M 93:4 Sulfuras.decrease_sell_in - A (1)Все методы GuildedRose теперь имеют цикломатическую сложность менее 5.
Рефакторинг кода 4/4. Добавьте новое требование
Напишите тест для реализации требования и скорректируйте ItemEntity:
-
Качество «Сотворенных» (Conjured) предметов ухудшается в два раза быстрее, чем у обычных предметов.
#test_gilded_rose.py ... def test_conjured_items_quality_decreases_twice_as_fast(): """ Качество «Сотворенных» (Conjured) предметов ухудшается в два раза быстрее, чем у обычных предметов. """ item = EntityFactory.create(item=Item(name="Conjured +5 Dexterity Vest", sell_in=10, quality=5)) item.update_quality() assert item.sell_in == 9 assert item.quality == 3 def test_conjured_aged_brie_produce_aged_brie_by_factory(): """ Предметы "Conjured Aged Brie" производят сущность AgedBrie """ aged_brie = EntityFactory.create(Item(name="Conjured Aged Brie", sell_in=10, quality=80)) assert isinstance(aged_brie, AgedBrie)Измерьте сколько времени ушло на добавление требования и тестирование в код, прошедший coding kata. У меня это заняло 10 минут.
Измерьте потраченное время на рефакторинг и посчитайте сколько раз Вы запускали тесты. Для сравнения у меня ушло 2 часа, а тесты запускались около 100 раз.
Финальный листинг
#gilded_rose.py class GildedRose(object): def __init__(self, items): self.items = items def update_quality(self): self.items = [EntityFactory.create(item=item).update_quality().to_item() for item in self.items] class Item: def __init__(self, name, sell_in, quality): self.name = name self.sell_in = sell_in self.quality = quality def __repr__(self): return "%s, %s, %s" % (self.name, self.sell_in, self.quality) class ItemEntity: def __init__(self, item): name = item.name self.name = name self.sell_in = item.sell_in self.quality = item.quality self.is_conjured = name.startswith("Conjured ") def increase_quality(self): if self.quality < 50: self.quality = self.quality + 1 def decrease_quality(self): if self.quality > 0: self.quality = self.quality - 1 if self.is_conjured: if self.quality > 0: self.quality = self.quality - 1 def decrease_sell_in(self): self.sell_in = self.sell_in - 1 def change_quality(self): self.decrease_quality() def change_quality_after_sell_in(self): self.decrease_quality() def update_quality(self): self.change_quality() self.decrease_sell_in() if self.sell_in < 0: self.change_quality_after_sell_in() return self def to_item(self): return Item(name=self.name, sell_in=self.sell_in, quality=self.quality) class EntityFactory: def remove_conjured_if_exists(name): if name.startswith("Conjured "): return name[9:] else: return name def create(item): stripped_name = EntityFactory.remove_conjured_if_exists(item.name) if stripped_name == "Aged Brie": return AgedBrie(item=item) elif stripped_name == "Backstage passes to a TAFKAL80ETC concert": return BackstagePasses(item=item) elif stripped_name == "Sulfuras, Hand of Ragnaros": return Sulfuras(item=item) else: return ItemEntity(item=item) class AgedBrie(ItemEntity): def change_quality(self): self.increase_quality() def change_quality_after_sell_in(self): self.increase_quality() class BackstagePasses(ItemEntity): def change_quality(self): self.increase_quality() def change_quality_after_sell_in(self): self.quality = 0 def increase_quality(self): super().increase_quality() if self.sell_in < 11: super().increase_quality() if self.sell_in < 6: super().increase_quality() class Sulfuras(ItemEntity): def decrease_quality(self): pass def decrease_sell_in(self): pass #test_gilded_rose.py import pytest from gilded_rose import Item, GildedRose, ItemEntity, Sulfuras, EntityFactory, AgedBrie, BackstagePasses def test_item_has_sell_in_attribute(): """ Все предметы имеют значение "продать в течение" (sell_in), которое обозначает количество дней, в течение которых мы должны продать предмет. """ item = Item(name="Foo", sell_in=10, quality=1) assert item.sell_in == 10 def test_item_has_quality_attribute(): """ Все предметы имеют значение качества (quality), которое указывает, насколько ценен предмет. """ item = Item(name="Foo", sell_in=10, quality=1) assert item.quality == 1 def test_system_decreases_quality_every_day(): """ В конце каждого дня наша система снижает оба значения для каждого элемента. """ item = EntityFactory.create(item=Item(name="+5 Dexterity Vest", sell_in=10, quality=1)) item.update_quality() assert item.sell_in == 9 assert item.quality == 0 def test_when_sell_in_expired_quality_decreases_twice_as_fast(): """ Как только срок продажи истек, качество ухудшается в два раза быстрее. """ item = EntityFactory.create(Item(name="+5 Dexterity Vest", sell_in=0, quality=10)) item.update_quality() assert item.sell_in == -1 assert item.quality == 8 def test_quality_never_becomes_negative(): """ Качество предмета никогда не бывает отрицательным """ item = EntityFactory.create(Item(name="+5 Dexterity Vest", sell_in=1, quality=0)) item.update_quality() assert item.sell_in == 0 assert item.quality == 0 def test_aged_brie_becomes_better_after_sell_in(): """ «Выдержанный бри» (Aged Brie) на самом деле тем лучше, чем старше он становится. """ item = EntityFactory.create(Item(name="Aged Brie", sell_in=10, quality=1)) item.update_quality() assert item.sell_in == 9 assert item.quality == 2 def test_item_quality_never_exceeds_50(): """ Качество предмета никогда не превышает 50. """ item = EntityFactory.create(Item(name="Aged Brie", sell_in=10, quality=50)) item.update_quality() assert item.sell_in == 9 assert item.quality == 50 def test_sulfuras_never_sells_in_and_never_looses_quality(): """ "Sulfuras", будучи легендарным предметом, никогда не продается и не теряет качества. """ item = EntityFactory.create(Item(name="Sulfuras, Hand of Ragnaros", sell_in=10, quality=80)) item.update_quality() assert item.sell_in == 10 assert item.quality == 80 def test_backstage_passes_increase_quality_by_2_if_10_to_6_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert) повышаются в качестве по мере приближения значения sell_in: качество повышается на 2, если осталось 10 дней или меньше. """ item = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=6)) item.update_quality() assert item.sell_in == 9 assert item.quality == 8 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), повышаются в качестве по мере приближения значения sell_in: качество повышается на 3, если осталось 5 дней или меньше. """ item = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=7)) item.update_quality() assert item.sell_in == 5 assert item.quality == 10 def test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left(): """ «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), после концерта качество падает до 0. """ item = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7)) item.update_quality() assert item.sell_in == -1 assert item.quality == 0 def test_item_entity_copies_attributes_from_item(): """ ItemEntity может быть создан из Item, при этом он копирует к себе поля Item """ item = Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7) item_entity = ItemEntity(item=item) assert item_entity.name == item.name assert item_entity.sell_in == item.sell_in assert item_entity.quality == item.quality def test_item_entity_detects_conjured_item(): """ ItemEntity имеет атрибут conjured, устанавливаемый в True если название предмета начинается с "Conjured " """ item = Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7) item_entity = ItemEntity(item=item) assert item_entity.is_conjured def test_item_entity_converts_to_item(): """ ItemEntity может быть преобразован в Item, при этом он копирует свои поля в Item """ item = Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7) item_entity = ItemEntity(item=item) copied_item = item_entity.to_item() assert copied_item.name == "Backstage passes to a TAFKAL80ETC concert" assert copied_item.sell_in == 0 assert copied_item.quality == 7 def test_sulfuras_produced_by_factory(): """ Предметы "Sulfuras, Hand of Ragnaros" производят сущность Sulfuras """ sulfuras = EntityFactory.create(Item(name="Sulfuras, Hand of Ragnaros", sell_in=10, quality=80)) assert isinstance(sulfuras, Sulfuras) def test_aged_brie_produced_by_factory(): """ Предметы "Aged Brie" производят сущность AgedBrie """ aged_brie = EntityFactory.create(Item(name="Aged Brie", sell_in=10, quality=80)) assert isinstance(aged_brie, AgedBrie) def test_backstage_passes_produced_by_factory(): """ Предметы "Backstage passes to a TAFKAL80ETC concert" производят сущность BackstagePasses """ backstage_passes = EntityFactory.create(Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=80)) assert isinstance(backstage_passes, BackstagePasses) def test_conjured_items_quality_decreases_twice_as_fast(): """ Качество «Сотворенных» (Conjured) предметов ухудшается в два раза быстрее, чем у обычных предметов. """ item = EntityFactory.create(item=Item(name="Conjured +5 Dexterity Vest", sell_in=10, quality=5)) item.update_quality() assert item.sell_in == 9 assert item.quality == 3 def test_conjured_aged_brie_produce_aged_brie_by_factory(): """ Предметы "Conjured Aged Brie" производят сущность AgedBrie """ aged_brie = EntityFactory.create(Item(name="Conjured Aged Brie", sell_in=10, quality=80)) assert isinstance(aged_brie, AgedBrie)Заключение
Coding kata для 30 строк кода и покрытие их тестами заняли около 3 часов.
В процессе выполнения coding kata автоматические тесты запускались более 100 раз.
Итоговый вариант имеет цикломатическую сложность 5 вместо 19.
Внесение нового требования в код заняло менее 10 минут.
Применение данного подхода в промышленном масштабе на практике показало успешное сокращение time2market производства до 96 раз.
Спасибо, что дочитали до этого места. О других причинах высокого time2market можно узнать здесь.
ссылка на оригинал статьи https://habr.com/ru/post/718746/
Добавить комментарий