A Walk Through the Gilded Rose Kata — Pt 5: Open to extension, closed to modification
Tags: ruby, refactorit, oop
Published: 2021-04-12 19:55:58

Consider the shape of our `GildedRose` application now, after 4 long and painful posts with me as your guest.

We traveled far, and now we have a factory using some `CommonItem` instances through polymorphism. Now we can concentrate on what Allison really wanted us to do: Add the possibility to deal with conjured items.

I can now implement such a functionality... nearly without editing the current codebase.

# Adding Conjured Items

I do not like this task that much, because it breaks the nice symmetry we had: We don't want to apply the rule to an exact item name, but to a match.

If it was a complete string, I would not have to change the code at all, but hey, when life gives you lemons, you squeeze them in the eyes of those stupid `else` keywords.

Let us start with the said rules: A conjured item is similar to a common item with twice as much a decay. Fine.

```ruby
class ConjuredItem < CommonItem
  def update
    item.sell_in -= 1

    item.quality -= 2
    item.quality -= 2 if item.sell_in < 0
    
    limit_quality
    self
  end
end
```

Now, we need our factory (`GildedRose`, that is to say) to create this `ConjuredItem` when needed.

```ruby
def class_from(name:)
  return ConjuredItem if name.downcase.start_with?("conjured ")

  ITEM_CLASSES[name] || CommonItem
end
```

Done.

Yup, done. How cool and how simple is that?
This is the meaning of "Open to extension, closed to modification": We can add new behaviour without changing the existing code.


# Quality of life: How I like to see Ruby code

We will complete the refactoring with my personal touch over this code. I like to handle concepts more than data.

What is `item.sell_in -= 1`? Right now, I kind of remember, but in two weeks, I am not so sure. Well, the concept is `ages_one_day`, and in 2 years, I will still understand what that means.

I do not like those `item.quality +=1` neither, especially when _afterwards_ I have to limit them.
I would prefer to `increase_quality_up_to_fifty` that would do both.

As soon as I stumble upon a concept, I shall extract it. Our `AgedBrie` will soon look like this:

```ruby
class AgedBrie < CommonItem
  def update
    ages_one_day
    increase_quality_up_to_fifty
    increase_quality_up_to_fifty if expired?

    self
  end
end
```

And that's my last trick: This increase_quality repeated twice is quite strange. But we live in a wonderful world, the Ruby world: `alias increase_quality_up_to_fifty increase_quality_up_to_fifty_again` will allow me to write something more elegant as:

```ruby
class AgedBrie < CommonItem
  def update
    ages_one_day
    increase_quality_up_to_fifty
    increase_quality_up_to_fifty_again if expired?

    self
  end
end
```

# Final Code

I do not have a conclusion: as I said, I do consider that my approach has value, but there may be other interesting ways of solving this kata 😊

In any case, here is the final code (every class being in its own file):

```ruby
require_relative("item")
require_relative("common_item")
require_relative("aged_brie")
require_relative("backstage")
require_relative("sulfuras")
require_relative("conjured_item")

class GildedRose
  ITEM_CLASSES = {
    "Aged Brie"                                 => AgedBrie,
    "Sulfuras, Hand of Ragnaros"                => Sulfuras,
    "Backstage passes to a TAFKAL80ETC concert" => Backstage
  }

  def initialize(items)
    @items = items.map { |item| class_from(name: item.name).new(item) }
  end

  def update_quality
    @items.each { |item| item.update }

    self
  end

  private
  
  def class_from(name:)
    return ConjuredItem if name.downcase.start_with?("conjured ")

    ITEM_CLASSES[name] || CommonItem
  end
end
```
```ruby
class Item
  attr_accessor :name, :sell_in, :quality

  def initialize(name, sell_in, quality)
    @name = name
    @sell_in = sell_in
    @quality = quality
  end

  def to_s()
    "#{@name}, #{@sell_in}, #{@quality}"
  end
end
```
```ruby
class CommonItem
  attr_reader :item

  def initialize(item)
    @item = item
  end

  def update
    ages_one_day
    decrease_quality_down_to_zero
    decrease_quality_down_to_zero_again if expired?
    
    self
  end

  private

  def ages_one_day
    item.sell_in -= 1
  end

  def expired?
    item.sell_in < 0
  end

  def decrease_quality_down_to_zero(time: 1)
    time.times { item.quality -= 1 if item.quality > 0 }
  end
  alias decrease_quality_down_to_zero_again decrease_quality_down_to_zero

  def increase_quality_up_to_fifty
    item.quality += 1 if item.quality < 50
  end
  alias increase_quality_up_to_fifty_again increase_quality_up_to_fifty
end
```
```ruby
require_relative("common_item")

class AgedBrie < CommonItem
  def update
    ages_one_day
    increase_quality_up_to_fifty
    increase_quality_up_to_fifty_again if expired?

    self
  end
end
```
```ruby
require_relative("common_item")

class Backstage < CommonItem
  def update
    ages_one_day
    item.quality = 0 and return self if expired?

    increase_quality_up_to_fifty
    increase_quality_up_to_fifty_again if item.sell_in < 10
    increase_quality_up_to_fifty_again if item.sell_in < 5

    self
  end
end
```
```ruby
require_relative("common_item")

class Sulfuras < CommonItem
  def update
    self
  end
end
```
```ruby
require_relative("common_item")

class ConjuredItem < CommonItem
  def update
    ages_one_day
    decrease_quality_down_to_zero(time: 2)
    decrease_quality_down_to_zero_again(time: 2) if expired?
    
    self
  end
end
```