Are you an Egoistic Programmer?
Agnieszka Małaszkiewicz at Fractal Soft
Poker - refactoring
```ruby
module Poker
# Poker hand
class Hand
def check(array)
array.sort!
return 9 if straight_flush?(array)
return 8 if four_of_a_kind?(array)
return 7 if full_house?(array)
return 6 if flush?(array)
return 3 if two_pair?(array)
return 2 if one_pair?(array)
return 4 if three_of_a_kind?(array)
return 5 if straight?(array)
return 1 if high_card?(array)
return 0
end
#Jeśli układ jest pokerem, numery kart modulo 4 są sobie równe
def straight_flush?(array)
if straight?(array) and flush?(array)
return true
else
return false
end
end
#jeśli mamy karetę, jeśli podzielę numery kart przez 4
#4 pierwsze lub 4 ostatnie powinny być sobie równe
def four_of_a_kind?(array)
tmp = array.clone
tmp.collect!{|x| x / 4 }
helper_array = [0] * 13
tmp.each do |elem|
helper_array[elem] += 1
end
helper_array.delete(0)
if helper_array.include?(4) and helper_array.include?(1)
return true
else
return false
end
end
def full_house?(array)
tmp = array.clone
tmp.collect!{|x| x / 4 }
helper_array = [0] * 13
tmp.each do |elem|
helper_array[elem] += 1
end
helper_array.delete(0)
if helper_array.include?(3) and helper_array.include?(2)
return true
else
return false
end
end
def flush?(array)
tmp = array.clone
tmp.collect!{|x| x % 4 }
if tmp.uniq.length == 1
return true
else
return false
end
end
def straight?(array)
if array.last / 4 == 12
return straight_with_ace?(array)
else
return true_straight?(array)
end
end
```
```ruby
def straight_with_ace?(array)
if true_straight?(array)
return true
else
tmp = array.clone
tmp.collect!{|x| x / 4}
tmp.delete(12)
sum = 0;
tmp.each do |num|
sum += num
end
if sum == 6
return true
else
return false
end
end
end
def true_straight?(array)
tmp = array.clone
minimum_card = tmp[0] / 4
tmp.collect!{|x| x / 4 - minimum_card}
sum = 0;
tmp.each do |num|
sum += num
end
if sum == 10
return true
else
return false
end
end
def three_of_a_kind?(array)
tmp = array.clone
tmp.collect!{|x| x / 4 }
helper_array = [0] * 13
tmp.each do |elem|
helper_array[elem] += 1
end
helper_array.delete(0)
if helper_array.include?(3) and helper_array.include?(1)
return true
else
return false
end
end
def two_pair?(array)
tmp = array.clone
tmp.collect!{|x| x / 4 }
helper_array = [0] * 13
tmp.each do |elem|
helper_array[elem] += 1
end
helper_array.delete(0)
if helper_array.include?(2) and helper_array.length == 3
return true
else
return false
end
end
```
```ruby
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)
if helper_array.include?(2)
return true
else
return false
end
end
def high_card?(array)
if array.last >= 36
return true
else
return false
end
end
end
end
```
[All code is here](https://github.com/womanonrails/poker/blob/55c9ae0ab921f7aa95bb7e47676d87b970a32033/lib/poker/hand.rb)
Stats
- LOC - 194
- LOT - 168
- Flog - 112.8
- Flay - 123
- Tests - 12 examples, 0 failures
First stats
- LOC - 194
- LOT - 168
- Flog - 112.8
- Flay - 123
- Tests - 12 examples, 0 failures
Last stats
- LOC - 37
- LOT - 173
- Flog - 28.0
- Flay - 0
- Tests - 145 examples, 0 failures
Step 1 - Linter cleanups
## Step 1 - Linter cleanups
#### Code before change:
```ruby
def straight_flush?(array)
if straight?(array) and flush?(array)
return true
else
return false
end
end
```
#### Code after change:
```ruby
def straight_flush?(array)
straight?(array) && flush?(array)
end
```
Step 2 - Simplify logic
## Step 2 - Simplify logic
#### Code before change:
```ruby
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
```
## Step 2 - Simplify logic
#### Code after:
```ruby
def one_pair?(array)
hash = Hash.new(0)
array.each { |item| hash[item / 4] += 1 }
hash.values.include?(2)
end
```
Old stats
- LOC - 194
- LOT - 168
- Flog - 112.8
- Flay - 123
- Tests - 12 examples, 0 failures
New stats
- LOC - 73
- LOT - 170
- Flog - 76.3
- Flay - 63
- Tests - 12 examples, 0 failures
Step 3 - From procedural to more OOP
#### Add initializer
```ruby
def initialize(array)
@array = array.sort
@cards = @array.map { |item| item / 4 }
end
```
#### Before
```ruby
def three_of_a_kind?(array)
hash = Hash.new(0)
array.each { |item| hash[item / 4] += 1 }
hash.values.include?(3)
end
```
#### After
```ruby
def three_of_a_kind?
hash = Hash.new(0)
@cards.each { |item| hash[item] += 1 }
hash.values.include?(3)
end
```
```ruby
class Hand
def initialize(array)
@array = array.sort
@carts = @array.map { |item| item / 4 }
end
def check
return 9 if straight_flush?
return 8 if four_of_a_kind?
return 7 if full_house?
return 6 if flush?
return 5 if straight?
return 4 if three_of_a_kind?
return 3 if two_pair?
return 2 if one_pair?
return 1 if high_card?
return 0
end
def straight_flush?
straight? && flush?
end
def four_of_a_kind?
hash = Hash.new(0)
@carts.each { |item| hash[item] += 1 }
hash.values.include?(4)
end
def full_house?
three_of_a_kind? && one_pair?
end
def flush?
color = @array.map { |item| item % 4 }
color.uniq.size == 1
end
def normal_straight?
@carts.each_cons(2) do |previous, current|
return false unless previous + 1 == current
end
true
end
def straight?
[0, 1, 2, 3, 12] == @carts || normal_straight?
end
def three_of_a_kind?
hash = Hash.new(0)
@carts.each { |item| hash[item] += 1 }
hash.values.include?(3)
end
def two_pair?
hash = Hash.new(0)
@carts.each { |item| hash[item] += 1 }
(hash.values - [2]).size == 1
end
def one_pair?
hash = Hash.new(0)
@carts.each { |item| hash[item] += 1 }
hash.values.include?(2)
end
def high_card?
@array.last >= 36
end
end
```
before stats...
Changes in Tests
Old stats
- LOC - 73
- LOT - 170
- Flog - 76.3
- Flay - 63
- Tests - 12 examples, 0 failures
New stats
- LOC - 76
- LOT - 190
- Flog - 70.9
- Flay - 57
- Tests - 104 examples, 0 failures
Step 4 - Remove duplication
## Step 4 - Remove duplication
```ruby
def three_of_a_kind?
hash = Hash.new(0)
@cards.each { |item| hash[item] += 1 }
hash.values.include?(3)
end
```
## Step 4 - Remove duplication
#### Change initializer
```ruby
def initialize(array)
@array = array.sort
@cards = @array.map { |item| item / 4 }
@frequency = cards_frequency
end
```
#### New method
```ruby
def cards_frequency
hash = Hash.new(0)
@cards.each { |item| hash[item] += 1 }
hash.values
end
```
## Step 4 - Remove duplication
#### Before
```ruby
def four_of_a_kind?
hash = Hash.new(0)
@cards.each { |item| hash[item] += 1 }
hash.values.include?(4)
end
```
#### After
```ruby
def four_of_a_kind?
@frequency.include?(4)
end
```
Old stats
- LOC - 76
- LOT - 190
- Flog - 70.9
- Flay - 57
- Tests - 104 examples, 0 failures
New stats
- LOC - 75
- LOT - 190
- Flog - 59.9
- Flay - 0
- Tests - 104 examples, 0 failures
Step 5 - small public interface
```ruby
class Hand
def initialize(array)
@array = array.sort
@cards = @array.map { |item| item / 4 }
@frequency = cards_frequency
end
def cards_frequency
hash = Hash.new(0)
@cards.each { |item| hash[item] += 1 }
hash.values
end
def check
return 9 if straight_flush?
return 8 if four_of_a_kind?
return 7 if full_house?
return 6 if flush?
return 5 if straight?
return 4 if three_of_a_kind?
return 3 if two_pair?
return 2 if one_pair?
return 1 if high_card?
return 0
end
def straight_flush?
straight? && flush?
end
def four_of_a_kind?
@frequency.include?(4)
end
def full_house?
three_of_a_kind? && one_pair?
end
def flush?
color = @array.map { |item| item % 4 }
color.uniq.size == 1
end
def normal_straight?
@cards.each_cons(2) do |previous, current|
return false unless previous + 1 == current
end
true
end
def straight?
[0, 1, 2, 3, 12] == @cards || normal_straight?
end
def three_of_a_kind?
@frequency.include?(3)
end
def two_pair?
(@frequency - [2]).size == 1
end
def one_pair?
@frequency.include?(2)
end
def high_card?
@array.last >= 36
end
end
```
## Step 5 - small public interface
```ruby
[
[0, 4, 8, 12, 16],
[5, 9, 13, 17, 21],
[10, 14, 18, 22, 26],
[15, 19, 23, 27, 31],
[16, 20, 24, 28, 32],
[37, 29, 33, 21, 25].shuffle,
[30, 34, 38, 26, 42].shuffle,
[31, 35, 47, 43, 39].shuffle,
[32, 48, 40, 44, 36].shuffle,
[49, 45, 41, 37, 33].shuffle
].each do |cards|
it "detects straight_flush for #{cards}" do
hand = described_class.new(cards)
expect(hand.check).to eq 9
end
end
```
Old stats
- LOC - 75
- LOT - 190
- Flog - 59.9
- Flay - 0
- Tests - 104 examples, 0 failures
New stats
- LOC - 77
- LOT - 190
- Flog - 59.9
- Flay - 0
- Tests - 104 examples, 0 failures
Step 6 - More descriptive output
## BONUS - Meta-programming
#### Add to initializer
```ruby
@order_checking = [
:straight_flush, :four_of_a_kind, :full_house, :flush,
:straight, :three_of_a_kind, :two_pair, :one_pair,
:high_card, :none
]
```
#### Code after
```ruby
def check
@order_checking.each do |name|
method_name = (name.to_s + '?').to_sym
return name if send(method_name)
end
end
```
Old stats
- LOC - 77
- LOT - 190
- Flog - 59.9
- Flay - 0
- Tests - 104 examples, 0 failures
New stats
- LOC - 80
- LOT - 190
- Flog - 62.5
- Flay - 0
- Tests - 104 examples, 0 failures
Step 7 - Small object
## Step 7 - Small object
```ruby
module Poker
class FourOfAKind
def initialize(array)
@array = array.sort
@figures, @colors = cards_figures_and_colors
@frequency = cards_frequency.values
end
def check
:four_of_a_kind if @frequency.include?(4)
end
private
def cards_figures_and_colors
@array.map { |item| [item / 4, item % 4] }.transpose
end
def cards_frequency
@figures.each_with_object(Hash.new(0)) do |item, hash|
hash[item] += 1
end
end
end
end
```
## Step 7 - Small object
#### Create/modify `if` statement
```ruby
def check
@order_checking.each do |name|
if name == :four_of_a_kind
return name if FourOfAKind.new(@array).check == name
else
method_name = (name.to_s + '?').to_sym
return name if send(method_name)
end
end
end
```
## Step 7 - Small object
```ruby
require 'spec_helper'
describe Poker::FourOfAKind do
[
[0, 1, 2, 3, 4],
[4, 5, 6, 0, 7],
[8, 9, 0, 10, 11],
[12, 0, 13, 14, 15],
[0, 16, 17, 18, 19],
[20, 21, 22, 23, 0].shuffle,
[24, 25, 26, 27, 0].shuffle,
[28, 29, 30, 31, 0].shuffle,
[32, 33, 34, 35, 0].shuffle,
[36, 37, 38, 39, 0].shuffle
].each do |cards|
it "detects four_of_a_kind for #{cards}" do
hand = described_class.new(cards)
expect(hand.check).to eq :four_of_a_kind
end
end
end
```
Old stats
- LOC - 80
- LOT - 190
- Flog - 62.5
- Flay - 0
- Tests - 104 examples, 0 failures
New stats
- LOC - 85
- LOT - 200
- Flog - 65.5
- Flay - 0
- Tests - 116 examples, 0 failures
First stats
- LOC - 194
- LOT - 168
- Flog - 112.8
- Flog total - 112.8
- Flay - 123
- Tests - 12 examples, 0 failures
Last stats
- LOC - 37
- LOT - 173
- Flog - 28.0
- Flog total- 182.4
- Flay - 0
- Tests - 145 examples, 0 failures
Rules & Examples
Names with meaning
Names with meaning
```ruby
n12
n1n2
```
Names with meaning
```ruby
n12 = n1 ^ 2
n1n2 = n1 * n2
```
Names with meaning
```ruby
field_ids
```
#### Bad
```ruby
field_ids = Field.pluck(:number)
```
#### Good
```ruby
field_ids = Field.pluck(:id)
```
Simple conditions/logic
Simple conditions/logic
```javascript
if (this.isContext && !item.hasContextId) {} else {
// more logic here
}
```
KISS, DRY and Rock & Roll
What does this code?
```basic
For i = 1 To m1
For j = 1 To m1
If i <> j Then
s3(i, j) = 0
GoTo 410
End If
s3(i, j) = 1
410 Next j
Next i
```
Is it helpful?
```basic
m1 = 2
```
```basic
For i = 1 To m1
For j = 1 To m1
If i <> j Then
s3(i, j) = 0
GoTo 410
End If
s3(i, j) = 1
410 Next j
Next i
```
Solution 1
```ruby
array = [
[1, 0],
[0, 1]
]
```
Solution 2
```basic
For i = 1 To m1
For j = 1 To m1
If i <> j Then s3(i, j) = 0
If i = j Then s3(i, j) = 1
Next j
Next i
```
More and more rules...
Write clean code
Small things matter
Are you an Egoistic Programmer?
Fractal Soft