Dużo czasu minęło od mojego ostatniego artykułu technicznego. Przez ten czas próbowałam wielu nowych rzeczy. Założyłam blog Be more, który dotyczy moich przemyśleń na temat życia, Kanał Woman on Rails na YouTube i podróżniczy kanał na Vimeo. To był czas odkrywania, co sprawia mi przyjemność a co nie. Ale wracając do tematu. Do tego artykułu przygotowywałam się naprawdę długo. Może nawet za długo. Pomysł pojawił się już 2015 roku, a teraz możesz zobaczyć jego rezultaty. Zaczynajmy!

Refaktoring jest jednym z moich ulubionych tematów. Uwielbiam porządki w prawdziwym życiu, ale też w kodzie źródłowym. Pracowałam i nadal pracuję nad aplikacjami internetowymi. I wciąż poszukuje odpowiedzi na następujące pytania: Jak pisać dobry kod? Co powoduje, że po pewnym czasie nasz kod staje się brzydki i nieczytelny? Jak radzić sobie z rosnącą złożonością w projektach? Każdego dnia uczę się jak robić dobry refaktoring. Bazuję na zdobytym przeze mnie, jak i przez innych, doświadczeniu. Dziś chciałabym się podzielić z Tobą przykładem refaktoringu zrobionego krok po kroku.

Do tego celu wykorzystam kod, który został napisany dawno temu przez młodego programistę w mojej firmie. Plan był następujący - ulepszyć ten kod źródłowy. W zasadzie cała logika to jedna klasa, którą możesz zobaczyć tutaj. W tej klasie znajdują się wszystkie zasady potrzebne do sprawdzenia tego, co mamy w ręce grając w pokera, ale bez użycia jokera. Kod nie jest zły. Kiedy znasz logikę biznesową (w tym przypadku zasady pokera), jesteś wstanie poruszać się po tym kodzie. Ten fragment kodu posiada też testy, co jest jego zaletą. Będzie nam o wiele łatwiej zmienić cokolwiek, gdy mamy testy pilnujące logiki. Jeżeli jednak nie cała logika jest przetestowana, to możemy zepsuć fragment funkcjonalności nie zdając sobie nawet z tego sprawy. Kod ten wygląda bardziej proceduralnie niż obiektowo i będę chciała się tym zająć w odpowiednim czasie. Posiada on też wiele powtórzeń. Czasami taki fragment kodu jest w zupełności wystarczający. Wszystko zależy od projektu i wymagań. Jeżeli kod został napisany raz, działa poprawnie i nikt do niego nie będzie musiał zaglądać, to może zostawienie go w takim stanie jest w jakiś sposób uzasadnione z biznesowego punktu widzenia. Natomiast jeżeli zdarzy się, że zmienią się wymagania, to prawdopodobnie kod źródłowy też ulegnie zmianie. To Ty musisz zdecydować, czy będziesz refaktoryzowała kod teraz czy później. Ja preferuje pierwszą opcję. Dopóki pamiętam logikę i zależności łatwiej jest mi kod zmienić. Po pewnym czasie trzeba najpierw jeszcze raz zrozumieć strukturę, zanim zacznie się coś modyfikować. No to zaczynamy!

Krok 1 - Przygotowanie środowiska

Zaczęłam od zaktualizowania wszystkich gemów w projekcie oraz doinstalowania narzędzi takich jak Rubocop czy Reek. Są to metryki czyli pewnego rodzaju wskaźniki jakości kodu. Pomogą nam sprawdzić na czym stoimy i gdzie można zacząć robić porządki. Trzeba jednak pamiętać, że są to tylko narzędzia. A narzędzia mogą się mylić i można je łatwo oszukać. Ale to temat na osobny artykuł.

Statystyki (bazując na metrykach):

LOC (Line of code - liczba linii kodu) - 194
LOT (Line of tests - liczba linii testów) - 168
Flog - 112.8
Flay - 123
Testy - 12 examples, 0 failures (12 przypadków testowych, 0 nieprzechodzących)

Krok 2 - Pierwsze porządki

Bazując na testach i metrykach, nie wchodząc w głębsze zrozumienie logiki, zrobiłam pierwsze usprawnienia. Usunęłam niektóre warunki i uprościłam kod.

Kod przed zmianami:

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

Kod po zmianach:

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

Cały kod możesz znaleźć tutaj. Te zmiany moim zdaniem poprawiły odrobinę czytelność kodu.

Po tym kroku, wszystkie testy przechodziły.

Krok 3 - Zrozumienie logiki i dalsze uproszczenia

Teraz gdy kod jest dla mnie bardziej przejrzysty, mogę przejść do właściwej zmiany logiki. Mam testy, więc każda zmiana będzie się na nich opierała. Cel polegała na ich spełnieniu, czyli sprawieniu, że testy przechodza. Wzięłam pierwszą metodę i usunęłam całe jej wnętrze. Oto co dostałam:

Kod przed zmianą:

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

Kod po zmianie:

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

Dla każdej metody w tej klasie powtarzałam następujące kroki:

Brałam metodę i usuwałam jej zawartość
Uruchamiałam testy (niektóre z nich przestały przechodzić) i na ich podstawie starałam się zrozumieć logikę
Pisałam nowy kod w prostszy sposób
Sprawdzałam czy wszystkie testy przechodzą

Kod po moich zmianach możesz znaleźć tutaj. Podczas tego kroku usunęłam również zakomentowany kod, komentarze po polsku i dodałam kilka testów jednostkowych, których moim zdaniem brakowało.

Statystyki:

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

Krok 4 - Od kodu proceduralnego do obiektowego

Nie wiem czy to zauważyłaś, ale do każdej metody przekazujemy argument array. Kod jest zamknięty w klasę, ale nie używamy tam inicjalizera (metody inicjującej instancję klasy). Poza tym mamy wiele miejsc, gdzie używamy array.each {|item| hash [item / 4] += 1}. Zacznijmy od przeniesienia tego fragmentu do inicjalizera i użyjmy stanu obiektu do przechowania tej wartości, zamiast wyliczać ją wielokrotnie.

Szybkie wyjaśnienie:

Myślę, że to dobry moment aby wytłumaczyć choć odrobinę, jak ten kod działa. Każdą kartę z talii reprezentuje jedna liczba od 0 do 51. Tak więc liczby od 0-3 reprezentują dwójki we wszystkich kolorach, liczby 4-7 reprezentują trójki itd. Całość tej zależności przedstawiona jest w tabeli poniżej:

0	2♠	4	3♠	8	4♠	12	5♠	16	6♠	20	7♠	24	8♠
1	2♣	5	3♣	9	4♣	13	5♣	17	6♣	21	7♣	25	8♣
2	2♥	6	3♥	10	4♥	14	5♥	18	6♥	22	7♥	26	8♥
3	2♦	7	3♦	11	4♦	15	5♦	19	6♦	23	7♦	27	8♦

28	9♠	32	10♠	36	J♠	40	D♠	44	K♠	48	A♠
29	9♣	33	10♣	37	J♣	41	D♣	45	K♣	49	A♣
30	9♥	34	10♥	38	J♥	42	D♥	46	K♥	50	A♥
31	9♦	35	10♦	39	J♦	43	D♦	47	K♦	52	A♦

Jeżeli mamy kod array.map {|item| item / 4} to tak naprawdę sprawdzamy jaką figurę od 2 do Asa reprezentuje liczba. Natomiast jeżeli mamy array.map {|item| item % 4} sprawdzamy jakiego koloru jest dana karta (♠, ♣, ♥, ♦).

Gdybyś potrzebowała dokładniejszego wytłumaczenia zasad pokera, to sprawdź listę wszystkich pokerowych ustawień ręki na Wikipedii.

Dodajemy inicjalizer:

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

Przykład metody przed zmianą:

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

Po zmianie:

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

Usunęłam tutaj powtarzające się fragmenty kodu, używając stanu trzymanego w instancji klasy. Kod po tym kroku możesz znaleźć tutaj. Mała uwaga - dodatkowo zrobiłam refaktoring w testach. Postanowiłam przenieść wszystkie możliwe przypadki testowe do tablicy by uniknąć powtórzeń, jakie były widoczne również w testach.

Statystyki:

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

Krok 5 - Usuwanie powtórzeń (duplikacji)

Bazując na metryce Reek zauważyłam dużo powtórzeń w kodzie. Zdecydowałam, że jeszcze raz wykorzystam stan obiektu, by się ich pozbyć. Wszystkie zmiany związane z tym krokiem możesz znaleźć tutaj. A poniżej zamieszczam skrót tego co zrobiłam:

Zmiana w inicjalizerze:

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

Dodanie nowej metody cards_frequency:

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

Przykład jednej metody przed zmianą:

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

Po zmianie:

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

Statystyki:

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

Krok 6 - Mały publiczny interface

Kiedy spojrzysz na kod z kroku 5, to na pewno zauważysz, że mamy bardzo dużo metod dostępnych publicznie do wykorzystania na obiekcie naszej klasy. Duży publiczny interface jest ciężki w utrzymaniu. Jeżeli chciałybyśmy zastąpić naszą klasę Hand inną klasą, to będziemy potrzebować dokładnie tyle samo metod publicznych, jak w przypadku klasy Hand. Dodatkowo każda publicznie dostępna metoda może zostać wykorzystana przez inny fragment kodu, co może powodować niepotrzebne zależności między obiektami. W naszym przypadku, jak przyjrzymy się bliżej okaże się, że nawet testy nie sprawdzają wszystkich dostępnych metod. Zajmują się tylko sprawdzeniem metody check. Zdecydowałam więc, że jedyną publicznie dostępną metodą będzie metoda check. Pozostałe metody będą pomocniczymi metodami prywatnymi. Zmiany możesz zobaczyć tutaj.

Statystyki:

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

Krok 7 - Jeszcze więcej porządków

Ten krok jest podobny do kroku 5. Usuwam dodatkowe powtórzenia w kodzie i zmieniam nazwy na bardziej opisowe, by ułatwić późniejsze czytanie kodu. Stworzyłam nową metodę cards_figures_and_colors, która przygotuje dwie rzeczy: figures czyli figury i colors czyli kolory kart. Możesz teraz powiedzieć: a gdzie jest zasada pojedynczej odpowiedzialności lub to jest mikro optymalizacja, ponieważ zamiast dwóch pętli masz tylko jedną. Moja intuicja podpowiada mi że to, co zrobiłam jest ok. Ale Ty możesz mieć inne zdanie i to też jest ok. Szanuję je. Oto jak wygląda ta metoda:

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

Jestem otwarta na dyskusję, czy moje podejście jest dobre czy nie. Ta zmiana pociąga za sobą także zmiany w metodzie initialize:

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

W tym kroku postanowiłam zmienić również metodę cards_frequency. Zamiast używać each używam each_with_object. Jeżeli jesteś zainteresowana większą ilością informacji na temat each_with_object zachęcam Cię do przeczytania mojego artykułu o użyciu metody each_with_object w Ruby. Oto jak teraz wygląda kod:

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

Dzięki zmiennej @colors mogę zmienić metodę flush? z:

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

na:

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

Wszystkie zmiany możesz zobaczyć tutaj.

Statystyki:

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

Podsumowanie

Podsumujmy co do tej pory udało nam się osiągnąć:

Użyłyśmy metryk do pierwszych, wstępnych porządków
Uprościłyśmy kod bazując na testach i zrozumieniu logiki
Zmieniłyśmy kod proceduralny na obiektowy
Usunęłyśmy powtórzenia w kodzie
i stworzyłyśmy mały publiczny interface

W następnym artykule chciałabym wejść jeszcze głębiej w temat tego refaktoringu i skupić się na:

Bardziej opisowym kodzie
Meta-programowaniu jako sposobie na pisanie elastycznego kodu
Przygotowaniu małych niezależnych klas, zamiast jednej dużej klasy
Budowaniu klas jako elementów wymiennych i takich, które można ze sobą łączyć
Wyjaśnieniu po co podawałam metryki na każdym kroku i co one nam wlaściwie mówią

Trzymaj się! Mój następny artykuł pojawi się już wkrótce! Jeżeli masz jakieś pytania lub przemyślenia, to podziel się nimi w komentarzach. Do zobaczenia następnym razem. Cześć!

Bibliografia

Książki

Prezentacje angielskojęzyczne

Tagged with Ruby

Woman on Rails

Premature optimization is the root of all evil.

Refaktoring w Ruby krok po kroku - część 1

Krok 1 - Przygotowanie środowiska

Statystyki (bazując na metrykach):

Krok 2 - Pierwsze porządki

Krok 3 - Zrozumienie logiki i dalsze uproszczenia

Statystyki:

Krok 4 - Od kodu proceduralnego do obiektowego

Szybkie wyjaśnienie:

Statystyki:

Krok 5 - Usuwanie powtórzeń (duplikacji)

Statystyki:

Krok 6 - Mały publiczny interface

Statystyki:

Krok 7 - Jeszcze więcej porządków

Statystyki:

Podsumowanie

Bibliografia

Książki

Prezentacje angielskojęzyczne