How to solve the Gilded Rose Kata with duplication?

The Gilded Rose Kata from Emily Bache is one of the famous refactoring Kata. Let’s see how we can solve it in C# by adding more duplication.

Duplicated Gilded Roses generated by Microsoft Bing Image AI

This solution is inspired by Woody Zuill who has a GitHub repository with his refactored solution.

Prerequisites

To solve the Golden Rose Kata with this duplication method, we should have the following requirements met:

• A golden mastering test (or approval tests) that takes a lot of inputs and characterizes what the code is actually doing
• An IDE (here we will use Visual Studio)
• A code coverage tool (here Fine Code Coverage, an extension of Visual Studio)

The methodology

The principle of this refactoring method is the following:

1. Identify what to isolate
2. Use an if / else statement to duplicate the code
3. Launch the approval tests
4. Use the code coverage to remove the duplication
5. Repeat

Application of this methodology on the Gilded Rose kata

Identify what to isolate

In the case of the Gilded Rose kata it’s quite intuitive to have some code depending on the article names.

The aim at the end is to have something like this:

if (Items[i].Name == "Sulfuras, Hand of Ragnaros")
{
  // do someting
}
else if (Items[i].Name == "Aged Brie")
{
  // do something else
}
else if (Items[i].Name == "Backstage passes to a TAFKAL80ETC concert")
{
  // do something else
}
else
{
  // do something else
}

Use an if/else statement to duplicate the code

Once the part to isolate was identified, a simple if/else statement can be done, adding temporary a lot of duplication.

if (Items[i].Name == "Aged Brie")
{
  // Legacy code
}
else
{
  // Duplicated legacy code
}

Launch approval tests

Nothing much to tell, just check nothing was broken with this modification

Use code coverage to remove duplication

The code coverage will highlight in red lines that are not covered by the approval tests. It means these lines have no use for the new implementation to be iso with the legacy code. These lines can be simply removed.

Highlighted lines that could be removed

Looking closely, it’s quite clear the lines can be removed: line 19 is unreachable due to negation to enclosing if at line 17.

Why does it work?

The purpose of code coverage is initially to identify lines of code that are not tested. When we reach optimal coverage (close to 100%), we can be reassured that the code is sufficiently tested. The fact that an alternative code providing exactly the same functionalities contains uncovered lines gives them a nature of dead code.

Generalizing, here are the patterns that can be applied to kill the introduced duplication:

• Red covered lines: dead code that can be removed
• Condition marked as always false: dead code as well, remove the conditional with the enclosing code
• Condition marked as always true: unwrap the code inside the condition, but delete the useless condition
• An empty if or else: simply remove it

Once done, the “Aged Brie” conditional statement should look like this:

if (Items[i].Name == "Aged Brie")
{
    if (Items[i].Quality < 50)
    {
        Items[i].Quality = Items[i].Quality + 1;
    }

    Items[i].SellIn = Items[i].SellIn - 1;

    if (Items[i].SellIn < 0)
    {
        if (Items[i].Quality < 50)
        {
            Items[i].Quality = Items[i].Quality + 1;
        }
    }
}

After that, launch the approval tests again to see if nothing has been broken. If the code coverage is showing only green lines, it means the code cannot be improved yet.

Repeat

Chose the other article name (here “Sulfuras, Hand of Ragnaros” for instance) and go over the process again.

Repeat the process until there are no articles to process with.

The follwing other if statements should look like this:

// if statement on Aged Brie
else if (Items[i].Name == "Backstage passes to a TAFKAL80ETC concert")
{
    if (Items[i].Quality < 50)
    {
        Items[i].Quality = Items[i].Quality + 1;

        if (Items[i].SellIn < 11)
        {
            if (Items[i].Quality < 50)
            {
                Items[i].Quality = Items[i].Quality + 1;
            }
        }

        if (Items[i].SellIn < 6)
        {
            if (Items[i].Quality < 50)
            {
                Items[i].Quality = Items[i].Quality + 1;
            }
        }
    }

    Items[i].SellIn = Items[i].SellIn - 1;

    if (Items[i].SellIn < 0)
    {
        Items[i].Quality = Items[i].Quality - Items[i].Quality;
    }
}

// other if statements
else if (Items[i].Name == "Sulfuras, Hand of Ragnaros")
{
    // does nothing
}
else // default case
{
    if (Items[i].Quality > 0)
    {
        Items[i].Quality = Items[i].Quality - 1;
    }

    Items[i].SellIn = Items[i].SellIn - 1;

    if (Items[i].SellIn < 0)
    {
        if (Items[i].Quality > 0)
        {
            Items[i].Quality = Items[i].Quality - 1;
        }
    }
}

More refactoring

This first refactoring was necessary to break the cyclomatic complexity that was in the legacy code.

It’s possible to refactor more by creating some methods. In the following result, DecreaseQuality and IncreaseQuality methods have been created, also the SellIn has been duplicated in each condition to be able to remove the duplication. Let’s see how much the code was reduced:

using System.Collections.Generic;

namespace csharp
{
    public class GildedRose
    {
        private const string Sulfuras = "Sulfuras, Hand of Ragnaros";
        private const string AgedBrie = "Aged Brie";
        private const string Backstage = "Backstage passes to a TAFKAL80ETC concert";

        readonly IList<Item> Items;
        public GildedRose(IList<Item> Items)
        {
            this.Items = Items;
        }

        public void UpdateQuality()
        {
            foreach (var item in Items)
            {
                UpdateQuality(item);
            }
        }

        private static void UpdateQuality(Item item)
        {
            if (item.Name == Sulfuras) return;

            item.SellIn--;

            if (item.Name == AgedBrie)
            {
                UpdateAgedBrieQuality(item);
                return;
            }

            if (item.Name == Backstage)
            {
                UpdateBackstageQuality(item);
                return;
            }

            UpdateDefaultQuality(item);
        }

        private static void UpdateAgedBrieQuality(Item item)
        {
            IncreaseQuality(item);
            if (item.SellIn < 0) IncreaseQuality(item);
        }

        private static void UpdateBackstageQuality(Item item)
        {
            IncreaseQuality(item);
            if (item.SellIn < 10) IncreaseQuality(item);
            if (item.SellIn < 5) IncreaseQuality(item);
            if (item.SellIn < 0) item.Quality -= item.Quality;
        }

        private static void UpdateDefaultQuality(Item item)
        {
            DecreaseQuality(item);
            if (item.SellIn < 0) DecreaseQuality(item);
        }

        private static void DecreaseQuality(Item item)
        {
            if (item.Quality > 0) item.Quality--;
        }

        private static void IncreaseQuality(Item item)
        {
            if (item.Quality < 50) item.Quality++;
        }
    }
}

In this refactored example, const were used. Enumaration could be also used as improvement. In all cases, this solution is much easier to understand that the original version.

To go further

Once the code is well refactored, there are some things that can be done.

First, the golden mastering test is only here to document the actual code and help us with the refactoring. Once it’s done, it can be removed.

Instead, some unit tests can be added to further uses.

Here are the unit tests that can be written, based on the refactored code comprehension:

• Tests for Sulfura cases

[Test]
public void ShouldSulfurasQualityAndSellInNeverChange()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.Sulfuras,
        SellIn = 0,
        Quality = 0 } };
    GildedRose app = new GildedRose(Items);
    for (int i = 0; i < 16; i++)
    {
        app.UpdateQuality();
    }

    Assert.AreEqual(0, Items[0].SellIn);
    Assert.AreEqual(0, Items[0].Quality);
}

• Tests for Aged Brie cases

[Test]
public void ShouldAgedBrieSellInDecreaseDaily()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.AgedBrie,
        SellIn = 10,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(9, Items[0].SellIn);

    app.UpdateQuality();

    Assert.AreEqual(8, Items[0].SellIn);
}

[Test]
public void ShouldAgedBrieQualityNeverExceed50()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.AgedBrie,
        SellIn = 10,
        Quality = 49 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(50, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(50, Items[0].Quality);
}

[Test]
public void ShouldAgedBrieQualityIncreaseDaily()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.AgedBrie,
        SellIn = 2,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(21, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(22, Items[0].Quality);
}

[Test]
public void ShouldAgedBrieQualityIncreaseTwiceAsFastWhenSellInIsElapsed()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.AgedBrie,
        SellIn = -1,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(22, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(24, Items[0].Quality);
}

• Tests for Backstage cases

[Test]
public void ShouldBackstageQualityIncreaseDaily()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.Backstage,
        SellIn = 12,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(21, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(22, Items[0].Quality);
}

[Test]
public void ShouldBackstageQualityIncreaseTwiceAsFastWhenSellInIsBelowTenDays()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.Backstage,
        SellIn = 7,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(22, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(24, Items[0].Quality);
}

[Test]
public void ShouldBackstageQualityIncreaseThriceAsFastWhenSellInIsBelowFiveDays()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.Backstage,
        SellIn = 2,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(23, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(26, Items[0].Quality);
}

[Test]
public void ShouldBackstageQualityBeZeroWhenSellInIsElapsed()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = GildedRose.Backstage,
        SellIn = 0,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(0, Items[0].Quality);
}

• Tests for default cases

[Test]
public void ShouldItemNameNeverChange()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = "Default Item",
        SellIn = 10,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual("Default Item", Items[0].Name);
}

[Test]
public void ShouldItemQualityDecreaseDaily()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = "Default Item",
        SellIn = 2,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(19, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(18, Items[0].Quality);
}

[Test]
public void ShouldItemQualityDecreaseTwiceAsFastWhenSellInIsElaped()
{
    IList<Item> Items = new List<Item> { new Item {
        Name = "Default Item",
        SellIn = -1,
        Quality = 20 } };
    GildedRose app = new GildedRose(Items);
    app.UpdateQuality();

    Assert.AreEqual(18, Items[0].Quality);

    app.UpdateQuality();

    Assert.AreEqual(16, Items[0].Quality);
}

All these tests enables 100% code coverage on the GildedRose class which is wanted. The key difference with the Golden master test is that these unit tests are documenting the code with the specifications.

If you read my previous article on mutation testing, you know that some improval can be done to get the tests more robust.

Only using IDE automatic refactoring tools, and code coverage linked with a golden master test, the Gilded Rose Kata was quite easy to do. Once the cyclomatic complexity has been resolved, more refactoring can be done. The golden master should not stay longer in the code and should be replaced by actual unit tests.

If you liked this article feel free to clap it, comment it or share it.