24.02.2023 09:49
vmihaylov 13 2100 Источник

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

За время проекта два раза поменяли команду и заказчика.

Аудит процесса производства с точки зрения time2market (сроков от появления идеи до запуска в промышленное использование) показал, что задержки возникают на трех стадиях:

  • долго вносятся изменения в код

  • долго выполняется ручное тестирование

  • часто после тестирования код возвращается на доработку

Первопричиной низкого time2market являлось то, что команда не владела практикой coding kata, о которой я подробно расскажу в данной статье на примере задачи "Позолоченная роза".

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

После применения приемов, описанных в этой статье, оставшиеся 95% проекта описанная выше команда реализовала и сдала за 6 месяцев вместо прогнозируемых 48 лет, что эквивалентно сокращению time2market в 96 раз.

Вы можете попробовать самостоятельно решить эту задачу в коде до прохождения рефакторинга и после рефакторинга используя ссылку https://github.com/vasiliy-mikhailov/gilded_rose_python.

Позолоченная роза: условия задачи для coding kata

Привет и добро пожаловать в команду Gilded Rose. Как вы знаете, мы - небольшая гостиница, расположенная в отличном месте одного известного города, которой управляет дружелюбный трактирщик по имени Эллисон. Мы также покупаем и продаем лучшие товары. К сожалению, качество наших товаров постоянно ухудшается по мере приближения срока реализации. У нас есть система, поддерживающая работу склада. Ее разработал серьезный тип по имени Лирой, отправившийся навстречу новым приключениям. Ваша задача — добавить новую функцию в нашу систему, чтобы мы могли начать продавать новую категорию товаров. Сначала введение в нашу систему:

  • Все предметы имеют срок реализации (sell_in), который обозначает количество дней, в течение которых мы должны продать предмет.

  • Все предметы имеют значение качества (quality), которое указывает, насколько ценен предмет.

  • В конце каждого дня наша система снижает оба значения для каждого элемента.

Довольно просто, правда? Ну вот тут становится интересно:

  • Как только срок реализации истек, качество ухудшается в два раза быстрее.

  • Качество предмета никогда не бывает отрицательным

  • «Выдержанный бри» (Aged Brie) на самом деле тем лучше, чем старше он становится.

  • Качество предмета никогда не превышает 50.

  • "Sulfuras", будучи легендарным предметом, никогда не продается и не теряет качества.

  • «Проходы за кулисы» (Backstage passes to a TAFKAL80ETC concert), также как выдержанный бри (Aged Brie), повышаются в качестве по мере приближения срока реализации: качество повышается на 2, если осталось 10 дней или меньше, и на 3, если осталось 5 дней или меньше, но после концерта качество падает до 0.

Недавно мы заключили договор с поставщиком колдовских предметов. Для этого требуется обновление нашей системы:

  • Качество «Сотворенных» (Conjured) предметов ухудшается в два раза быстрее, чем у обычных предметов.

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

Ваша работа должна быть завершена к пятнице, 18 февраля 2011 г., 08:00:00 по тихоокеанскому стандартному времени (примечание: задание было опубликовано 13 февраля 2011 года).

Просто для пояснения: качество предмета никогда не может повыситься выше 50, однако «Sulfuras» — это легендарный предмет, и поэтому его Качество равно 80, и оно никогда не меняется.

Исходный код: 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 " перед названием предмета. Проверить гипотезу я не смог, поэтому принял ее на веру.

  • Возник вопрос как поведет себя сотворенный Aged Brie и другие предметы. Я выдвинул гипотезу, что качество сотворенного Aged Brie будет повышаться на 2 единицы каждый день. Качество других "особенных" предметов также будет изменяться в соответствии с ранее описанными требованиями, но в 2 раза быстрее. Проверить эту гипотезу я не смог, поэтому принял ее на веру.

Аудит процесса производства 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

Запишите рекомендации по итогу изучения.

Для сравнения привожу свои рекомендации:

  1. Для сокращения time2market проектирования: провести рефакторинг кода update_quality в соответствии с принципами SOLID

  2. Для сокращения time2market разработки: снизить цикломатическую сложность кода до 5.

  3. Для сокращения 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 == 4
    assert items[0].quality == 10

def test_backstage_passes_drop_quality_to_zero_when_0_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 == 4
    assert item.quality == 10

def test_backstage_passes_drop_quality_to_zero_when_0_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 == 4
    assert item.quality == 10

def test_backstage_passes_drop_quality_to_zero_when_0_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)

def test_conjured_sulfuras_does_not_sell_and_quality_does_not_decrease():
    """
    Качество «Сотворенного» Sulfuras не ухудшается и он не продается.
    """
    item = EntityFactory.create(item=Item(name="Conjured Sulfuras, Hand of Ragnaros", sell_in=10, quality=80))
    item.update_quality()
    assert item.sell_in == 10
    assert item.quality == 80

def test_conjured_aged_brie_becomes_better_after_sell_in_twice_as_fast():
    """
    Сотворенный «Выдержанный бри» (Aged Brie) на самом деле становится тем лучше, чем старше он становится в 2 раза быстрее, чем не сотворенный.
    """
    item = EntityFactory.create(item=Item(name="Conjured Aged Brie", sell_in=10, quality=1))
    item.update_quality()
    assert item.sell_in == 9
    assert item.quality == 3

def test_conjured_backstage_passes_increase_quality_by_4_if_10_to_6_days_left():
    """
    Сотворенные «Проходы за кулисы» (Conjured Backstage passes to a TAFKAL80ETC concert) повышаются в качестве по мере приближения значения sell_in: качество повышается на 4, если осталось 10 дней или меньше.
    """
    item = EntityFactory.create(item=Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=6))
    item.update_quality()
    assert item.sell_in == 9
    assert item.quality == 10

def test_conjured_backstage_passes_increase_quality_by_6_if_5_to_1_days_left():
    """
    Сотворенные «Проходы за кулисы» (Conjured Backstage passes to a TAFKAL80ETC concert), повышаются в качестве по мере приближения значения sell_in: качество повышается на 6, если осталось 5 дней или меньше.
    """
    item = EntityFactory.create(item=Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=7))
    item.update_quality()
    assert item.sell_in == 4
    assert item.quality == 13

def test_conjured_backstage_passes_drop_quality_to_zero_when_0_days_left():
    """
    Сотворенные «Проходы за кулисы» (Conjured Backstage passes to a TAFKAL80ETC concert), после концерта качество падает до 0.
    """
    item = EntityFactory.create(item=Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7))
    item.update_quality()
    assert item.sell_in == -1
    assert item.quality == 0
#gilded_rose_cyclomatic_complexity_5.py

...

class ItemEntity:
    def __init__(self, item):
        ...
        self.is_conjured = name.startswith("Conjured ")
        self.quality_increase_factor = 2 if self.is_conjured else 1

    def increase_quality(self):
        self.quality = min(self.quality + 1 * self.quality_increase_factor, 50)

    def decrease_quality(self):
        self.quality = max(self.quality - 1 * self.quality_increase_factor, 0)

...

Измерьте сколько времени ушло на добавление требования и тестирование в код, прошедший coding kata. У меня это заняло 10 минут.

Измерьте потраченное время на рефакторинг и посчитайте сколько раз Вы запускали тесты. Для сравнения у меня ушло 2 часа, а тесты запускались около 100 раз.

Финальный листинг

#gilded_rose_cyclomatic_complexity_5.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 ")
        self.quality_increase_factor = 2 if self.is_conjured else 1

    def increase_quality(self):
        self.quality = min(self.quality + 1 * self.quality_increase_factor, 50)

    def decrease_quality(self):
        self.quality = max(self.quality - 1 * self.quality_increase_factor, 0)

    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 decrease_quality(self):
        super().decrease_quality()

    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_cyclomatic_complexity_5.py

import pytest
import gilded_rose_cyclomatic_complexity_5

from gilded_rose_cyclomatic_complexity_5 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 == 4
    assert item.quality == 10

def test_backstage_passes_drop_quality_to_zero_when_0_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)

def test_conjured_sulfuras_does_not_sell_and_quality_does_not_decrease():
    """
    Качество «Сотворенного» Sulfuras не ухудшается и он не продается.
    """
    item = EntityFactory.create(item=Item(name="Conjured Sulfuras, Hand of Ragnaros", sell_in=10, quality=80))
    item.update_quality()
    assert item.sell_in == 10
    assert item.quality == 80

def test_conjured_aged_brie_becomes_better_after_sell_in_twice_as_fast():
    """
    Сотворенный «Выдержанный бри» (Aged Brie) на самом деле становится тем лучше, чем старше он становится в 2 раза быстрее, чем не сотворенный.
    """
    item = EntityFactory.create(item=Item(name="Conjured Aged Brie", sell_in=10, quality=1))
    item.update_quality()
    assert item.sell_in == 9
    assert item.quality == 3

def test_conjured_backstage_passes_increase_quality_by_4_if_10_to_6_days_left():
    """
    Сотворенные «Проходы за кулисы» (Conjured Backstage passes to a TAFKAL80ETC concert) повышаются в качестве по мере приближения значения sell_in: качество повышается на 4, если осталось 10 дней или меньше.
    """
    item = EntityFactory.create(item=Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=6))
    item.update_quality()
    assert item.sell_in == 9
    assert item.quality == 10

def test_conjured_backstage_passes_increase_quality_by_6_if_5_to_1_days_left():
    """
    Сотворенные «Проходы за кулисы» (Conjured Backstage passes to a TAFKAL80ETC concert), повышаются в качестве по мере приближения значения sell_in: качество повышается на 6, если осталось 5 дней или меньше.
    """
    item = EntityFactory.create(item=Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=7))
    item.update_quality()
    assert item.sell_in == 4
    assert item.quality == 13

def test_conjured_backstage_passes_drop_quality_to_zero_when_0_days_left():
    """
    Сотворенные «Проходы за кулисы» (Conjured Backstage passes to a TAFKAL80ETC concert), после концерта качество падает до 0.
    """
    item = EntityFactory.create(item=Item(name="Conjured Backstage passes to a TAFKAL80ETC concert", sell_in=0, quality=7))
    item.update_quality()
    assert item.sell_in == -1
    assert item.quality == 0

Финальный листинг также можно скачать на github: https://github.com/vasiliy-mikhailov/gilded_rose_python

Заключение

Coding kata для 30 строк кода и покрытие их тестами заняли около 3 часов.

В процессе выполнения coding kata автоматические тесты запускались более 100 раз.

Итоговый вариант имеет цикломатическую сложность 5 вместо 19.

Внесение нового требования в код заняло менее 10 минут.

Применение данного подхода в промышленном масштабе на практике показало успешное сокращение time2market производства до 96 раз.

Спасибо, что дочитали до этого места. О других причинах высокого time2market можно узнать здесь.

Комментарии (13)


  1. Survtur
    00.00.0000 00:00
    #25264280

    За время проекта два раза поменяли команду и заказчика.

    Подскажите, как можно и команду, и заказчика?


    1. vmihaylov Автор
      00.00.0000 00:00
      #25264310
      +1

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


  1. haaji
    00.00.0000 00:00
    #25264826
    +2

    Ради интереса - эту статью какая-то продвинутая gpt-3 сеть сгенерировала?


    1. vmihaylov Автор
      00.00.0000 00:00
      #25264862

      Нет, но такая тоже скоро будет. ChatGPT пишет хорошо, хотя и ошибается.


  1. andrtos
    00.00.0000 00:00
    #25265012

    А почему у предмета Sulfuras качество 80, если по условиям оно не может быть выше 50?


    1. vmihaylov Автор
      00.00.0000 00:00
      #25265070

      Спасибо за вопрос. В первоначальном тексте статьи я использовал страницу с неполными требованиями. В других источниках есть такое продолжение:

      Your work needs to be completed by Friday, February 18, 2011 08:00:00 AM PST.

      Just for clarification, an item can never have its Quality increase above 50, however "Sulfuras" is a legendary item and as such its Quality is 80 and it never alters.

      Дополнил текст статьи.


  1. amakhrov
    00.00.0000 00:00
    #25265676
    +3

    Первопричиной низкого time2market являлось то, что команда не владела практикой coding kata, о которой я подробно расскажу в данной статье

    Так и не увидел в этой статье подробного рассказа (или хотя бы общего описания, чего уж там) о практике coding kata. И частности, о том как эта практика помогает с time2market. Собственно, даже и самого определения этого самого time2market.


  1. Eugene_Rymarev
    00.00.0000 00:00
    #25266368
    +1

    @vmihaylov, мне кажется, что в тесте test_backstage_passes_increase_quality_by_3_if_5_to_1_days_left
    ошибка. Там после выполнения метода не меняется sell_in с 5 на 4.
    А ещё имя следующей функции (теста) точно такое же.

    В ItemEntity.__init__ непонятно зачем нужно name = item.name, когда проще сразу написать self.name = item.name, а в конце вместо name.startswith, написать self.name.startswith.

    Конструкции вида self.quality = self.quality + 1 тоже непонятно - зачем? Есть же self.quality += 1.

    Если не считать эти моменты, которые скорее являются опечатками со стороны автора и придирками с моей стороны, то статья мне показалась очень интересной.


    1. vmihaylov Автор
      00.00.0000 00:00
      #25266450

      Евгений, спасибо большое за комментарий и найденную ошибку в тесте. Я перепроверю и вернусь. Скорее всего, первый вариант метода (с ошибкой) не исполняется вовсе - специфика python.


    1. vmihaylov Автор
      00.00.0000 00:00
      #25266992

      Поправил. Когда python встречает два метода с одним названием, он оставляет вторую реализацию. Изменение названия метода решило проблему.


      1. Eugene_Rymarev
        00.00.0000 00:00
        #25267116

        А если бы писалось в IDE, то она бы подсветила "задвоение" функции. А что насчёт двух других моментов, о которых я написал?


  1. Andrey_Solomatin
    00.00.0000 00:00
    #25295222
    +1

    Спасибо за разбор задачи.

    Про финальное покрытие не написали, а это та редкая задача, которая позволяет достичь 100% покрытия. Полное покрытие кончено не значит, что всё проверили.

    Например test_item_quality_never_exceeds_50 тестирует только сыр, но не тестирует билеты. Если смотреть на это как на юнит-тесты, то тестировать на билетах не надо, ведь мы знаем про наследования, но если смотреть на них как на e2e тесты (задача такая изолированная, что все виды тестов выглядят одинаково) то такого теста не хватает.


    Этот код можно значительно упростить просто убрав его.

    def decrease_quality(self): super().decrease_quality()


    Мне с тестами проще смотреть последовательностями, ну и конечно генератор тестов. Черновой код примерно такой:





  1. Andrey_Solomatin
    00.00.0000 00:00
    #25295244
    +1

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