Ruby Refactoring step by step - part 1

How refactor your code?
From procedural to more object related code.
| 9 min.

It was a long break from the last technical article. During that time I was trying many different things. You can look here: Be More - my lifestyle blog in Polish, Woman on Rails YouTube channel and my travel Vimeo channel. It was a great time to discover what I like to do and what not. But back to the topic. I prepared this article for a long time. I can say even too long. I started in 2015 and now you will see the results. Let’s get started.

Refactoring is one of my favorite topics. I love to clean up things in real life and also in code. I’ve worked and I’m still working on web application projects. And I look for answers on how to write good code. What are the reasons that after some time our code is messy and not readable? So day by day I learn how to refactor code in a good way based on my experience and the experience of others. Today I would like to share an example of refactoring with you.

I got a code which was written by a person on internship long time ago in my company. The plan was to improve this code. It was mostly one class which you can see here. In this one class you have all rules in poker to check a hand only for cards without jokers. That code is not bad. When you know business logic (in this case poker rules), you will be able to move around this code. This code has also tests. So, this is good news. It will be easier to change something if tests cover all logic. If they don’t, we can break functionality, when we won’t be careful. The code looks more procedural than object oriented and it has some duplications. Sometimes this will be enough. When this is written once and you will not need to change this code for long time maybe you don’t need to clean it up. But if the requirements can/will change, probably your code does too. You need to decide if it is better to refactor it now or later. (I prefer refactoring now, when I still remember logic and code, then later when I need to understand it one more time.) I would like to show you how I do refactoring base on this example.

Step 1 - Preparing environment to work

I updated gems which were in the project and I installed gems like Rubocop or Reek to help myself start refactoring. Those tools are helpful to see where problem in code can be. To do this, first do the overview. But those are only tools, sometimes they are right and sometimes are not. And it is easy to cheat them. But this is another topic.

Stats (based on metrics):

  • LOC (Line of code) - 194
  • LOT (Line of tests) - 168
  • Flog - 112.8
  • Flay - 123
  • Tests - 12 examples, 0 failures

Step 2 - Start first clean ups

Base on tests without going deep into logic, I made some improvements in the code. I removed some condition and simplified this code.

Code before change:

def straight_flush?(array)
  if straight?(array) and flush?(array)
    return true
  else
    return false
  end
end

Code after change:

def straight_flush?(array)
  straight?(array) && flush?(array)
end

All the code you can find here. This change, in my opinion, improves readability a little bit.

After that all tests passed.

Step 3 - Understand the logic and try to simplify it

Now when code is more readable, I can start changing the logic to simplify it. I have tests, so each of my changes relies on tests. I took the first method. I removed it (remove all content inside). And start from the beginning. This is what I got.

Code before change:

def one_pair?(array)
  tmp = array.clone
  tmp.collect!{|x| x / 4 }

#  (0..8).each do |elem|
#    tmp.delete(elem)
#  end

  helper_array = [0] * 13

  tmp.each do |elem|
    helper_array[elem] += 1
  end

  helper_array.delete(0)

  helper_array.include?(2)
end

Code after:

def one_pair?(array)
  hash = Hash.new(0)
  array.each { |item| hash[item / 4] += 1 }
  hash.values.include?(2)
end

I repeat that approach over and over again for all methods.

  1. Take method and remove content
  2. Run the tests (some tests are failing), understand the logic
  3. Write new code in a simpler way
  4. Check the tests

After this step you can find the code here. During that step I also removed commented code, Polish comments and I added some tests, which for me, were missing.

Stats:

  • LOC - 73
  • LOT - 170
  • Flog - 76.3
  • Flay - 63
  • Tests - 12 examples, 0 failures

Step 4 - From procedural to more object oriented code

I don’t know if you noticed this, but everywhere we put as argument array. And we have class, but we don’t use initializer for this class. The second thing is that in many places we use something like array.each {|item| hash [item / 4] += 1} let’s move this also for initializer and use class state instead of calculating this everywhere.

Quick explanation:

I think at this point I should explain a little bit how this code works. Each card has representation as a number from 0 to 51. So number 0-3 represents 2 with all colors, 4-7 represents 3 and so on. Like in this table:

02♠43♠84♠125♠166♠207♠248♠
12♣53♣94♣135♣176♣217♣258♣
22♥63♥104♥145♥186♥227♥268♥
32♦73♦114♦155♦196♦237♦278♦


289♠3210♠36J♠40D♠44K♠48A♠
299♣3310♣37J♣41D♣45K♣49A♣
309♥3410♥38J♥42D♥46K♥50A♥
319♦3510♦39J♦43D♦47K♦52A♦

So code like array.map {|item| item / 4} tells us which figure from 2 to ace is on card and code like this array.map {|item| item % 4} represents the color of a card (♠, ♣, ♥, ♦).

For more explanation of poker rules, please check wiki page to list all of poker hands.

We add an initializer:

def initialize(array)
  @array = array.sort
  @cards = @array.map { |item| item / 4 }
end

Example of a method before change:

def three_of_a_kind?(array)
  hash = Hash.new(0)
  array.each { |item| hash[item / 4] += 1 }
  hash.values.include?(3)
end

After:

def three_of_a_kind?
  hash = Hash.new(0)
  @cards.each { |item| hash[item] += 1 }
  hash.values.include?(3)
end

We remove some of duplication and use state of the class instance to improve our code. After this step you can find the code here. Quick note - I also did small refactoring on tests. I just moved the test cases to the array to also remove duplication there.

Stats:

  • LOC - 76
  • LOT - 190
  • Flog - 70.9
  • Flay - 57
  • Tests - 104 examples, 0 failures

Step 5 - Remove duplication

Base on Reek metric I noticed a lot of duplication in code. So I decided to move this code to one method and use the state of the class’ instance one more time. You can find the whole step here. And this is a quick overview.

We change the initalizer:

def initialize(array)
  @array = array.sort
  @cards = @array.map { |item| item / 4 }
  @frequency = cards_frequency
end

New method cards_frequency:

def cards_frequency
  hash = Hash.new(0)
  @cards.each { |item| hash[item] += 1 }
  hash.values
end

Example of a method before change:

def four_of_a_kind?
  hash = Hash.new(0)
  @cards.each { |item| hash[item] += 1 }
  hash.values.include?(4)
end

After:

def four_of_a_kind?
  @frequency.include?(4)
end

Stats:

  • LOC - 76
  • LOT - 190
  • Flog - 61.0
  • Flay - 0
  • Tests - 104 examples, 0 failures

Step 6 - Small public interface

When you will look at the code from step 5, you will notice that all methods are public. The big public interface is something hard to maintain. If you will try to replace class Hand by other class you need to prepare class with the same big interface. If something is public you can use it everywhere, what also can provide new dependencies in the code. In this case, when you look closer, you will see that even tests are checking only the method check. I decided to declare all others method as private. This change is visible here.

Stats:

  • LOC - 77
  • LOT - 190
  • Flog - 59.9
  • Flay - 0
  • Tests - 104 examples, 0 failures

Step 7 - More clean ups

This step is similar to step 5. I removed more duplication in code and decided to change names to be more specific. I created new method cards_figures_and_colors which prepare now two things: figures and colors. You can say now: where is Single responsibility principle or that is micro optimalization because now I use only one loop instead 2. My intuition tells me that this is ok. But you can have a different opinion. That’s fine for me. I respect that. This is how this method looks like:

def cards_figures_and_colors
  @array.map { |item| [item / 4, item % 4] }.transpose
end

and I’m open to discuss if this was a good or bad step. This change affects also our initialize method:

def initialize(array)
  @array = array.sort
  @figures, @colors = cards_figures_and_colors
  @frequency = cards_frequency.values
end

In this step I also did change in method cards_frequency. I used there each_with_object (you can find more about this method in my article Quick overview - each_with_object method) instead of each like this:

def cards_frequency
  @figures.each_with_object(Hash.new(0)) do |item, hash|
    hash[item] += 1
  end
end

Thanks to @colors instance variable I can change flush? method from:

def flush?
  color = @array.map { |item| item % 4 }
  color.uniq.size == 1
end

to:

def flush?
  @colors.uniq.size == 1
end

You can see all the changes here

Stats:

  • LOC - 80
  • LOT - 190
  • Flog - 64.5
  • Flay - 0
  • Tests - 104 examples, 0 failures

Summarize

Let’s summarize what we’ve done till this point:

  • We used metrics to do first cleanup in the code - to start
  • We simplified the code using tests and our understanding of logic
  • We changed procedural code and used for that more object oriented approach
  • We removed duplications
  • and we created a small public interface

In my next article I would like to go deeper with this refactoring and focus on:

  • Code which is more descriptive
  • Metaprograming as method to write more elastic code
  • Preparing small independent classes, then one big class
  • Building classes as elements which are replaceable and possible to combine
  • Explanation why I used metrics on each step and what they tell us

Stay tuned! My next article will come soon! If you like this article share your thoughts with me in the comments below. Thank you so much for being here. And see you next time. Bye!

Bibliography

Books

Presentations


Tagged with