GildedRose Refactoring in Ruby

Muratatak
8 min readFeb 8, 2023

There is dirty code and someone asked you to refactor it.

Where will I start?
What is the aim of refactoring code?
Should I separate some classes and methods?

I am gonna not mention the whole refactoring concept here. I will go by using an example. We will refactor step by step through the dirty code.

Refactor consists of improving the internal structure of an existing program’s source code, while preserving its external behavior.

I am gonna share directly the dirty GildedRose class for now.

require 'test/unit'

class GildedRose

attr_reader :name, :quality, :days_remaining

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

def tick

if @name != 'Aged Brie' && @name == 'Backstage passes to a TAFKAL80ETC concert'
if @quality > 0
if @name != 'Sulfuras, Hand of Ragnaros'
@quality -= 1
end
end
else
if @quality < 50
@quality += 1
if @name == 'Backstage passes to a TAFKAL80ETC concert'
if @days_remaining < 11
if @quality < 50
@quality += 1
end
end
if @days_remaining < 6
if @quality < 50
@quality += 1
end
end
end
end
end

if @name != 'Sulfuras, Hand of Ragnaros'
@days_remaining -= 1
end

if @days_remaining < 0
if @name != 'Aged Brie'
if @name != 'Backstage passes to a TAFKAL80ETC concert'
if @quality > 0
if @name != 'Sulfuras, Hand of Ragnaros'
@quality -= 1
end
end
else
@quality = @quality - @quality
end
end
else
if @quality < 50
@quality += 1
end
end
end
end


class GildedRoseTest<Test::Unit::TestCase

def test_normal_item_before_sell_date
item = GildedRose.new('normal', 10, 5)
item.tick
assert_equal 12, item.quality
assert_equal 4, item.days_remaining
end

def test_normal_item_on_sell_date
item = GildedRose.new('normal', 10, 0)
item.tick
assert_equal 10, item.quality
assert_equal -1, item.days_remaining
end


def test_normal_item_on_zero_date
item = GildedRose.new('normal', 0, 5)
item.tick
assert_equal 2, item.quality
assert_equal 4, item.days_remaining
end

def test_brie_item_before_sell_date
item = GildedRose.new('Aged Brie', 10, 5)
item.tick
assert_equal 12, item.quality
assert_equal 4, item.days_remaining
end

end

You can see everything placed in just one class. We will start to refactor the ‘tick’ method first. As seen, there is a name parameter and we will separate if cases and we can generate some methods using ‘name’ values.

Refactor Step 1 (# separate small methods)

#seperate small methods

require 'test/unit'

class GildedRose

attr_reader :name, :quality, :days_remaining

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

def tick
case @name
when 'normal'
return normal_tick
when 'Aged Brie'
return brie_tick
when 'Sulfuras, Hand of Ragnaros'
return sulfuras_tick
end
end

def normal_tick
@days_remaining -= 1
return @quality += 2 if @quality == 0
@quality += 2
@quality -= 2 if @days_remaining <= 0
end

def brie_tick
@days_remaining -= 1
return @quality += 2 if @quality > 50
@quality += 2
@quality -= 2 if @days_remaining <= -1
end

def sulfuras_tick
#some calculation
end

end

Refactoring Step 2 (# separate small objects)

We generate some methods that cover the ‘tick’ method. And we used case block. But still, everything depends on just one class and we can see still some duplicate calculations.

We need to reach the SOLID principle of at least 3 points :

S — Single Responsibility Principle (every module, class, or function in a computer program should have responsibility over a single part of that program’s functionality)
O — Open / Closed Principle — (software entities (classes, modules, methods, etc.) should be open for extension, but closed for modification)
L — Liskov Substitution Principle (objects of a superclass shall be replaceable with objects of its subclasses without breaking the application)

#seperate small objects

class GildedRose

attr_reader :name, :quality, :days_remaining

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

def quality
return item.quality if item
@quality
end

def days_remaining
return item.days_remaining if item
@days_remaining
end

def tick
case @name
when 'normal'
return normal_tick
when 'Aged Brie'
return brie_tick
when 'Sulfuras, Hand of Ragnaros'
return sulfuras_tick
end
end

def normal_tick
@item = Normal.new(@quality, @days_remaining)
@item.tick
end

def brie_tick
@item = Normal.new(@quality, @days_remaining)
@item.tick
end


def sulfuras_tick
@item = Sulfuras.new(@quality, @days_remaining)
@item.tick
end

end

class Normal

attr_reader :quality, :days_remaining

def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
@days_remaining -= 1
return @quality += 2 if @quality == 0
@quality += 2
@quality -= 2 if @days_remaining <= 0
end
end


class Brie
attr_reader :quality, :days_remaining

def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
@days_remaining -= 1
return @quality += 2 if @quality > 50
@quality += 2
@quality -= 2 if @days_remaining <= -1
end
end


class Sulfuras
end

Refactoring Step 3 — (# separate small objects)

We generated some classes depending on ‘name’ attributes. Will need to change the ‘name’ method.

#seperate small objects

class GildedRose

attr_reader :name, :quality, :days_remaining

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

def quality
return item.quality if item
@quality
end

def days_remaining
return item.days_remaining if item
@days_remaining
end

def tick
case @name
when 'normal'
@item = Normal.new(@quality, @days_remaining)
@item.tick
when 'Aged Brie'
@item = Brie.new(@quality, @days_remaining)
@item.tick
when 'Sulfuras, Hand of Ragnaros'
@item = Sulfuras.new(@quality, @days_remaining)
@item.tick
end
end

class Normal

attr_reader :quality, :days_remaining

def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
@days_remaining -= 1
return @quality += 2 if @quality == 0
@quality += 2
@quality -= 2 if @days_remaining <= 0
end
end


class Brie
attr_reader :quality, :days_remaining

def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
@days_remaining -= 1
return @quality += 2 if @quality > 50
@quality += 2
@quality -= 2 if @days_remaining <= -1
end
end


class Sulfuras
end


end

Refactoring Step 4 (#abstraction of duplication)

Still a lot of duplicates in the tick method and initialize parts. We can see our attributes name are the same as the method name. There is a clue here. We can use the name attr and we can create a subclass using directly the ‘name’ attr. And we can do it in the initialize.

class GildedRose

attr_reader :item

def initialize(name, quality, days_remaining)
@item = klass_for(name).new(quality, days_remaining)
end

def quality
item.quality
end

def days_remaining
item.days_remaining
end

def klass_for(name)
case name
when 'normal'
Normal
when 'Aged Brie'
Brie
when 'Sulfuras, Hand of Ragnaros'
Sulfuras
end
end

Refactoring Step 5 (#Factory)

Go back to the test.

class GildedRoseTest<Test::Unit::TestCase

def test_normal_item_before_sell_date
item = GildedRose.new('normal', 10, 5)
item.tick
assert_equal 12, item.quality
assert_equal 4, item.days_remaining
end

in the test, we generate a GildedRose instance.
And where is our ‘tick’ method right now? Inside the subclasses. We can directly call the ‘tick’ method from the main class and this method will call the subclass ‘tick’ method.

class GildedRose

attr_reader :item

def initialize(name, quality, days_remaining)
klass_for(name).new(quality, days_remaining)
end

def tick
@item.tick
end

def quality
item.quality
end

def days_remaining
item.days_remaining
end

def klass_for(name)
case name
when 'normal'
Normal
when 'Aged Brie'
Brie
when 'Sulfuras, Hand of Ragnaros'
Sulfuras
end
end

The ‘item’ will be a GoldedRose instance, like a ‘Brie’ class and we call using the ‘tick’ method for subclass ‘tick’ method.

Refactoring Step 6 — ( #still duplicate)

We have still duplicate code and methods in the subclasses. Let’s generate a common ‘Item’ class. And the Item class will be a superclass for each product.

#still duplicate

class GildedRose

def initialize(name, quality, days_remaining)
klass_for(name).new(quality, days_remaining)
end

def klass_for(name)
case name
when 'normal'
Normal
when 'Aged Brie'
Brie
when 'Sulfuras, Hand of Ragnaros'
Sulfuras
end
end

class Item

attr_reader :quality, :days_remaining

def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
end

end

class Normal < Item
def tick
@days_remaining -= 1
return @quality += 2 if @quality == 0
@quality += 2
@quality -= 2 if @days_remaining <= 0
end
end


class Brie < Item
def tick
@days_remaining -= 1
return @quality += 2 if @quality > 50
@quality += 2
@quality -= 2 if @days_remaining <= -1
end
end

class Sulfuras < Item
end

end

Refactoring Step 7— ( #making hash, # separate small object)

There is a very common way in the refactoring process. If you call a class almost with the same attributes you can use a Hash method that provides the relative class using the key, and value store.

We have some products in the class and we can put a hash. The key will be the name and the value will be the class instance name.

  SPECIALIZED_CLASSES = {
'normal' => Normal,
'Aged Brie' => Brie,
'Sulfuras ' => Sulfuras
}

And we can use directly this hash to create an instance :

SPECIALIZED_CLASSES[name].new(quality, days_remaining)
#making hashe
##small objects

class GildedRose

class Item
attr_reader :quality, :days_remaining
def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
end

end

class Normal < Item
def tick
@days_remaining -= 1
return @quality += 2 if @quality == 0
@quality += 2
@quality -= 2 if @days_remaining <= 0
end
end

class Brie < Item
def tick
@days_remaining -= 1
return @quality += 2 if @quality > 50
@quality += 2
@quality -= 2 if @days_remaining <= -1
end
end

class Sulfuras < Item
#some calculations
end


DEFAULT_CLASS = Item
SPECIALIZED_CLASSES = {
'normal' => Normal,
'Aged Brie' => Brie,
'Sulfuras ' => Sulfuras
}

def self.for(name, quality, days_remaining)
(SPECIALIZED_CLASSES[name] || DEFAULT_CLASS).new(quality, days_remaining)
end

end

Refactoring Step 8 — ( # new methods — new calculation methods)

Right now we can create a new calculation method very easily and we don’t need to change our main logic. That was our goal by following the Open / Closed Principle.

We can see each class and method has just its own responsibility.
That means we caught a Single Responsibility Principle.

What about the Liskov Substitution Principle (objects of a superclass shall be replaceable with objects of its subclasses without breaking the application)

We have the ‘tick’ method in the ‘Item’ superclass. This is the same responsibility for each product. And we don’t call the ‘tick’ method from the Item class. Any subclass that inherits the base class needs to have the ‘tick’ method. The caller can always be sure that the ‘tick’ method is present in the subclass.
Final code :

# new method new calcution methods

class GildedRose

class Item
attr_reader :quality, :days_remaining
def initialize(quality, days_remaining)
@quality = quality
@days_remaining = days_remaining
end

def tick
end

end

class Normal < Item
def tick
@days_remaining -= 1
return @quality += 2 if @quality == 0
@quality += 2
@quality -= 2 if @days_remaining <= 0
end
end

class Brie < Item
def tick
@days_remaining -= 1
return @quality += 2 if @quality > 50
@quality += 2
@quality -= 2 if @days_remaining <= -1
end
end

class Sulfuras < Item
#some calculations
end

class Conjured < Item
#some calculations
end

DEFAULT_CLASS = Item
SPECIALIZED_CLASSES = {
'normal' => Normal,
'Aged Brie' => Brie,
'Sulfuras ' => Sulfuras,
'Conjured' => Conjured
}

def self.for(name, quality, days_remaining)
(SPECIALIZED_CLASSES[name] || DEFAULT_CLASS).new(quality, days_remaining)
end

end

Hopefully, you have got some ideas and knowledge about refactoring in Ruby from this article. This is not a professional article but might still work in your project.

Sign up to discover human stories that deepen your understanding of the world.

Free

Distraction-free reading. No ads.

Organize your knowledge with lists and highlights.

Tell your story. Find your audience.

Membership

Read member-only stories

Support writers you read most

Earn money for your writing

Listen to audio narrations

Read offline with the Medium app

Muratatak
Muratatak

Written by Muratatak

Software Engineer, Rubyist, Follow Rails Way

No responses yet

Write a response