gilded-rose.md

title	author	date	css	highlightTheme
Gilded rose kata	Zeger Hendrikse	2023-09-29	css/neon.css	github-dark

The Gilded Rose kata

 

 

 

 

by Zeger Hendrikse

 

 

---

• Explain various refactoring techniques
  • Creating a golden master / baseline
  • Getting acquainted with mutation testing
  • Replace conditional with polymorphism
  • Refactor Negate If
  • Extract method
• Learn to refactor in small steps
• The one and only useful use of code coverage 😉

 

 

 

---

Martin Fowler: good programmers

A fool can write code that a computer can understand. Good programmers write code that humans can understand.

---

Gilded Rose kata

System updates items daily

• SellIn = number of days left to sell the item
• Quality = how valuable the item is
• At the end of each day both values are lowered

---

Task: add a new category of items

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

---

<iframe frameborder="0" width="100%" height="700px" src="https://replit.com/@zwh/tdd?lite=false"></iframe>

---

😱 How do I even begin 😱

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

---

Start with approval tests

import unittest
from approvaltests.combination_approvals import verify_all_combinations
from gilded_rose import GildedRose, Item

class GildedRoseTest(unittest.TestCase):
  def test_add_combinatorial(self):
    names = ["foo"]
    sellIns = [0]
    qualities = [0]
    verify_all_combinations(self.do_update_quality, [names, sellIns, qualities])

  def do_update_quality(self, name: str, sellIn: int, quality:int) -> str:
    items = [Item(name, sellIn, quality)]
    app = GildedRose(items)
    app.update_quality()
    return app.items[0]

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

---

Start with all the names

• “Sulfuras, Hand of Ragnaros”
• “Aged Brie”
• “Backstage passes to a TAFKAL80ETC concert”

---

Quality and Sell In days

• Quality 50
• SellIn 11
• SellIn -1
• Quality 49

---

Edge cases & mutation testing

• What if we change the "6" → "7" in line 19?
• Add "6" to sell in
• What if we change the "50" → "49" in line 17?
• Add "48" to quality
• What if we change the "50" → "49" in line 20?
• Etc...

---

End result of approval tests

import unittest
from approvaltests.combination_approvals import verify_all_combinations
from gilded_rose import GildedRose, Item

class GildedRoseTest(unittest.TestCase):
  def test_add_combinatorial(self):
    names = ["foo", "Aged Brie", "Sulfuras, Hand of Ragnaros", "Backstage passes to a TAFKAL80ETC concert"]
    sellIns = [-1, 2, 6, 0, 11, 7]
    qualities = [0, 48, 49, 50, 47]
    verify_all_combinations(self.do_update_quality, [names, sellIns, qualities])

  def do_update_quality(self, name: str, sellIn: int, quality:int) -> str:
    items = [Item(name, sellIn, quality)]
    app = GildedRose(items)
    app.update_quality()
    return app.items[0]

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

---

Extract method

• Turn the fragment into a method whose name explains the purpose of the method

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

---

Refactor Negate If

• Eliminate negate and "flip" the conditional

if item.name == "Aged Brie" or item.name == "Backstage passes to a TAFKAL80ETC concert":

---

Lift-Up Conditional — step 1

• Create temporary foo(self, item) with all logic

def update_item_quality(self, item):
  self.foo(item)

def foo(self, item):
  if item.name == "Aged Brie" or item.name == "Backstage passes to a TAFKAL80ETC concert":
  ...  

---

Lift-Up Conditional — step 2

• Add conditional

def update_item_quality(self, item):
  if item.name == "Aged Brie":
    self.foo(item)
  else: 
    self.foo(item)

---

Lift-Up Conditional — step 3

1. Inline foo(), run and inspect coverage!
2. Remove dead code
3. Inspect coverage of conditionals
   • Hover over to see more details
   • Remove dead branches
4. Take out "Agred Brie" from else-clause

---

Rinse and repeat

The same for the else-branch of "Aged Brie"

else:
  if item.name == "Backstage passes to a TAFKAL80ETC concert":
    self.foo(item)
  else:
    self.foo(item)

---

Rinse and repeat

Do the same for the else-branch of "Backstage passes to a TAFKAL80ETC concert"

else:
  if item.name == "Sulfuras, Hand of Ragnaros":
    self.foo(item)
  else:
    self.foo(item)

---

Rearrange item type conditional

if item.name == "Aged Brie":
  ...
elif item.name == "Backstage passes to a TAFKAL80ETC concert":
  ...
else:
  if item.name != "Sulfuras, Hand of Ragnaros":
    ...

---

Refactor Negate If

if item.name == "Aged Brie":
  ...
elif item.name == "Backstage passes to a TAFKAL80ETC concert":
  ...
elif item.name == "Sulfuras, Hand of Ragnaros":
  pass
else:
  ...

---

Replace conditional with polymorphism

---

Introduce Aged Brie class

class AgedBrieItem(Item):
  def update_quality(self) -> None:
    if self.quality < 50:
      self.quality += 1
    self.sell_in -= 1
    if self.sell_in < 0:
      if self.quality < 50:
        self.quality += 1

Test case

def do_update_quality(self, name: str, sellIn: int, quality: int) -> str:
  if name == "Aged Brie":
    items = [AgedBrieItem(name, sellIn, quality)]
  else:
    items = [Item(name, sellIn, quality)]

---

The other subclasses

class BackstagePassItem(Item):
  def update_quality(self) -> None:
    if self.quality < 50:
      self.quality += 1
      if self.sell_in < 11:
        if self.quality < 50:
          self.quality += 1
      if self.sell_in < 6:
        if self.quality < 50:
          self.quality += 1

    self.sell_in -= 1
    if self.sell_in < 0:
      self.quality = 0

---

The other subclasses

class SulfurasItem(Item):
  def update_quality(self) -> None:
    pass

---

The base class

class Item:
  ...

  def update_quality(self) -> None:
    if self.quality > 0:
      self.quality -= 1
    self.sell_in -= 1

---

Set name in constructor

class BackstagePassItem(Item):
  def __init__(self, sell_in, quality):
    super().__init__("Backstage passes to a TAFKAL80ETC concert", sell_in, quality)

---

Primitive obsession

class Quality:
  def __init__(self, quality: int):
    self.value = quality

  def increase(self):
    if self.value < 50:
      self.value += 1

  def decrease(self):
    if self.value > 0:
      self.value -= 1

with

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

  def __repr__(self):
    return "%s, %s, %s" % (self.name, self.sell_in, self.quality.value)

---

Retrospective

• Mocks, stubs, fakes, spies, ...
• The Clean Architecture: how to cope with dependencies on external systems
• London school / Detroit school of TDD
• Developer tests his own code: the nightmare of every auditor!