Are you an Egoistic Programmer?

Agnieszka Matysek at Fractal Soft

Notes: - Agnieszka Matysek - nowe technologie w rolnictwie - 8 lat doświadczenia - team leader, mentor - współwłaściciel firmy Fractal Soft - zainteresowania: good code standard, refactoring, testing
Notes: - większość przykładów w Ruby - pracuje i lubię ten język
Notes: - projekt w kiepskiej sytuacji - nic nie pasuje do niczego - pojawia się strach - wołamy o pomoc
## What did we wrong? Notes: - co poszło nie tak? - przerzucamy się winą - To wina technologii - nie jest dostosowana do potrzeb - To wina klienta - naciska tylko na nowe funkcjonalności - nie mogłem inaczej
Notes: - zamiast obwiniać się to dbać - być jak skauci / harcerze - pozostaw miejsce obozowania czystsze niż było - wchodzisz do pliku z kodem i zostawiasz go czystszym, lepszym

Poker - refactoring

Poker
```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

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

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

OOP
## Step 3 - From procedural to more OOP #### Array everywhere ```ruby def three_of_a_kind?(array) hash = Hash.new(0) array.each { |item| hash[item / 4] += 1 } hash.values.include?(3) end ``` Notes: - w każdej metodzie tak samo - `array` argument - przekazujemy stan
#### 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

Cleanups
## 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

Exhibitionism
```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

Bla bla bla
## Step 6 - More descriptive output #### Before ```ruby 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 ``` Notes: - liczby nie wiele nam mówią - za każdym razem trzeba sprawdzać
## Step 6 - More descriptive output #### After ```ruby def check return :straight_flush if straight_flush? return :four_of_a_kind if four_of_a_kind? return :full_house if full_house? return :flush if flush? return :straight if straight? return :three_of_a_kind if three_of_a_kind? return :two_pair if two_pair? return :one_pair if one_pair? return :high_card if high_card? return :none end ``` Notes: - zmiana testów!!!
## 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

SOLID
## 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

Name 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

Simplicity

Simple conditions/logic

```javascript if (this.isContext && !item.hasContextId) {} else { // more logic here } ```

KISS, DRY and Rock & Roll

KISS, DRY and Rock & Roll

What does this code?

```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 ```

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?

Woman on Rails logotype

Agnieszka Matysek

agnieszka (at) fractalsoft (dot) org