A Walk Through the Gilded Rose Kata — Pt 5: Open to extension, closed to modification

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.

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.

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:

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:

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):

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

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

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

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

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

require_relative("common_item")

class Sulfuras < CommonItem
  def update
    self
  end
end

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