DEV Community

Cover image for A Walk Through the Gilded Rose Kata — Pt 4: Discovering what GildedRose really is
Lomig
Lomig

Posted on • Updated on

A Walk Through the Gilded Rose Kata — Pt 4: Discovering what GildedRose really is

In the last part, we left GildedRose in a very awkward position.

  • It is not a huge mess anymore (Good!)
  • It is not a huge conditional mess anymore (Even better!)
  • It is not a huge procedural conditional mess anymore...

... or is it?
It looks more OO, but in reality, is a mimicry: I just camouflaged bits of procedural code in objects.

Unfortunately, in my real life:

  • lots of the OO code I see looks like what we had in part 1;
  • most of the OO code I see looks like what we first created with smaller methods in part 2;
  • some well-behaved kids try OO and get to those small objects we created in part 3;
  • ... and we are still in the middle of Faux-O. Oh, we did a HUGE step forward, it is just that we are not there yet.

But before continuing our Journey, a quite unrelated side-step, for you to understand why I did a part of what you will see in future code samples.

Good OO looks like Functional Programming

This is a little exaggerated, but not by much. Chaining messages, when done properly, can feel like composing functions.

You may have been told rules like "No more than 2 dots on a line".
I feel this rule stupid. The functional developper inside of my stomach shifts uneasily when hearing such sayings.
The heart is at the right place, I understand what the people mean, but I am not sure it means the same thing for them as it does for me.

When the returned object of each message is different, we should not go above 2. It would mean that we send messages to objects we should not be aware of in the first place.

But what about something like:

str = "a STRING is like a spring!"
str.downcase
   .gsub(/(s.ring)/, 'beautiful \1')
   .capitalize

# => A beautiful string is like a beautiful spring!
Enter fullscreen mode Exit fullscreen mode

Each method is called on the same object; it does not count as "three dots"!

I want all my classes to behave like String and allow me to chain calls as I see fit.

To that avail, a simple rule: "A public method which does not return a meaningful result should return self"

So you will see some self coming in the mix soon enough 😊



But let us go back to our main subject. What does GildedRose? It responds to 1 message, and that is it.

def update_quality
  @items.each do |item|
    case item.name
    when "Aged Brie"
      AgedBrie.new(item).update
    when "Backstage passes to a TAFKAL80ETC concert"
      Backstage.new(item).update
    when "Sulfuras, Hand of Ragnaros"
      Sulfuras.new(item).update
    else
      CommonItem.new(item).update
    end
  end

  self
end
Enter fullscreen mode Exit fullscreen mode

Let us see it from another angle: GildedRose creates some classes and propagates a message to those classes.

Oh. God. I fear GildedRose is in reality... a kind of Factory.

No, not the gross factories from the Java world: The pure, aesthetic factories promoted by Object Oriented Programming.

Now that we considered GildedRose that way, impossible to go back: we need to refine it and shape it like the factory it is.

From a standard class to a factory

@items is an Array of Item instances. I would prefer it to be an Array of CommonItem. My problem is: which child of CommonItem?
Well, it depends on the item name.

Let just code some helper for that!
(In reality, the code is already there, can you see it?)

private

def class_from(name:)
  case name
  when "Aged Brie"
    AgedBrie
  when "Backstage passes to a TAFKAL80ETC concert"
    Backstage
  when "Sulfuras, Hand of Ragnaros"
    Sulfuras
  else
    CommonItem
  end
end
Enter fullscreen mode Exit fullscreen mode

Given an Item, I can now instantiate the right object with a simple class_from(name: item.name).new!

We just can now refactor our code around this idea.

class GildedRose
  def initialize(items)
    # I do not store Item instances anymore, but instances of CommonItem
    @items = items.map { |item| class_from(name: item.name).new(item) }
  end

  def update_quality
    # item is now an instance of CommonItem.
    # it can just be sent the "update" message!
    @items.each { |item| item.update }

    self
  end

private

  def class_from(name:)
    case name
    when "Aged Brie"
      AgedBrie
    when "Backstage passes to a TAFKAL80ETC concert"
      Backstage
    when "Sulfuras, Hand of Ragnaros"
      Sulfuras
    else
      CommonItem
    end
  end
end
Enter fullscreen mode Exit fullscreen mode

I do not know for you, but for me, it starts to look really elegant. And simple.

But let us go a little further. The content of #class_from is a real pain. Come on, you know me by now: it is a dreaded conditional! Even worse: it's a configuration conditional 🤢

Luckily for us, we know perfectly well a data structure that can handle a simple key/value configuration: we shall use an Hash.

And here is what the (final?) code looks like:

class CommonItem
  attr_reader :item

  def initialize(item)
    @item = item
  end


  def update
    item.sell_in -= 1

    item.quality -= 1
    item.quality -= 1 if item.sell_in < 0

    limit_quality
    self
  end

  private

  def limit_quality
    return item.quality = 0 if item.quality < 0

    item.quality = 50 if item.quality > 50
  end
end
Enter fullscreen mode Exit fullscreen mode
class AgedBrie < CommonItem
  def update
    item.sell_in -= 1

    item.quality +=1
    item.quality +=1 if item.sell_in < 0

    limit_quality
    self
  end
end
Enter fullscreen mode Exit fullscreen mode
class Backstage < CommonItem
  def update
    item.sell_in -= 1
    item.quality = 0 and return self if item.sell_in < 0

    item.quality += 1
    item.quality += 1 if item.sell_in < 10
    item.quality += 1 if item.sell_in < 5

    limit_quality
    self
  end
end
Enter fullscreen mode Exit fullscreen mode
class Sulfuras < CommonItem
  def update
    self
  end
end
Enter fullscreen mode Exit fullscreen mode
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:)
    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
Enter fullscreen mode Exit fullscreen mode

Technically speaking, the refactoring is now finished.

  • We have a very lean class
  • Its responsibility is well defined
  • We can add a functionality very easily

The last part will deal with this new functionality, and a sprinkle of refactoring: this time, we will try to accommodate my tiny brain with quality of life changes.

Bye for now :)

Oldest comments (0)