• By Christopher Hoffman
• Posted in blog, codekata, software, tutorial
• On June 13, 2016
• No Comments.

As a software developer, I am a big fan of code katas. For those who aren’t familiar with katas, they come from a martial arts term for the series of forms which are repeated over and over as a way to practice. In software development, a code kata is similar; they are small exercises which allow you to practice your coding skills. Generally they are simple problems which allow you to think not about the solution, but rather the act of solving them. They allow you to deliberately practice the art of programming.

An example of a martial arts kata

In this post I’m going to talk about the Gilded Rose kata, a kata about unit testing, refactoring, and working in legacy systems. We all have to do it sometime, you get put on a project which has been in existence for longer than computers have been around, which was developed by a single goblin still lurking in the basement. Seriously, this kata has to do with that goblin, which might come upstairs and attack you if you touch the wrong part of the code.

Every kata has a prompt, I’ll give a short rundown below, but if you want to see the full prompt you can pull the project I started with from Terry Hughes and NotMyself’s github project. You can also find my solution on my own github, complete with a few different methods of solving the problems. Feel free to pull those projects and try it out yourself!

The Prompt

You are hired by a local inn, the Gilded Rose, to make some changes to their inventory software. It was written by an adventurer who hasn’t been seen in a while, and they need a new type of item added to the system. However, the item class is owned by a goblin in the corner who will not be happy if you touch his code.

Thus, the inventory program has evolved to take account for the many various item types, each with their own rules for how the item quality degrades over time. You are to add another new item type to their inventory, with its own special rules.

Again, please check out the README of the project for the full prompt. The core of the current program centers around the UpdateQuality function. For each item in the inventory, it runs through this giant function with many nested if-statements, updating the item’s Quality and SellIn value. It is painful to look at, and continues for 76 lines mixing together the business logic for all the different items. Just take a look at it below:

37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67

       public void UpdateQuality()
        {
            for (var i = 0; i < Items.Count; i++)
            {
 
 
                if (Items[i].Name != "Aged Brie" && Items[i].Name != "Backstage passes to a TAFKAL80ETC concert")
                {
                    if (Items[i].Quality > 0)
                    {
                        if (Items[i].Name != "Sulfuras, Hand of Ragnaros")
                        {
                            Items[i].Quality = Items[i].Quality - 1;
                        }
                    }
                }
                else
                {
                    if (Items[i].Quality < 50)
                    {
                        Items[i].Quality = Items[i].Quality + 1;
 
                        if (Items[i].Name == "Backstage passes to a TAFKAL80ETC concert")
                        {
                            if (Items[i].SellIn < 11)
                            {
                                if (Items[i].Quality < 50)
                                {
                                    Items[i].Quality = Items[i].Quality + 1;
                                }
                            }

And it goes on and on. I’m sure we’ve seen functions like this in our own production code. So now, how do we go about implementing our new feature? Sure, we could follow the pattern that is already there, stuffing in new if statements checking for our new item. In this environment though, it is easy to change the wrong if statement, and break one of the existing items. And we wouldn’t know about it until the inn owner is yelling at us. No, first, we should probably write some tests to document the system as it stands currently. Then, just maybe we can make changes without the goblin hunting us down.

Writing the Unit Tests

So I added a GildedRose.Tests program to the solution, and started writing tests. Now, because the business logic that I was interested lived in the main program, the only way to currently test them was to create a brand new instance of the program each time, and run through the program with the item that I was testing. A majority of the tests follow the following form:

22
23
24
25
26
27
28
29
30
31
32
33
34

        [TestMethod()]
        public void NormalItem_DegradeQualityByOne_ItemHasPositiveSellIn()
        {
            int originalQuality = 20;
            program.Items = new List<item> { new Item { Name = "+5 Dexterity Vest", SellIn = 10, Quality = originalQuality } };
 
            program.UpdateQuality();
 
            Item alteredItem = program.Items[0];
 
            Assert.AreEqual(originalQuality - 1, alteredItem.Quality);
        }
</item>

Nothing too crazy there. Create a new program, add the item under test, run the programs UpdateQuality method, and verify the outcome. Easy, and it allows us to document exactly what the program does with each of the items we are interested in. Luckily, this program has few dependencies, and the item class is pretty basic, so we didn’t have to worry about mocking objects inside our tests.

Fifteen unit tests later we have all the current code being unit tested. Perfect! We are ready to jump into the actual code and play around. If we change something, we will be alerted that we actually broke something! We can put the fear of refactoring behind us, and actually get our hands dirty.

The Plan

So how can we clean this up, and in a way which doesn’t make the goblin in the corner mad for touching his item class. I tried a few different options (you can see the different branches on my github page), but the one I will discuss here is to create a series of new objects, which I call Update Strategies for each set of business logic. First, I defined an interface, called the UpdateStrategyInterface, which has a single method, Update. The idea would be, I would create the appropriate strategy for each item, and have the strategy itself update the item.

Pseudocode:
 
public void UpdateQuality()
 
   foreach item in Items
      UpdateStrategyInterface strat = getAppropriateStrategy(item)
      strat.Update(item)

Enough Talk, lets do it!

So, first is to create the interface. This is a simple interface, just a single method which takes in an Item. It’ll perform the update directly on the item itself, so there is no return type.

22
23
24
25
26
27
28

namespace GildedRose.Console
{
    interface UpdateStrategyInterface
    {
        void Update(Item item);
    }
}

Next, we should tackle how the correct strategy will be created. For this, I chose to implement the Factory pattern. To quote the Gang of Four (Erich Gamma, Richard Helm, Ralph Johnson and John Vlissides), authors of the influencial book Design Patterns: Elements of Reusable Object-Oriented Software:

Factory Pattern: Define an interface for creating an object, but let subclasses decide which class to instantiate. The Factory method lets a class defer instantiation it uses to subclasses

To break it down, you move the decision of which object to use into the factory object, which then decides which object to instantiate, and returns an interface common to all of those possibilities. That way, the code which calls the factory doesn’t care about which object it receives, it only cares that it implements the interface it expects. This is perfect for our use case. So lets implement the UpdateStrategyFactory:

3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28

    class UpdateStrategyFactory
    {
        public static UpdateStrategyInterface Create(Item item)
        {
            if (item.Name.Contains("Sulfuras"))
            {
                return new SulfurasUpdateStrategy();
            }
            else if (item.Name.Contains("Aged Brie"))
            {
                return new AgedBrieUpdateStrategy();
            }
            else if (item.Name.Contains("Backstage pass"))
            {
                return new BackstagePassesUpdateStrategy();
            }
            else if (item.Name.Contains("Conjured"))
            {
                return new ConjuredUpdateStrategy();
            }
            else
            {
                return new StandardUpdateStrategy();
            }
        }
    }

Here, we put the logic of which strategy to create, and return back the interface we created. We check the incoming item for the name, and based on that we create the appropriate UpdateStrategy. Now that we have the core of the pattern laid down, we can start moving the business logic out from the monsterous UpdateQuality() method and into our new classes.

Here is an example of the StandardUpdateStrategy. You can see it is much easier to see what is going on in this function. There is no crazy nested if-statements to check which type of item you are dealing with. It is easy to read as a developer, and easy to test. Much, much better.

3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

class StandardUpdateStrategy : UpdateStrategyInterface
    {
        public void Update(Item item)
        {
 
            if (item.Quality > 0)
            {
                item.Quality -= 1;
            }
 
            // Decrease SellIn Date
            item.SellIn -= 1;
 
            if (item.SellIn < 0)
            {
                if (item.Quality > 0)
                {
                    item.Quality -= 1;
                }
            }
        }
    }

And, we don’t have to implement all of these strategies at once. I extracted the old UpdateQuality method into a new function, and had the Factory return nulls for Strategies we haven’t implemented yet. Then, in the Program.CS below, we either call the strategies Update() method if it exists, or we pass the item through to the old method. This way we can easily move one item at a time, adding it to the UpdateStrategyFactory we needed. And, all our unit tests are still passing, meaning we can release this code at any time and not break our customers!

37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61

        public void UpdateQuality()
        {
            for (var i = 0; i < Items.Count; i++)
            {
                UpdateStrategyInterface strat = UpdateStrategyFactory.Create(Items[i]);
 
                if (strat != null)
                {
                    strat.Update(Items[i]);
                }
                else
                {
                    OldMethod(i);
                }
            }
        }
 
        private void OldMethod(int i)
        {
 
            if (Items[i].Name != "Aged Brie" && Items[i].Name != "Backstage passes to a TAFKAL80ETC concert")
            {
                if (Items[i].Quality > 0)
                {
...

We are getting close to being done with our refactor! Once all the new UpdateStrategies have been created, we can then remove that old UpdateQuality method once and for all. All of our logic has been moved to our new classes, nice and separated like they want to be. Hurrah!

The rest of the kata is almost trivial at this point. We can create our new ConjuredItemUpdateStrategy, and add it to the UpdateStrategyFactory that we created.

99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120

 +    class ConjuredUpdateStrategy : UpdateStrategyInterface
 +    {
 +        public void Update(Item item)
 +        {
 +
 +            if (item.Quality > 0)
 +            {
 +                item.Quality = item.Quality - 2;
 +            }
 +
 +            // Decrease SellIn Date
 +            item.SellIn -= 1;
 +
 +            if (item.SellIn < 0)
 +            {
 +                if (item.Quality > 0)
 +                {
 +                    item.Quality = item.Quality - 2;
 +                }
 +            }
 +        }
 +    }

22
23
24
25
26
27
28
29
30
31

              {
                  return new BackstagePassesUpdateStrategy();
              }
 +            else if (item.Name.Contains("Conjured"))
 +            {
 +                return new ConjuredUpdateStrategy();
 +            }
              else
              {
                  return new StandardUpdateStrategy();

Throw in a few unit tests to verify that our business logic was implemented correctly, and we are finished!

Retrospective

This is a great kata for showing how valuable effective unit tests can be in a legacy system. Those tests saved me more than a few times from making simple mistakes when refactoring, and finding those mistakes outside of unit tests would be extremely costly. It also allowed me to explore the factory design pattern, and try out a new technique. I would highly recommend you pull this project from NotMyself’s github and give it a try yourself!

When you have completed it, I would love to see how you solved it! Post links to your solution in the comments below, I can’t wait to learn from you all!

Now, grab a mead from the inn, you deserve it.
-Christopher Hoffman