The Gilded Rose is a very popular refactoring exercise that is intended for practicing your test case writing and refactoring skills. It represents the dreaded situation we have to face as software engineers when we have to work on a legacy codebase and add a new feature to it. Soon after implementing the feature, we discover that some part of our system starts to malfunction even though we have not touched it. Refactoring becomes a necessity then. And refactoring without writing unit tests is a pain in the neck because you do not know what side effects you are going to introduce that may eventually make the whole system grind to a halt.

What is the story?

Gilded Rose sells some finest goods belonging to the following categories:

• Normal Items
• Aged Bries
• Sulfuras
• Backstage passes

You are given the task of implementing a new feature to an existing legacy system so that they can sell a new category of Conjured items. But the system is nothing short of a complete mess of unmaintainable spaghetti code with a bunch of if-else statements. Furthermore, there are no test cases!

Please take your time and read the Gilded Rose Requirements Specification here.

How to do the refactoring?

Before you refactor a codebase, you must make sure:

• You do not alter the behavior of the system in any way. The system must continue to function the same way it used to function i.e. the correct behaviors must be retained.
• You do not make any change that forces the consumer code to change as well.

When you refactor your codebase, you must make sure:

• Your code is SOLID-compliant. You can read this beautifully crafted article on SOLID principles- SOLID: The First 5 Principles of Object-Oriented Design
• Avoid nested conditionals, instead use guard clauses.
• Avoid comments. Make your code as readable and expressive as possible. Comments only indicate your lack of ability to express your code correctly. Instead document your code. In Python, we use docstrings for code documentation. Docstrings are not separate from the actual code, they are a part of the code itself.
• Avoid duplicating code. If you find yourself repeating something, better wrap that logic in a helper function and reuse it.
• Write unit tests, and write them a lot.
• Aim for high cohesion and low-coupling.

Let’s start solving the kata now.

The Unit Tests

Guess what? You are given a system that does not have test cases and is very tightly coupled. First thing first, let’s write some unit tests for all the categories of items that Gilded Rose will sell. (I am assuming that you have already read the requirement specification of Gilded Rose)

There are several libraries in Python for writing unit tests. The most popular ones are being unittest and pytest. In this tutorial, I will be using the built-in unittest library.

We want to update the quality of an item multiple times. So it’s better to have a bulk updater. Let’s implement a static method for that. Here is the complete test case suite for the items sold by Gilded Rose.

import unittest
from gilded_rose import GildedRose, Item


class GildedRoseTest(unittest.TestCase):

    @staticmethod
    def bulk_updater(obj: GildedRose, times: int = 1) -> None:
        for _ in range(times):
            obj.update_quality()

    def test_normal_item(self) -> None:
        item = Item("Mountain Dew", 20, 40)
        gilded_rose = GildedRose([item])
        gilded_rose.update_quality()
        self.assertEqual("Mountain Dew", item.name)
        self.assertEqual(item.sell_in, 19)
        self.assertEqual(item.quality, 39)
        GildedRoseTest.bulk_updater(gilded_rose, item.sell_in + 1)

        # sell by date passed
        self.assertEqual(item.sell_in, -1)

        # Quality degrades twice as fast now
        self.assertEqual(item.quality, 18)
        GildedRoseTest.bulk_updater(gilded_rose, 10)
        self.assertEqual(item.quality, 0)  # never be less than 0
        self.assertEqual(item.sell_in, -11)

    def test_aged_brie(self) -> None:
        item = Item("Aged Brie", 20, 20)
        gilded_rose = GildedRose([item])
        gilded_rose.update_quality()
        self.assertEqual("Aged Brie", item.name)
        self.assertEqual(item.sell_in, 19)
        self.assertEqual(item.quality, 21)
        GildedRoseTest.bulk_updater(gilded_rose, item.sell_in + 1)

        # sell by date passed
        self.assertEqual(item.sell_in, -1)

        # Quality increases twice as fast now
        self.assertEqual(item.quality, 42)
        GildedRoseTest.bulk_updater(gilded_rose, 10)
        self.assertEqual(item.quality, 50)  # never be more than 50
        self.assertEqual(item.sell_in, -11)

    def test_sulfuras(self) -> None:
        item = Item("Sulfuras, Hand of Ragnaros", 20, 80)
        gilded_rose = GildedRose([item])
        gilded_rose.update_quality()
        self.assertEqual("Sulfuras, Hand of Ragnaros", item.name)
        self.assertNotEqual(item.sell_in, 19)
        self.assertEqual(item.sell_in, 20)  # does not change
        self.assertEqual(item.quality, 80)  # never alters
        GildedRoseTest.bulk_updater(gilded_rose, item.sell_in + 1)

        # sell by date passed
        self.assertEqual(item.sell_in, 20)  # still same
        self.assertNotEqual(item.sell_in, -1)  # it is not equal so passes

    def test_backstage_passes(self) -> None:
        item = Item("Backstage passes to a TAFKAL80ETC concert", 20, 20)
        gilded_rose = GildedRose([item])
        gilded_rose.update_quality()
        self.assertEqual("Backstage passes to a TAFKAL80ETC concert", item.name)
        self.assertEqual(item.sell_in, 19)
        self.assertEqual(item.quality, 21)
        GildedRoseTest.bulk_updater(gilded_rose, 9)

        # sellin approaches to 10 or less
        self.assertEqual(item.sell_in, 10)
        self.assertEqual(item.quality, 30)
        GildedRoseTest.bulk_updater(gilded_rose)
        self.assertEqual(item.sell_in, 9)
        self.assertEqual(item.quality, 32)

        # sellin approaches to 5 or less
        GildedRoseTest.bulk_updater(gilded_rose, 4)
        self.assertEqual(item.sell_in, 5)
        self.assertEqual(item.quality, 40)
        GildedRoseTest.bulk_updater(gilded_rose)
        self.assertEqual(item.sell_in, 4)
        self.assertEqual(item.quality, 43)
        GildedRoseTest.bulk_updater(gilded_rose, 2)
        self.assertEqual(item.sell_in, 2)
        self.assertEqual(item.quality, 49)

        # corner cases: never gets beyond 50 and drops to 0 once sellin passed
        GildedRoseTest.bulk_updater(gilded_rose)
        self.assertEqual(item.sell_in, 1)
        self.assertEqual(item.quality, 50)

        GildedRoseTest.bulk_updater(gilded_rose, 2)
        self.assertEqual(item.sell_in, -1)
        self.assertEqual(item.quality, 0)

    def test_conjured(self) -> None:
        item = Item("Conjured Mana Cake", 20, 50)
        gilded_rose = GildedRose([item])
        gilded_rose.update_quality()
        self.assertEqual("Conjured Mana Cake", item.name)
        self.assertEqual(item.sell_in, 19)
        self.assertEqual(item.quality, 48)
        GildedRoseTest.bulk_updater(gilded_rose, item.sell_in + 1)

        # sell by date passed -- will decrease twice as fast as normal items (-4)
        self.assertEqual(item.sell_in, -1)
        self.assertEqual(item.quality, 6)

        GildedRoseTest.bulk_updater(gilded_rose)
        self.assertEqual(item.sell_in, -2)
        self.assertEqual(item.quality, 2)

        GildedRoseTest.bulk_updater(gilded_rose)
        self.assertEqual(item.sell_in, -3)
        self.assertEqual(item.quality, 0)   # never less than 0


if __name__ == '__main__':
    unittest.main()

Refactoring the codebase

If you look at the current state of Gilded Rose’s codebase, especially the update_quality method of the GildedRose class, you’ll see how messy it is.

You cannot implement a new feature to this codebase without refactoring it first.

So, how to refactor this chaos without affecting the behavior of the system?

Don’t do this

You may be tempted to make the Item class a subclass of ABC – the abstract base class in Python and have a update_quality abstract method. Any class that inherits from the Item class will require to implement the update_quality method of their own.

from abc import ABC, abstractmethod


class Item(ABC):
    MAX_QUALITY = 50
    MIN_QUALITY = 0

    def __init__(self, name, sell_in, quality):
        self.name = name
        self.sell_in = sell_in
        self.quality = quality

    def __repr__(self):
        return f"{self.name}, {self.sell_in}, {self.quality}"

    @abstractmethod
    def update_quality(self):
        pass


class NormalItem(Item):

    def update_quality(self) -> None:
        cls = self.__class__
        if cls.MIN_QUALITY <= self.quality  0 else self.quality - 2
            self.quality = max(quality, cls.MIN_QUALITY)
        self.sell_in -= 1

        
# so on
...
...
...

But there is something terribly wrong. Although you have successfully separated items and retained their behavior, you have failed to comply with the stated requirements not to alter the Item class:

However, do not alter the Item class or Items property as those belong to the goblin in the corner who will insta-rage and one-shot you as he doesn't believe in shared code ownership (you can make the UpdateQuality method and Items property static if you like, we'll cover for you).

You were not given access to the consumer code, which only instantiates objects of type Item, so the system will fail altogether.

Do it the right way

Let’s implement some free functions and let them update the items. First, refactor the update_quality method of the GildedRose class.

class GildedRose:

    def __init__(self, items):
        self.items = items

    def update_quality(self):
        for item in self.items:

            if item.name == "Aged Brie":
                aged_brie_updater(item)
            elif item.name == "Backstage passes to a TAFKAL80ETC concert":
                backstage_passes_updater(item)
            elif item.name == "Sulfuras, Hand of Ragnaros":
                continue
            elif item.name == "Conjured Mana Cake":
                conjured_updater(item)
            else:
                normal_item_updater(item)

This is cleaner compared to what we have started with. We have removed all the nested `if conditionals.

Let’s define constants for MAX and MIN quality.

MAX_QUALITY = 50
MIN_QUALITY = 0

Normal Item Updater

In general, all items have the following properties as specified in the requirement specification:

• All items have a SellIn value which denotes the number of days we have to sell the item
• All items have a Quality value which denotes how valuable the item is
• At the end of each day our system lowers both values for every item

But there is a special condition:

• Once the sell by date has passed, Quality degrades twice as fast

Let’s convert all these requirements into code.

def normal_item_updater(normal_item: Item):

    if MIN_QUALITY <= normal_item.quality  0 else normal_item.quality - 2
        
        normal_item.quality = max(quality, MIN_QUALITY)
        
    normal_item.sell_in -= 1

Aged Brie Updater

There is an interesting fact about the Aged Brie items.

• "Aged Brie" actually increases in Quality the older it gets

def aged_brie_updater(aged_brie: Item):

    if MIN_QUALITY <= aged_brie.quality  0 else aged_brie.quality + 2
        aged_brie.quality = min(MAX_QUALITY, quality)
    aged_brie.sell_in -= 1

Sulfuras Updater

If we read the requirement specification, we will see that we don’t even need an updater for the Sulfuras items.

• "Sulfuras", being a legendary item, never has to be sold or decreases in Quality

Ahh! Good riddance!

Backstage Passes Updater

This is a little bit tricky to implement.

Let’s revisit the special condition of Backstage passes items.

• "Backstage passes", like aged brie, increases in Quality as its SellIn value approaches; Quality increases by 2 when there are 10 days or less and by 3 when there are 5 days or less but Quality drops to 0 after the concert

Now, let’s convert this into code:

def backstage_passes_updater(backstage: Item):

    if MIN_QUALITY <= backstage.quality <= MAX_QUALITY:
        if backstage.sell_in <= 0:
            backstage.quality = 0
        else:
            quality = backstage.quality + 3 if 0 <= backstage.sell_in <= 5 else \
                backstage.quality + 2 if 5 < backstage.sell_in <= 10 else backstage.quality + 1
            backstage.quality = min(MAX_QUALITY, quality)
    backstage.sell_in -= 1

Conjured Updater

We are done refactoring the codebase. It’s time to implement a new feature for the Conjured items.

• "Conjured" items degrade in Quality twice as fast as normal items

def conjured_updater(conjured: Item):
    if MIN_QUALITY <= conjured.quality  0 else conjured.quality - 4
        conjured.quality = max(quality, MIN_QUALITY)
    conjured.sell_in -= 1

Verification

We have added unit tests, refactored the legacy codebase and also implemented the new feature. Let’s run the tests to verify if we have been able to retain the behavior of the systems.

Hooray! All the five test cases passed!