GildedRose Refactoring in Ruby
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
endYou 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
endRefactoring 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
endRefactoring 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
endRefactoring 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
endRefactoring 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
endin 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
endThe ‘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
endRefactoring 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
endRefactoring 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
endHopefully, 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.
Comments
Post a Comment