Refactoring - what it is and an example

8 december 2022
Jakub Rojek Jakub Rojek
Photo by Kei Scampa on Pexels (https://www.pexels.com/pl-pl/zdjecie/rozne-czesci-aparatu-2286953/)
Categories: Programming, Methodologies, Guides

Przy paru okazjach wspominaliśmy o tym, jak ważne jest utrzymanie odpowiedniej jakości kodu źródłowego. To nie jest tylko kwestia honoru i pokazania, jak piękny styl prezentują programiści - to realna i przydatna umiejętność, pozwalająca na łatwiejsze rozwijanie oprogramowania w przyszłości, rozszerzenia go i odnalezienia się w nim przez nowych programistów, a także starszych po długiej przerwie. To także możliwość szybszego dotarcia do źródła błędów i ich poprawienia. Tto wszystko przekłada się na przyjemniejsze programowanie, a to z kolei na wyższą jakość samego oprogramowania z funkcjonalnego punktu widzenia. Szczęśliwy programista to dobry software.

Problem w tym, że pisanie ładnego kodu jest trudne, także dla dobrych programistów i nie ma to związku tylko z umiejętnościami lub wiedzą. Nie da się ukryć, że tworzenie wypielęgnowanych linijek składających się na aplikację wymaga także czasu, możliwości spokojnego zastanowienia się nad strukturą klas, wcześniejszego zaprojektowania sobie wszystkiego (jeśli nie na papierze, to w głowie) itd. Co więcej, o ile inżynieria oprogramowania i zdrowy rozsądek jasno mówią, że należy dążyć do kodu o jak najwyższej jakości, to w praktyce i biznesie zdarzają się sytuacje, w których... perfekcja nie zawsze jest pożądana. Tym niemniej, warto się o nią starać "naturalnie", przy czym jest to wyzwanie i wymaga doświadczenia oraz wielu prób oraz porażek.

Stąd w programowaniu mamy do czynienia z pojęciem refaktoryzacji, które weźmiemy na tapet w dzisiejszym tekście. Dowiemy się, co to jest, co daje, jakie ma skutki uboczne, czy zawsze należy ją stosować, a także - co najważniejsze - pokażemy sobie rozbudowany przykład, krok po kroku, aby zobaczyć, dlaczego nie należy tej operacji robić "na hura". I właśnie ze względu na prezentację kodu ten tekst może być dość długi - lojalnie o tym ostrzegam, gdybyście chcieli podzielić sobie jego czytanie na części lub zrobić wcześniej herbatę.

Czym jest refaktoryzacja?

Refaktoryzacja to zmiana organizacji kodu źródłowego bez zmiany jego funkcjonalności. Nie jest to operacja, która dodaje nam funkcje aplikacji lub je odbiera. To takie przekształcenie kodu (a raczej seria przekształceń), które sprawia, że struktura programu jest lepsza, bardziej przejrzysta i łatwiejsza w rozszerzaniu. Oczywiście, po drodze może się okazać, że w ten sposób odkryjemy potencjalne problemy i je poprawimy, natomiast cele refaktoryzacji są związane z pielęgnacją źródeł oprogramowania, a nie z jego działaniem. Ten proces nie polega też np. na optymalizacji, choć ponownie - przy okazji może się okazać, że programista skróci kod i usunie niepotrzebne fragmenty spowalniające działanie.

Refaktoryzacja jest procesem, który składa się z szeregu przekształceń kodu. Są to różne operacje, które - często wspierane przez IDE - doprowadzają w końcu do tego, że program zaczyna wyglądać bardziej klarownie. Brzmi bardzo matematycznie, ale prawda jest taka, że wiele z tych kroków jest bardzo intuicyjnych i wynika z doświadczenia każdego programisty. Są to przykładowo:

  • wyekstrahowanie (wyciągnięcie) metody lub klasy,
  • przeniesienie metody, klasy lub pola,
  • zmiana nazwy metody, klasy lub pola,
  • dodanie, usunięcie lub zmiana parametrów metody,
  • przeniesienie metody lub pola do klasy bazowej lub potomnej,
  • usunięcie elementu z kodu,
  • ukrycie metody

i wiele innych. Istnieją opracowania, które omawiają tę tematykę bardziej dokładnie, jak np. ten artykuł autorstwa naukowców z Wydziału Informatyki Politechniki Poznańskiej.

Zalety refaktoryzacji

Pośrednio już je wymieniliśmy - za pomocą refaktoryzacji można osiągnąć:

  • większą przejrzystość kodu, a co za tym idzie - szybsze odnalezienie się w nim przez nową osobę,
  • większą podatność na rozszerzenie kodu o nowe funkcje, co ograniczy późniejszą rozpacz przy utrzymywaniu tzw. spaghetti code,
  • skrócenie kodu - nie zawsze jest to możliwe, ale często po zmianach okazuje się, że wiele operacji w systemie jest zduplikowanych i można utworzyć metodę, która je zgrupuje,
  • lepszy podział odpowiedzialności kodu, aby łatwiej można było znaleźć miejsce odpowiadające za daną funkcję,
  • usunięcie nieużywanego kodu,
  • większy zapał do dalszych prac - wyczyszczony kod odświeża umysł programistów i motywuje,
  • większą wiedzę o działaniu kodu,
  • reasumując wiele z powyższych punktów - pozbycie się brzydkich zapachów.

Jak widać, refaktoryzacja jest ogólnie pozytywnym zjawiskiem i pozwala utrzymać kod w dobrym stanie w dłuższym wymiarze czasowym. To trochę jak z dbaniem o samochód, który, co prawda, może nadal jeździć, ale jeśli zatroszczymy się o pielęgnację pewnych elementów, to wehikuł będzie bezproblemowo jeździł więcej lat niż mniej. Natomiast, jak wiadomo, nie ma róży bez kolców, więc także tutaj muszą pojawić się niekoniecznie pozytywne zjawiska związane z tym procesem.

Skutki uboczne refaktoryzacji

Przede wszystkim, refaktoryzacja zabiera czas i pozornie nie rozwija projektu. To takie programistyczne "zrobienie jednego kroku w tył, aby zrobić kilka kroków naprzód" i słusznie większość osób zauważy, że to nie są żadne wady, jeśli długofalowo doprowadzą do oszczędności. Niestety, tak jest w idealnym świecie, a w biznesowym nie zawsze się to sprawdza. Klienci chcą widzieć efekty, dalsze funkcje i nie są skorzy płacić za poprawki, zwłaszcza czegoś, co przecież działa. Przecież skoro programiści wiedzą, jak to skorygować, to powinni pisać dobrze od początku, prawda?

I tutaj dochodzimy do tego, że nawet najbardziej doświadczeni koderzy nie zawsze są w stanie od początku wyobrazić sobie najlepszą strukturę oraz ją wdrożyć. Powodów jest mnóstwo, ale najczęściej jest to wynik braku czasu (spowodowany np. potrzebą szybkiego wypuszczenia prototypu, który potem okazuje się trwały), początkowego braku wiedzy o wszystkich możliwościach oprogramowania (nie zawsze można od razu napisać elastyczny kod) czy różnego poziomu doświadczenia wykonawców.

Dlatego właśnie wymyślono refaktoryzację - gdyby od początku wszyscy pisali kod idealnie, żaden proces reorganizacji kodu nie byłby potrzebny. Należy to zaakceptować i odpowiednio wdrożyć w proces tworzenia oprogramowania, np. poprzez dzielenie ją na mniejsze części. Tzw. "wielka refaktoryzacja" (czyli reorganizacja całego kodu oprogramowania) prawie nigdy nie kończy się dobrze i zazwyczaj finałem jest napisaniem systemu od nowa, co jest drogie i stresujące. Zamiast tego lepiej zrobić to zgodnie z metodykami zwinnymi i refaktoryzować mniejsze fragmenty kodu, ale częściej. Dzięki temu zmian jest wprowadzenia jest mniej w jednym momencie i łatwiej je zrealizować.

Nie da się też ukryć, że o ile skutkiem refaktoryzacji jest poczucie czystego kodu, a co za tym idzie, zadowolenia programistów i czystości w projekcie, o tyle sam proces bywa żmudny i niewdzięczny. Przez to jest nielubiany przez zespół IT i czasami odkładany, a to rodzi problemy. Bywa też, że cały czas trwają prace rozwijające kod i trudno znaleźć dogodny moment, w którym należy zatrzymać realizację nowych wymagań oraz skupić się na modyfikacji kodu istniejących. Na koniec warto wspomnieć, że sama refaktoryzacja jest po prostu trudna i wymaga solidnej wiedzy o inżynierii oprogramowania oraz docelowej "wizji" systemu IT. To kolejny czynnik demotywujący i czasami odpychający te modyfikacje na dalszy plan.

Żebym nie został źle zrozumiany - refaktoryzacja jest potrzebna i powinna być wliczona w zadania projektowe. Problem w tym, że trudno uzasadnić ją biznesowo laikom, a także istnieje zbyt duża pokusa, aby z niej rezygnować i tym samym dopuścić do katastrofy w dłuższej perspektywie. Trzeba ją zatem realizować z głową.

Czy zawsze trzeba refaktoryzować do końca?

Wiemy już, że refaktoryzacja jest potrzebna - nigdy nie uda się zaplanować i zrealizować całego kodu idealnie od początku do końca i czasem trzeba zmodyfikować choćby małe fragmenty, aby dalszy rozwój był łatwiejszy lub po prostu możliwy. Nie zawsze muszą to być duże zmiany - czasami nawet delikatne korekty sprawią, że kod będzie znacznie lepszej jakości.

Należy jednak też rozwiać drugą wątpliwość - czy zawsze trzeba refaktoryzować kod do końca i czyścić go na 100%. I tutaj odpowiedź może być inna w zależności od osoby i dziedziny, którą się zajmuje. Pod kątem biznesowym nie zawsze jest to ani możliwe, ani wykonalne, ani opłacalne. Nie bez powodu istnieje powiedzenie "lepsze jest wrogiem dobrego", gdyż może okazać się, że znaczne zwiększenie jakości kodu jest możliwe po zastosowaniu już 20% proponowanych zmian, a pozostaje 80% czyni kod perfekcyjnym, ale zajmuje to tyle czasu, że lepiej to sobie odpuścić, gdyż staje się nierentowne. To zawsze trudne decyzje i rację mogą mieć perfekcjoniści, którzy nie uznają niczego poza ideałem. Tylko że osiągnięcie go jest możliwe przy ogromnym budżecie, a z tym rzadko spotykają się software house'y w swoich projektach. Wyobraźcie sobie, że płacicie mechanikowi za wstawienie nowego silnika do Waszego samochodu, a ten przedstawia dużą wycenę, uzasadniając to w ten sposób, że gdy pobrudzi maszynerię, to musi ją rozmontować i wyczyścić od środka, aby błyszczała.

Dlatego nie jest wcale naganne przyjęcie postawy osiągania kodu "wystarczająco dobrego" (ang. good enough), tzn. takiego, który już daje się polubić, ale jeszcze nie jest idealny. Pamiętajmy, że refaktoryzacja może być procesem iteracyjnym i nie jest wykluczone, aby do pewnych miejsc wrócić później, jeszcze poprawiając ich czytelność.

Jak należy refaktoryzować i na co zwracać uwagę?

Pierwszą i podstawową sprawą jest fakt, że refaktoryzacja nie może zmieniać funkcjonalności programu. To oznacza, że niedopuszczalne jest, aby ten proces wprowadził nowe błędy do aplikacji lub zmienił sposób działania widoczny z punktu widzenia użytkownika (chyba że na lepsze). Dlatego refaktoryzacja powinna:

  • być ostrożna,
  • przeprowadzana krok po kroku (tzn. konieczne jest weryfikowanie poprawności po każdym przekształceniu).

Z tego powodu programiści często wspomagają się przy niej testami automatycznymi i jest to też dobra okazja do ich napisania. Nie jest to temat tego tekstu i nie będziemy się nim teraz zajmowali, jednak warto zaznaczyć, że taki sposób weryfikacji bardzo wspomaga procesy refaktoryzacyjne (i nie tylko), choć również ma swoje konsekwencje.

Pozostaje jeszcze pytanie, na co zwracać uwagę przy analizie kodu, który zamierzamy refaktoryzować. Często jest bowiem tak, że podczas przeglądów widać na oko, że kod jest brzydki, ale nie zawsze jest jasne, w jaki sposób należy go poprawić w szczegółach. Dlatego warto weryfikować go pod kątem:

  • wykonywania podobne rzeczy w kilku miejscach (mimo że teoretycznie nie zawsze jest to niepożądane),
  • parametrów i wartości zwracanych przez funkcję,
  • często powtarzających się, długich lub skomplikowanych instrukcji warunkowych,
  • długości klas i metod,
  • przesłanek, które potencjalnie powodują złe zrozumienie przeznaczenia kodu,
  • zamkniętych warunków, przez które trudno będzie dodać np. nowy podtyp danego obiektu,
  • nieodpowiednie przypisanie metod lub pól do klas, częste odwoływanie się z jednej klasy do drugiej,
  • własnoręcznie napisanych funkcji, które stanowią element biblioteki standardowej
  • zgodności ze standardem obowiązującym w zespole.

Nie jest to pełna lista, ale za to dobry punkt startowy. Widać, że tak naprawdę wiele z tych punktów to przywoływane już wyżej brzydkie zapachy i faktycznie to temat związany ściśle z refaktoryzacją. Dodatkowo, wraz z nabieranym doświadczeniem, programiści mogą dojrzeć inne problemy, które będą wymagały zmian, także mając w perspektywie nadchodzący rozwój systemu lub nowe funkcje.

Przykład refaktoryzacji krok po kroku

Przejdźmy zatem do wspominanego już przykładu. Refaktoryzacja jest bowiem obiektem badań naukowców z obszaru inżynierii oprogramowania, jednak jak najbardziej to praktyczna umiejętność, która umożliwia programistom poprawianie jakości kodu, a tym samym całego oprogramowania. Warto zatem prześledzić, jak w rzeczywistości może wyglądać doprowadzanie programu do bardziej przejrzystej formy.

W chwili, gdy piszę te słowa, odbywają się Mistrzostwa Świata w Piłce Nożnej (potoczne zwane mundialem), w których, jak wiadomo, istnieje faza grupowa i faza pucharowa. Nas będzie interesowała pierwsza z tych faz, w której uczestniczą cztery drużyny grające ze sobą w systemie "każdy z każdym" i dwie z nich awansują do dalszego etapu rozgrywek. Jak nietrudno się domyślić, promocję uzyskują te dwie ekipy, które spiszą się najlepiej, a więc zdobędą największą liczbę punktów (3 za zwycięstwo, 1 za remis). Jednakże każdy kibic piłkarski (i nie tylko) zdaje sobie sprawę, że na przestrzeni meczów grupowych nierzadko zdarza się, że kilka drużyn zdobywa tyle samo "oczek" i wówczas ważne stają się kolejne kryteria awansu. Dodaje to emocji, ale także problemy osobom, które chcą aktywnie śledzić szanse drużyn na wyjście z grupy. Jak widać, przydałaby się do tego jakaś aplikacja, prawda?

I właśnie program ustalający kolejność drużyn w grupie na podstawie wyników będzie przedmiotem naszego zainteresowania. Na potrzeby tego tekstu przygotowaliśmy prosty skrypt w języku PHP, który po odczytaniu rezultatów meczów w formie JSON-a pokaże końcowy porządek. A jest on dosyć skomplikowany i został opisany zarówno w oficjalnym regulaminie, jak i w serwisach sportowych. W skrócie, kryteria kolejności zespołów są następujące:

  1. Liczba zdobytych punktów.
  2. Bilans bramek zdobytych i straconych.
  3. Liczba zdobytych bramek.
  4. Liczba zdobytych punktów w bezpośrednich meczach. Tutaj może warto wyjaśnić, czym jest tzw. mała tabela - jeśli sytuacja pomiędzy zespołami (najczęściej dwoma, ale może być więcej) jest nierozstrzygnięta, brane są pod uwagę mecze wyłącznie pomiędzy tymi ekipami. Krótko mówiąc - w tej sytuacji przewagę będzie miała drużyna, która wygrała bezpośrednie starcie.
  5. Bilans bramek w bezpośrednich meczach.
  6. Liczba zdobytych bramek w bezpośrednich meczach.
  7. Klasyfikacja fair play - im mniej kartek zdobędzie dany zespół, tym lepiej.

Jak widać, może to być zajmujące i dlatego mecze w trzeciej kolejce fazy grupowej (które są rozgrywane równocześnie) są takie emocjonujące. I właśnie jedną z grup w 2022 roku, gdzie sprawa awansu nie była pewna aż do ostatnich minut (w pewnym momencie nawet ważna okazywała się klasyfikacja fair play) była grupa C, w której grała Polska wraz z Argentyną, Meksykiem i Arabią Saudyjską. Rozgrywki pomiędzy tymi narodami będą zestawem testowym dla naszego skryptu. Oczywiście, zadziała on także dla innych grup i turniejów, w których obowiązują identyczne zasady, ale to grupa C będzie dla nas weryfikująca. Końcowa rzeczywista kolejność była następująca:

  1. Argentyna - 6 punktów, bilans bramek 5-2, -2 punkty klasyfikacji fair play
  2. Polska - 4 pkt., bilans 2-2, -5
  3. Meksyk - 4 pkt., bilans 2-3, -7
  4. Arabia Saudyjska - 3 pkt., bilans 3-5, -14

Schemat działania był następujący - na początku został napisany skrypt, który dawał dobre rezultaty (choć zawierał jeden błąd, o którym później), ale był "brzydki" i czuć, że nie poświęcono mu dużo czasu. Następnie został on zrefaktoryzowany krok po kroku, w taki sposób, aby nie tylko poprawić jakość kodu, ale także pokazać, jakie były kolejne decyzje i przekształcenia programisty. Całość możecie zobaczyć na repozytorium umieszczonym na naszym profilu na GitHubie - pierwszy commit zawiera oryginalny, "nieczysty" skrypt, późniejsze - kolejne zmiany, które omówimy krok po kroku. Listę znajdziecie tutaj, a podczas analizy należy przeglądać ją od dołu do góry (od najstarszego do najmłodszego).

Oryginalny skrypt

Początkowo program prezentował się następująco:

$matchesFile = $argv[1];
$matches = json_decode(file_get_contents($matchesFile), true);

$teams = sumResults();
$order = calculateOrder($teams);

$i = 1;
foreach($order as $idx) {
	$team = $teams[$idx];
	
	$name = $team['name'];
	$points = $team['points'];
	$scoredGoals = $team['scoredGoals'];
	$concededGoals = $team['concededGoals'];
	$fairPlay = $team['fairPlay'];
	
	echo ($i++).". $name - $points pkt., $scoredGoals-$concededGoals ($fairPlay)\n";
}

function sumResults($obligatorySymbols = []) {
	global $matches;

	$teams = [];
	foreach($matches as $match) {
		$a = $match['teamA'];
		$b = $match['teamB'];

		if(!empty($obligatorySymbols) && (!in_array($a['symbol'], $obligatorySymbols) || !in_array($b['symbol'], $obligatorySymbols))) {
			continue;
		}
		
		if(!array_key_exists($a['symbol'], $teams)) {
			$teams[$a['symbol']] = ['name' => $a['symbol'], 'points' => 0, 'scoredGoals' => 0, 'concededGoals' => 0, 'goalsBalance' => 0, 'fairPlay' => 0];
		}
		if(!array_key_exists($b['symbol'], $teams)) {
			$teams[$b['symbol']] = ['name' => $b['symbol'], 'points' => 0,'scoredGoals' => 0, 'concededGoals' => 0, 'goalsBalance' => 0, 'fairPlay' => 0];
		}
		
		if($a['goals'] > $b['goals']) {
			$teams[$a['symbol']]['points'] += 3;
		}
		elseif($a['goals'] == $b['goals']) {
			$teams[$a['symbol']]['points']++;
			$teams[$b['symbol']]['points']++;
		}
		else {
			$teams[$b['symbol']]['points'] += 3;
		}
		
		$teams[$a['symbol']]['scoredGoals'] += $a['goals'];
		$teams[$a['symbol']]['concededGoals'] += $b['goals'];
		$teams[$b['symbol']]['scoredGoals'] += $b['goals'];
		$teams[$b['symbol']]['concededGoals'] += $a['goals'];
		
		$teams[$a['symbol']]['goalsBalance'] = $teams[$a['symbol']]['scoredGoals'] - $teams[$a['symbol']]['concededGoals'];
		$teams[$b['symbol']]['goalsBalance'] = $teams[$b['symbol']]['scoredGoals'] - $teams[$b['symbol']]['concededGoals'];
		
		$teams[$a['symbol']]['fairPlay'] += ($a['yellowCards'] * -1) + ($a['doubleYellowCards'] * -3) + ($a['directRedCards'] * -4) + ($a['directYellowAndRedCards'] * -5);
		$teams[$b['symbol']]['fairPlay'] += ($b['yellowCards'] * -1) + ($b['doubleYellowCards'] * -3) + ($b['directRedCards'] * -4) + ($b['directYellowAndRedCards'] * -5);
	}

	return $teams;
}

function calculateOrder($teams, $verifyFairPlay = false) {
	$order = array_keys($teams);
	$smallTables = [];

	for($i = 0; $i < count($order) - 1; ++$i) {
		for($j = $i + 1; $j < count($order); ++$j) {
			if($i == $j) continue;
			
			$team1 = $teams[$order[$i]];
			$team2 = $teams[$order[$j]];
			
			if($team2['points'] > $team1['points']) {
				swapOrder($order, $i, $j);
			}
			elseif($team2['points'] == $team1['points']) {
				if($team2['goalsBalance'] > $team1['goalsBalance']) {
					swapOrder($order, $i, $j);
				}
				elseif($team2['goalsBalance'] == $team1['goalsBalance']) {
					if($team2['scoredGoals'] > $team1['scoredGoals']) {
						swapOrder($order, $i, $j);
					}
					elseif($team2['scoredGoals'] == $team1['scoredGoals']) {
						if($verifyFairPlay) {
							if($team2['fairPlay'] > $team1['fairPlay']) {
								swapOrder($order, $i, $j);
							}
						}
						else {
							$hash = md5($team2['points'].$team2['goalsBalance'].$team2['scoredGoals']);
							if(!array_key_exists($hash, $smallTables)) {
								$smallTables[$hash] = [];
							}
							
							$smallTables[$hash][] = $team1['name'];
							$smallTables[$hash][] = $team2['name'];
						}
					}
				}
			}
		}
	}
	
	if(!empty($smallTables)) {
		foreach($smallTables as $hash => $table) {
			$subTeams = sumResults($table);
			$newOrder = calculateOrder($subTeams, true);
			$smallTables[$hash] = $newOrder;
		}
	}
	
	$result = [];
	$visitedTeams = [];
	for($i = 0; $i < count($order); ++$i) {
		if(in_array($order[$i], $visitedTeams)) continue;
		
		$smallTableIncludingThisTeam = null;
		foreach($smallTables as $smallTable) {
			if(in_array($order[$i], $smallTable)) {
				$smallTableIncludingThisTeam = $smallTable;
				break;
			}
		}
		
		if(!empty($smallTableIncludingThisTeam)) {
			foreach($smallTableIncludingThisTeam as $team) {
				if(in_array($team, $visitedTeams)) continue;
				
				$result[] = $team;
				$visitedTeams[] = $team;
			}
		}
		else {
			$result[] = $order[$i];
			$visitedTeams[] = $order[$i];
		}
	}
	
	return $result;
}

function swapOrder(&$array, $i, $j) {
	list($array[$i], $array[$j]) = [$array[$j], $array[$i]];
}

Warto na początku wyjaśnić sobie, jak ten kod działa. Przyjmuje on w parametrze plik JSON z meczami do wczytania, a następnie na jego podstawie zlicza wszystkie wyniki, bramki i określa wskaźnik fair play. W ten sposób powstaje tablica drużyn (w tym przypadku 4-elementowa) z rezultatami. Następnie algorytm ustala kolejność, a więc sortuje nasze drużyny według przedstawionych wyżej kryteriów. Po drodze może się okazać, że niektóre zespoły muszą zostać sprawdzone pod kątem bezpośrednich meczów w małej tabeli, co oznacza jeszcze raz wczytanie tylko pewnych meczów (pamiętajmy, że drużyn w małej tabeli teoretycznie może być więcej niż dwie) i przeliczenie jej kolejności. Wówczas, mając kolejność z dużej i małych tabel, po kolei zwracane są symbole drużyn. Na samym końcu wypisywany jest wynik.

Jak już wcześniej wspomniałem, ten kod pozwala uzyskać poprawny rezultat (poza jednym błędem, który ujawni się później). Jednak już na oko widać, że nie jest to coś, czym byśmy chcieli się pochwalić. Nie chodzi nawet o brak obiektowości, gdyż choć będziemy do niej dążyć, to nie zawsze jest ona potrzebna. Natomiast tutaj widać, że kod jest po prostu zbyt długi, zawiły, bardzo polega na tablicach asocjacyjnych i polach wpisywanych z ręki (co może doprowadzić do pomyłek), a fragment przestawiający kolejność zespołów w zależności od warunków może być nie tylko trudny do zrozumienia, ale wręcz powodować koszmary w nocy. Będziemy to wszystko modyfikować. Warto jeszcze pokazać, jak wygląda przykładowy plik wejściowy w formacie JSON.

[
	{
		"teamA": {
			"symbol": "ARG",
			"goals": 1,
			"yellowCards": 0,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		},
		"teamB": {
			"symbol": "SA",
			"goals": 2,
			"yellowCards": 6,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		}
	},
	{
		"teamA": {
			"symbol": "MEX",
			"goals": 0,
			"yellowCards": 2,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		},
		"teamB": {
			"symbol": "POL",
			"goals": 0,
			"yellowCards": 1,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		}
	},
	{
		"teamA": {
			"symbol": "POL",
			"goals": 2,
			"yellowCards": 3,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		},
		"teamB": {
			"symbol": "SA",
			"goals": 0,
			"yellowCards": 2,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		}
	},
	{
		"teamA": {
			"symbol": "ARG",
			"goals": 2,
			"yellowCards": 1,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		},
		"teamB": {
			"symbol": "MEX",
			"goals": 0,
			"yellowCards": 4,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		}
	},
	{
		"teamA": {
			"symbol": "SA",
			"goals": 1,
			"yellowCards": 6,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		},
		"teamB": {
			"symbol": "MEX",
			"goals": 2,
			"yellowCards": 1,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		}
	},
	{
		"teamA": {
			"symbol": "POL",
			"goals": 0,
			"yellowCards": 1,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		},
		"teamB": {
			"symbol": "ARG",
			"goals": 2,
			"yellowCards": 1,
			"doubleYellowCards": 0,
			"directRedCards": 0,
			"directYellowAndRedCards": 0
		}
	}
]

Mała uwaga - słusznie możecie zauważyć, że o ile Polska z Meksykiem mają równą liczbę punktów, to już ich bilans bramkowy jest inny, co sprawia, że być może przypadek tej grupy nie jest tak atrakcyjny w liczeniu, jak początkowo mogłoby się wydawać. Dlatego podczas testowania warto wprowadzić chwilowe małe modyfikacje w wynikach, aby zweryfikować również inne przypadki:

  • zmiana wyniku meczu Arabii Saudyjskiej z Meksykiem z 1-2 na 0-2 sprawia, że Meksyk i Polska mają identyczny bilans bramek i liczbę strzelonych, a że mecz tych ekip skończył się 0-0, to można sprawdzić liczenie klasyfikacji fair play,
  • zmiana wyniku tego samego meczu z 1-2 na 1-3 sprawia, że Meksyk wyprzedzi Polskę, gdyż mimo identycznego bilansu bramek ma ich więcej strzelonych,
  • jeśli dodamy Polsce parę żółtych kartek i ustawimy wynik meczu Arabii Saudyjskiej z Meksykiem na 0-2, to również Meksyk wyprzedzi Polskę.

Uruchomienie skryptu sprowadza się do prostego polecenia w konsoli: php index.php matches.json.

Będziemy teraz wykonywali po kolei przekształcenia refaktoryzacyjne, które spowodują, że kod stanie się optycznie krótszy (tzn. jeden plik nie będzie tak długi lub ulegnie logicznemu podziałowi), czytelniejszy, zostawi mniejsze możliwości popełnienia błędu przez programistę oraz zostanie przygotowany na dalszy rozwój. Zaczynamy.

Krok nr 1 - wprowadzenie obiektowości (OOP)

Tak, jak już wspomniałem, obiektowość nie jest wymagana do tego, aby uznać kod za ładny. Co więcej, w przypadku krótkich kodów źródłowych nie ma dużego sensu (o ile nie jest konieczna ze względu na sam język programowania). Pozwala jednak wydzielić wspólne paczki danych i przybliżyć strukturę do logicznego rozumowania przy wyobrażaniu sobie kolejnych operacji. Nie da się też ukryć, że odpowiednio użyta może uprościć proces myślowy u kogoś, kto dopiero poznaje aplikację od środka.

W naszym przypadku istnieje jeszcze jeden powód wprowadzenia obiektowości. Co prawda, cały czas mówimy o skrypcie uruchamianym z konsoli, ale łatwo wyobrazić sobie (zwłaszcza, że to PHP), że na bazie tego kodu mogłaby powstać aplikacja webowa, która zwróci kolejność drużyn poprzez wystawione API. Sama logika będzie identyczna, jednak sposób uruchomienia - zupełnie inny. Dlatego przeniesiemy kod do nowej klasy Group, z której będą wywoływane operacje i która w przyszłości mogłaby tworzyć obiekty także w aplikacji niekonsolowej.

Commit ze zmianami w kroku nr 1: https://github.com/WildaSoftware/FootballGroupCalculator/commit/4e889922de4dd001584e3e255972cdd2bd51dca1

Krok nr 2 - wprowadzenie klasy zespołu (Team)

Gdy popatrzymy na metodę sumResults, to zobaczymy mocne wykorzystanie tablic asocjacyjnych, które w PHP są bardzo łatwo dostępne. Są przez to też bardzo użyteczne, natomiast podawanie klucza do uzyskania konkretnej wartości może powodować błędy wynikające ze zwykłych literówek. Oczywiście, można sobie z tym poradzić przez stałe, ale nadal pozostaje przydługi, bardzo "szeroki" kod. Ponadto, nie wykorzystujemy jednej z wielu przewag edytorów do programowania - podpowiadania elementów klasy, co znakomicie przyspiesza pracę specjalisty. Dodając do tego fakt, że przy nieco obszerniejszym kodzie naprawdę łatwo stracić panowanie nad tym, jakie pola ma każdy zespół, podejmujemy decyzję o wprowadzeniu klasy Team, która nam to uporządkuje, a w dalszym procesie pozwoli "przygarnąć" część metod.

W poniższym przekształceniu widzimy, że udało się nie tylko trochę wyklarować kod w sumResults, ale także skrócić printResults. Przy okazji, możemy zobaczyć, że wcale nie trzeba podawać wszystkich atrybutów klasy w postaci standardowych pól, zwłaszcza, jeśli jest ich dużo - jeśli stosujemy się do zasad obiektowej enkapsulacji, to nagle klasa Team stałaby się bardzo długa przez same settery i gettery. Alternatywą jest zrezygnowanie z hermetyzacji i upublicznienie wszystkich pól i odwoływanie się do nich bezpośrednio, ale wówczas programista traci pełną kontrolę nad tym, kiedy zmienia się dana wartość. Na szczęście, PHP udostępnia zestaw magicznych funkcji, wśród których są __set oraz __get, obejmujące wszystkie pola klasy. Co prawda, tracimy w ten sposób podpowiadanie, ale to można "naprawić" poprzez specjalnie komentarze nad definicją klasy.

class Team {

    private $attributes = [];

    public function __construct(string $symbol) {
        $this->symbol = $symbol;
    }

    public function __set($name, $value) {
        if(!in_array($name, ['symbol', 'points', 'scoredGoals', 'concededGoals', 'goalsBalance', 'fairPlay'])) {
            throw new \Exception("There is no \"$name\" attribute to set in Team");
        }

        $this->attributes[$name] = $value;
    }

    public function __get($name) {
        return $this->attributes[$name] ?? null;
    }
}

W ten sposób można wywołać $team->points = 3 czy $goals = $team->scoredGoals bez jawnego definiowania i używania metod typu "set" lub "get". Podobny mechanizm działa w C#, gdzie można zabezpieczyć klasę przed próbą dostania się do niewłaściwego pola, ale także tutaj możemy to zrobić poprzez wylistowanie dopuszczalnych atrybutów. Najważniejsze jednak, że wyodrębniliśmy klasę zespołu, co pozwala lepiej zgrupować dotyczące go elementy i odciążyć inne fragmenty programu.

Commit ze zmianami w kroku nr 2: https://github.com/WildaSoftware/FootballGroupCalculator/commit/f73779ab223a328a6c07938ae4eece1d2df98104

Krok nr 3 - obiekt DTO, poprawienie konwencji nazewnictwa w sumResults i wyodrębnienie obliczeń fair play

Wczytując JSON-a z pliku, robiliśmy to "ręcznie", tzn. nie kontrolując do końca, jakie pola odczytamy i z których możemy potem korzystać. Tutaj znowu używaliśmy tablic asocjacyjnych, więc dobrze będzie zainteresować się obiektowością. W tym celu tworzymy klasy MatchDTO oraz TeamDTO - co prawda, w jednym pliku, ale powiedzmy, że to jest właśnie takie niegroźne odchylenie od perfekcji, a w zamian nieco przyspieszające cały proces. Ogólnie klasy typu DTO rozwija się jako Data Transfer Object i, jak sama nazwa mówi, służą do przenoszenia danych. To właśnie w MatchDTO nastąpi wczytanie rezultatów danego meczu i zapisanie ich w celu późniejszego wykorzystania. Dodatkowo zmieniamy name na symbol, dostosowując się do konwencji z pliku JSON, ale to akurat szczegół.

Wracając do sumResults, gdzie oczywiście korzystamy z nowego "wynalazku" - widzimy też, że bardzo często występują wywołania $a['symbol'] oraz $b['symbol'], w dodatku jako indeksy w tablicach asocjacyjnych. Z uwagi na częste występowanie i "rozpychanie" kodu, uznajemy, że warto je skrócić do zmiennych o krótkich nazwach $a oraz $b. Dzięki temu zabiegowi kod wygląda bardziej przejrzyście. Co więcej, zauważamy, że nazwa zmiennej obligatorySymbols może być myląca i niekoniecznie zawsze odczytywana jako lista jedynych symboli, które nas interesują w danym przebiegu (wykorzystuje się to do zliczania wyników dla małej tabeli). Dlatego zmieniamy nazwę na restrictedTeamSymbols - jest ona dłuższa, ale dzięki IDE nie jest to dla nas problemem, a lepiej "mówi", do czego służy ten parametr.

Na koniec, zauważamy, że procedura liczenia punktów fair play w zależności od uzyskanych kartek nie tylko zajmuje trochę miejsca, jest zduplikowana, ale też trudno później będzie znaleźć odpowiedni kod, jeśli przyjdzie nam zmieniać wartości. Dlatego, korzystając z tego, że te punkty liczone są już przy wczytywaniu danych z pliku, przenosimy obliczenia do TeamDTO i odpowiednio formatujemy kod, aby łatwo dostrzec, za co jest ile "oczek". Teoretycznie powinniśmy wyciągnąć wagi kartek do osobnych stałych lub parametrów, aby programiści łatwo mogli je potem zmieniać. Jednak już na ten moment jest to bardziej przejrzyste i w przyszłości odpowiednia zmiana będzie prosta do wprowadzenia.

Commit ze zmianami w kroku nr 3: https://github.com/WildaSoftware/FootballGroupCalculator/commit/47ab8cf30dde253e81497ea638262c9008572e4e

Krok nr 4 - streszczenie sumResults i lepsza kontrola wczytywanego pliku

Patrząc na sumResults po zmianach, nadal można zobaczyć, że coś tutaj jest do poprawy. Zbyt podobne są do siebie linijki dodające strzelone i stracone gole, a także ich bilans - wszystko występuje podwójnie, a czasami wydaje się, że nawet poczwórnie. Z drugiej strony, dysponujemy już klasą Team, która być może powinna liczyć to sama. Zatem tak zróbmy - przenieśmy ten kod do Team->addMatchResult, podając przy tym, z perspektywy którego zespołu liczymy wynik - jako pierwszy (A) czy drugi (B). W dodatku możemy zobaczyć, że kod znacznie się skrócił i zamiast 18 linijek, który były dotychczas, sumowanie wartości zajmuje 10 i stał się bardziej przejrzysty.

W tym momencie niektórzy mogą się słusznie zastanawić, czy nie jest zbyt łatwo pomylić się mając klasy o nazwach TeamDTO oraz Team. To prawda, można mieć takie wrażenie, ale ten "DTO" tutaj dużo zmienia i dlatego czasami układa się klasy w ten sposób. Aczkolwiek nie jest to jedyne rozwiązanie i można to jeszcze bardziej od siebie odróżnić.

// metoda klasy Team
public function addMatchResult(TeamDTO $myTeam, TeamDTO $opponent) {
	if($myTeam->goals > $opponent->goals) {
		$this->points += 3;
	}
	elseif($myTeam->goals == $opponent->goals) {
		$this->points++;
	}
	
	$this->scoredGoals += $myTeam->goals;
	$this->concededGoals += $opponent->goals;
	
	$this->goalsBalance = $this->scoredGoals - $this->concededGoals;
	
	$this->fairPlay += $myTeam->calculateFairPlay();
}

Dodatkowo, zauważmy, że program zachowa się bardzo źle przy podaniu nieistniejącego pliku lub przy pominięciu tego argumentu. Dlatego odpowiednio zabezpieczamy konstruktor klasy Group przyjmujący ten plik oraz sam plik wejściowy index.php.

Commit ze zmianami w kroku nr 4: https://github.com/WildaSoftware/FootballGroupCalculator/commit/c3329c5b9c80d9d942b9fdb1df596ce4b58e8bd9

Krok nr 5 - utworzenie obiektu tabeli

Skoro sumResults mamy w miarę wyczyszczone, zabieramy się za calculateOrder Już na oko widzimy, że tutaj czeka nas dużo modyfikacji, ale chwilowo zostawmy główną pętlę i zajmijmy się tym, że metoda zbiera dane nie zespołów, tylko ich symboli. Przez to jest dość dużo problemów, konwersji, szukania obiektu drużyny itd. Krótko mówiąc - gdy ktoś pierwszy raz spojrzy na ten kod, to bez dodatkowych komentarzy trudno będzie mu się odnaleźć w tym galimatiasie. Czy nie można zatem utrzymywać jednej tablicy (obiektów klasy Team) i to na niej działać? Można i to zrobimy, ale po kolei.

Problemem jest to, że oprócz liczenia dużej tabeli, mamy też rekurencyjne wywoływanie sortowania dla małych tabel (może być ich wiele), co też trzeba uwzględnić. Pewnym rozwiązaniem byłoby rozdzielenie obliczeń dla poszczególnych typów tabel, ale nie ma to do końca sensu, gdyż te kalkulacje byłyby bardzo do siebie podobne, przez co doprowadzilibyśmy do duplikacji kodu na dużą skalę. Dlatego działamy na tym samym calculateOrder i spróbujemy coś tutaj zrobić. Intuicyjnie czujemy, że można pozbyć się tutaj tablic symbolów, jednak nie od razu widać jak. A zatem spokojnie, krok po kroku, zacznijmy od czegoś, co może nie od razu się ich pozbędzie, ale raczej zgrupuje - wprowadźmy klasę tabeli TeamTable.

Korzystanie z listy zespołów przechowywanych w obiekcie tabeli oznacza również, że to do tej klasy możemy przenieść operację zamiany kolejności oraz tam bezpośrednio sortować drużyny. To właśnie powoli zrobimy, a przy okazji wprowadzamy metodę porządkującą tabelę, specjalnie dla bilansów bezpośrednich starć.

Niestety, to wszystko powoduje, że zmian jest trochę więcej i są głębsze niż w poprzednich krokach, ale należało to zrobić, aby przekształcenie było spójne i dało dobry wynik.

Commit ze zmianami w kroku nr 5: https://github.com/WildaSoftware/FootballGroupCalculator/commit/069c9440f1746b456430a857136afbceceea5ade

Krok nr 6 - dalsze przenoszenie sortowania do TeamTable

Poprzedni krok pokazał nam, że TeamTable powinno zostać jeszcze bardziej wykorzystane. Doprowadzamy w końcu do tego, żeby calculateOrder zwracał nie listę symboli, ale obiekt tabeli. Dodatkowo, mając sytuację, w której nie musimy osobno tworzyć listy drużyn w odpowiedniej kolejności, tylko tworzy się ona naturalnie podczas sortowania, znacznie upraszcza się końcowy foreach, w którym naszą kolejność modyfikujemy tylko o małe tabele. W związku z tym powstaje metoda TeamTable->reorderPartByTable, która... w sumie to robi to samo, co wcześniej działo się w Group, ale w bardziej zorganizowany sposób. Przykładowo, pozbywamy się listy odwiedzonych węzłów, zastępując ją zbieraniem wszystkiego, a na końcu usunięciem powtórzeń poprzez array_unique, wykorzystując przy tym narzędzia dane nam przez język PHP.

Mając tabelę jako wynik, a nie listę symboli, możemy zmodyfikować metodę printResults. Widzimy tutaj, że prawdopodobnie zaraz będzie okazja przeniesienia tego elementu do innej klasy, ale tak, jak wcześniej pisaliśmy - przekształcenia refaktoryzacyjne wykonujemy po kolei.

Poprawiamy również jeden błąd, który pojawił się w trakcie - w TeamTable->getTeamBySymbol iterowaliśmy po złej tablicy. Niestety, to się czasem zdarza i tutaj zawiodło niedostateczne testowanie. Widać, że przydałyby się automatyczne testy jednostkowe.

Commit ze zmianami w kroku nr 6: https://github.com/WildaSoftware/FootballGroupCalculator/commit/f7c89d497e70f76ec85f187e72456590a44a68a0

Krok nr 7 - skrócenie kodu czyszczenia sortowania

Po poprzedniej operacji widzimy, że dwie pętle porządkujące kolejność na podstawie małych tabel są dość podobne do siebie i mogą zostać scalone. Po analizie można dostrzec, że wywołanie $smallTables[$hash]->reorderBySymbols staje się niepotrzebne, gdyż już samo calculateOrder gwarantuje nam dobrą kolejność (zakładamy, że mała tabela nie ma w sobie dalszych małych tabel). Co więcej, jako że reorderBySymbols stało się elementem TeamTable i jest uruchamiane tylko tam, możliwe staje się ukrycie tej metody i uczynienie jej prywatną.

Commit ze zmianami w kroku nr 7: https://github.com/WildaSoftware/FootballGroupCalculator/commit/7109b3b2f1c78b57502f76e19cb2fccbf575e81a

Krok nr 8 - przeniesienie metody do wyświetlania tabeli

Tak, jak napomknęliśmy w kroku nr 6 - metoda printResults jako argument przyjmuje obiekt TeamTable i wykorzystuje tylko ten, z pominięciem elementów klasy, do której jest przypisana, a więc Group. Jest to typowy brzydki zapach nazywany feature envy i naprawa polega na zwyczajnym przeniesieniu metody do tej "interesującej" klasy. To robimy właśnie z printResults, przy okazji zmieniając jej nazwę na print. Przy okazji musimy zmienić wszystkie powiązane linijki kodu.

Commit ze zmianami w kroku nr 8: https://github.com/WildaSoftware/FootballGroupCalculator/commit/30982a2ed34d65dfe4e2fbd5c093e2f21003f606

Krok nr 9 - naprawienie błędu z liczeniem kartek

Wspominałem o tym, że refaktoryzacja nie naprawia błędów funkcjonalnych, ale może tak wyczyścić kod, że ujawni ich istnienie i pozwoli łatwiej się ich pozbyć. W ten sposób odkryliśmy buga, który polegał na dodatkowym naliczaniu kartek drużynom, które są zestawiane w małej tabeli. Przez to, że wyniki bezpośrednich starć pomiędzy tymi zespołami są liczone jeszcze raz, powieliły się też wyniki punktacji fair play, ale tylko z tych spotkań. Naprawiamy to poprzez globalne wskaźniki.

Po zastanowieniu się, można dostrzec, że tak naprawdę wprowadzamy mały bałagan - użycie dodatkowej tablicy fair play nie wygląda jak pożądane działanie refaktoryzacyjne. Tym niemniej, inne podejście do naprawy tego buga zostawiam Wam, drodzy czytelnicy.

Commit ze zmianami w kroku nr 9: https://github.com/WildaSoftware/FootballGroupCalculator/commit/d346ee8e43a7dd5bb17561761ecc904d14668e03

Krok nr 10 - przesunięcie metod sumResults i calculateOrder do TeamTable

Patrząc jeszcze bliżej, możemy dostrzec, że przecież dwie główne metody klasy Group również w większości operują na obiekcie klasy TeamTable Przenieśmy je zatem tam. Niestety, tutaj już widać zły wpływ decyzji z kroku nr 9, gdyż musimy dać możliwość odczytania tabeli fair play, a co za tym idzie - podać referencję do swojego obiektu i zostawić dodatkowe sumResults u siebie. Tym niemniej, chwilowo z tym możemy żyć, ale zostawiam to jako nauczkę.

Commit ze zmianami w kroku nr 10: https://github.com/WildaSoftware/FootballGroupCalculator/commit/19b9e457f5ede29d1eb127a4e4bb125660d6d715

Krok nr 11 - usunięcie flagi decydującej, czy liczone jest fair play

Wielu osobom nie spodoba się metoda calculateOrder także z tego powodu, że przyjmuje pojedynczy parametr typu prawda/fałsz, który steruje przepływem w określonym momencie. Co prawda, uważam, że trzeba mieć zdrowy rozsądek i czasami taka konstrukcja wiele ułatwia, jednak jeśli jest możliwe pozbycie się takiego argumentu, to warto to zrobić. Zwłaszcza, że liczenie fair play zachodzi tylko wówczas, gdy sprawdzenie małej tabeli nie da rozstrzygnięcia, a więc mamy sytuację, w której do calculateOrder algorytm wchodzi dla tabeli, która jest elementem innej tabeli.

Wykorzystajmy to i uzupełnijmy tabelę nadrzędną w przypadku liczenia zestawienia bezpośrednich starć - co prawda, nie pozbyliśmy się ifa, ale metoda wygląda ciut lepiej.

Commit ze zmianami w kroku nr 11: https://github.com/WildaSoftware/FootballGroupCalculator/commit/bb5126d6a751f81a9eb6d3ed5e77281dae1d4404

Krok nr 12 - robimy coś w końcu z tym porównywaniem

Na deser zostawiliśmy coś, co już z daleka pachnie potrzebą refaktoryzacji. Drabinka instrukcji warunków z powtarzającym się wywołaniem swapOrder oraz zagnieżdżonymi ifami sprawdzającymi równość może i działa, ale wygląda fatalnie. Dodatkowo, zmiana w regulaminie ustalania miejsc staje się trudna do wprowadzenia w programie. Co możemy z tym zrobić?

Patrząc szerzej, widzimy też zastosowany tutaj algorytm sortowania bąbelkowego, który jest swoistym paradoksem - nie jest ani intuicyjny, ani szybki, a jednak często od razu przychodzi na myśl programiście, który ma własnoręcznie napisać porządkowanie. Na razie to zostawmy, gdyż mimo, że PHP ma swoje funkcje do sortowania tablic, to jeszcze nie do końca może być jasne, jak je tutaj możemy wykorzystać. Skupmy się na samym warunkach.

Wiedza o mechanizmach sortowania w językach programowania pozwala nam stwierdzić, że zastosowanie tutaj mogą mieć tzw. funkcje porównawcze lub wręcz całe klasy komparatorów. Zazwyczaj sprawdzają one poszczególne warunki, zwracając wartość -1 (gdy lewy obiekt powinien być wcześniej w porządku), 1 (w odwrotnej sytuacji) lub 0 (gdy elementy są równe). Tutaj zrobimy coś podobnego i wprowadzimy klasę TeamComparator, która zawiera metody poświęcone poszczególnym kryteriom - punktom, bilansowi, strzelonym bramkom i klasyfikacji fair play. Zauważmy, że ponieważ dla małej tabeli warunki są prawie identyczne, nie ma potrzeby pisania ich na nowo.

class TeamComparator {

    private $teamTable;

    public function __construct(TeamTable $teamTable) {
        $this->teamTable = $teamTable;
    }

    public function comparePoints(Team $team1, Team $team2): int {
        if($team2->points == $team1->points) {
            return 0;
        }

        return $team2->points > $team1->points ? 1 : -1;
    }

    public function compareGoalsBalance(Team $team1, Team $team2): int {
        if($team2->goalsBalance == $team1->goalsBalance) {
            return 0;
        }

        return $team2->goalsBalance > $team1->goalsBalance ? 1 : -1;
    }

    public function compareScoredGoals(Team $team1, Team $team2): int {
        if($team2->scoredGoals == $team1->scoredGoals) {
            return 0;
        }

        return $team2->scoredGoals > $team1->scoredGoals ? 1 : -1;
    }

    public function compareFairPlayResults(Team $team1, Team $team2): int {
        $fp1 = $this->teamTable->getGroup()->getTeamFairPlayResultBySymbol($team1->symbol);
        $fp2 = $this->teamTable->getGroup()->getTeamFairPlayResultBySymbol($team2->symbol);

        if($fp2 == $fp1) {
            return 0;
        }

        return $fp2 > $fp1 ? 1 : -1;
    }
}

Taka forma komparatora to krok pośredni - widzimy, że o ile drabinka instrukcja warunkowych pozostała, to kod wydaje się znacznie przyjemniejszy. To jednak preludium do kolejnego kroku.

Commit ze zmianami w kroku nr 12: https://github.com/WildaSoftware/FootballGroupCalculator/commit/f35f03ffc3c53fa6b385fe6923680a74347c5826

Krok nr 13 - elastyczny komparator i skrócenie kodu

Teraz poznamy moc TeamComparator - wykorzystując fakt, że poza momentem z fair play kryteria porządku są te same i podawane w tej samej kolejności dla dużej i małej tabeli, napiszmy jedną metodę compare, która zbierze te warunki i - co więcej - umożliwi łatwą zmianę ich kolejności w przyszłości. Wykorzystamy tutaj kolejną charakterystyczną cechę PHP, a mianowicie mechanizm refleksji, czyli wywoływanie kodu za pomocą nazw. W ten sposób możemy wywoływać metody związane z kolejnymi kryteriami w pętli i za każdym razem sprawdzać, czy już można zatrzymać ten proceder, bo wynik jest rozstrzygnięty. Oczywiście, refleksja ma swoje wady, ale w niektórych przypadkach pozwala znakomicie uprzyjemnić pisanie kodu.

class TeamComparator {

    private $teamTable;

    public function __construct(TeamTable $teamTable) {
        $this->teamTable = $teamTable;
    }

    public function compare(Team $team1, Team $team2): int {
        $conditions = ['points', 'goalsBalance', 'scoredGoals'];
        $conditionsForSmallTable = ['fairPlayResults'];

        $result = 0;
        foreach($conditions as $condition) {
            $methodName = 'compare'.ucwords($condition);
            $result = $this->{$methodName}($team1, $team2);

            if(!empty($result)) {
                return $result;
            }
        }

        if(!empty($this->teamTable->getParentTable())) {
            foreach($conditionsForSmallTable as $conditionSt) {
                $methodName = 'compare'.ucwords($conditionSt);
                $result = $this->{$methodName}($team1, $team2);
    
                if(!empty($result)) {
                    return $result;
                }
            }
        }

        return $result;
    }

    private function comparePoints(Team $team1, Team $team2): int {
        if($team2->points == $team1->points) {
            return 0;
        }

        return $team2->points > $team1->points ? 1 : -1;
    }

    private function compareGoalsBalance(Team $team1, Team $team2): int {
        if($team2->goalsBalance == $team1->goalsBalance) {
            return 0;
        }

        return $team2->goalsBalance > $team1->goalsBalance ? 1 : -1;
    }

    private function compareScoredGoals(Team $team1, Team $team2): int {
        if($team2->scoredGoals == $team1->scoredGoals) {
            return 0;
        }

        return $team2->scoredGoals > $team1->scoredGoals ? 1 : -1;
    }

    private function compareFairPlayResults(Team $team1, Team $team2): int {
        $fp1 = $this->teamTable->getGroup()->getTeamFairPlayResultBySymbol($team1->symbol);
        $fp2 = $this->teamTable->getGroup()->getTeamFairPlayResultBySymbol($team2->symbol);

        if($fp2 == $fp1) {
            return 0;
        }

        return $fp2 > $fp1 ? 1 : -1;
    }
}

Dodatkowo, ponieważ reszta warunków nie musi być już widoczna na zewnątrz, możemy zmienić widoczność metod typu comparePoints na private. Jednak prawdziwa rewolucja czeka nas w calculateOrder - tak wyglądało to wcześniej:

public function calculateOrder() {
	// ...

	for($i = 0; $i < count($this->teams) - 1; ++$i) {
		for($j = $i + 1; $j < count($this->teams); ++$j) {
			if($i == $j) {
				continue;
			}
			
			$team1 = $this->teams[$i];
			$team2 = $this->teams[$j];
			$comparator = new TeamComparator($this);
			
			if($comparator->comparePoints($team1, $team2) > 0) {
				$this->swapOrder($i, $j);
			}
			elseif($comparator->comparePoints($team1, $team2) == 0) {
				if($comparator->compareGoalsBalance($team1, $team2) > 0) {
					$this->swapOrder($i, $j);
				}
				elseif($comparator->compareGoalsBalance($team1, $team2) == 0) {
					if($comparator->compareScoredGoals($team1, $team2) > 0) {
						$this->swapOrder($i, $j);
					}
					elseif($comparator->compareScoredGoals($team1, $team2) == 0) {
						if(!empty($this->parentTable)) {
							if($comparator->compareFairPlayResults($team1, $team2) > 0) {
								$this->swapOrder($i, $j);
							}
						}
						else {
							$hash = md5($team2->points.$team2->goalsBalance.$team2->scoredGoals);
							if(!array_key_exists($hash, $smallTables)) {
								$smallTables[$hash] = new TeamTable($this->group, $this);
							}
							
							$smallTables[$hash]->addTeam($team1);
							$smallTables[$hash]->addTeam($team2);
						}
					}
				}
			}
		}
	}
	
	// ...
}

A tak wygląda teraz:

public function calculateOrder() {
	// ...

	for($i = 0; $i < count($this->teams) - 1; ++$i) {
		for($j = $i + 1; $j < count($this->teams); ++$j) {
			if($i == $j) {
				continue;
			}
			
			$team1 = $this->teams[$i];
			$team2 = $this->teams[$j];
			$comparator = new TeamComparator($this);

			$compareResult = $comparator->compare($team1, $team2);
			if($compareResult > 0) {
				$this->swapOrder($i, $j);
			}
			elseif($compareResult == 0) {
				$hash = md5($team2->points.$team2->goalsBalance.$team2->scoredGoals);
				if(!array_key_exists($hash, $smallTables)) {
					$smallTables[$hash] = new TeamTable($this->group, $this);
				}
				
				$smallTables[$hash]->addTeam($team1);
				$smallTables[$hash]->addTeam($team2);
			}
		}
	}
	
	// ...
}

Trzeba przyznać, że jest poprawa.

Commit ze zmianami w kroku nr 13: https://github.com/WildaSoftware/FootballGroupCalculator/commit/fca9ffcb14bdfe261749c2d7be6c41e1d576105b

Czy to koniec refaktoryzacji?

Nie i co więcej - programiści analizujący powyższy proces na pewno stwierdzili w paru momentach "ja bym to zrobił inaczej". Choćby krok nr 9 może zostać przeprowadzony inaczej lub można zamienić sortowanie bąbelkowe na inny algorytm. Refaktoryzacja nie musi być identyczna w każdym przypadku - tutaj warto korzystać ze swojego doświadczenia, ale też dostosować działania do kontekstu, rodzaju projektu, warunków biznesowych, przyzwyczajeń zespołu (o ile są dobre) itd. Sami widzieliście, że po drodze wprowadziłem kontrowersyjną tablicę dla wyników fair play, podczas gdy zapewne można było to rozwiązać inaczej. Można też było wykonywać dalsze przekształcenia i stworzyć wręcz perfekcyjny kod. Ale nie zawsze jest to możliwe i pożądane - czasami wystarczy wspominane wcześniej good enough. Dzięki temu macie też dobrą bazę do samodzielnych ćwiczeń ;)

Podsumowanie

W powyższym artykule staraliśmy się pokazać sobie na czym polega refaktoryzacja i kiedy się ją wykonuje. Przedstawiliśmy jej podstawowe zalety, ale także skutki uboczne, z którymi należy się zmierzyć oraz przedyskutowaliśmy kwestie związane ze środowiskiem biznesowym. W drugiej części przeanalizowaliśmy przykład refaktoryzacji skryptu do liczenia kolejności drużyn w grupie na turnieju piłkarskim. Mam nadzieję, że dobrze się bawiliście.

Pozdrawiam i dziękuję - Jakub Rojek.

We like to write, even a lot, but on a daily basis we develop web and mobile applications. Check some of the programs we have made.

About author

Jakub Rojek

Lead programmer and co-owner of Wilda Software, with many years of experience in software creation and development, but also in writing texts for various blogs. A trained analyst and IT systems architect. At the same time he is a graduate of Poznan University of Technology and occasionally teaches at this university. In his free time, he enjoys playing video games (mainly card games), reading books, watching american football and e-sport, discovering heavier music, and pointing out other people's language mistakes.

Jakub Rojek