Refactoring Legacy Code Del 5 - Spelets testbara metoder

Gammal kod. Ugly code. Komplicerad kod. Spaghetti kod. Gibberish nonsens. I två ord, Legacy Code. Det här är en serie som hjälper dig att arbeta och hantera det.

I vår tidigare handledning testade vi våra Runner-funktioner. I den här lektionen är det dags att fortsätta där vi slutade genom att testa vår Spel klass. Nu när du börjar med en så stor bit av kod som vi har här, är det frestande att börja testa i topp-ner-sätt, metod för metod. Detta är för det mesta omöjligt. Det är mycket bättre att börja testa det med sina korta testbara metoder. Så här gör vi i den här lektionen: hitta och testa dessa metoder.

Skapa ett spel

För att testa en klass måste vi initiera ett objekt av den specifika typen. Vi kan anse att vårt första test är att skapa ett sådant nytt objekt. Du kommer bli förvånad över hur många hemligheter konstruktörer kan gömma sig.

require_once __DIR__. '/ ... /trivia/php/Game.php'; klass GameTest utökar PHPUnit_Framework_TestCase funktion testWeCanCreateAGame () $ game = new Game (); 

Till vår förvåning, Spel kan faktiskt skapas ganska enkelt. Inga problem medan du kör bara nytt spel(). Ingenting bryter. Detta är en mycket bra start, särskilt med tanke på det SpelKonstruktören är ganska stor och det gör många saker.

Hitta den första testbara metoden

Det är frestande att förenkla konstruktören just nu. Men vi har bara den gyllene mästaren för att se till att vi inte bryter något. Innan vi går till konstruktören måste vi testa det mesta av resten av klassen. Så var ska vi börja?

Leta efter den första metoden som returnerar ett värde och fråga dig själv, "Kan jag ringa och kontrollera returvärdet för denna metod?". Om svaret är ja, det är en bra kandidat för vårt test.

funktionen är spelbar () $ minimumNumberOfPlayers = 2; returnera ($ this-> howManyPlayers ()> = $ minimumNumberOfPlayers); 

Vad sägs om den här metoden? Det verkar som en bra kandidat. Endast två rader och det returnerar ett booleskt värde. Men vänta, det kallar en annan metod, howManyPlayers ().

funktion howManyPlayers () return count ($ this-> spelare); 

Detta är i grund och botten bara en metod som räknar elementen i klassen " spelare array. OK, så om vi inte lägger till några spelare, borde det vara noll. isPlayable () ska returnera falskt. Låt oss se om vårt antagande är korrekt.

funktionstestAJustCreatedNewGameIsNotPlayable () $ game = new Game (); $ This-> assertFalse ($ Spel-> isPlayable ()); 

Vi bytte namn på vår tidigare testmetod för att återspegla vad vi verkligen vill testa. Då hävdar vi bara att spelet inte är spelbart. Testet passerar. Men falska positiva är vanliga i många fall. Så för sinnesro kan vi hävda sant och se till att testet misslyckas.

$ This-> assertTrue ($ Spel-> isPlayable ());

Och det gör det!

PHPUnit_Framework_ExpectationFailedException: Misslyckades hävdar att falskt är sant.

Hittills ganska lovande. Vi lyckades testa metodens ursprungliga returvärde, det värde som representeras av initialen stat av Spel klass. Observera det betonade ordet: "state". Vi måste hitta ett sätt att kontrollera spelets tillstånd. Vi behöver ändra det, så det kommer att ha det minsta antalet spelare.

Om vi ​​analyserar Spel's Lägg till() metod kommer vi att se att det lägger till element i vårt sortiment.

array_push ($ this-> spelare, $ playerName);

Vårt antagande verkställs på det sätt som Lägg till() Metoden används i RunnerFunctions.php.

funktionskörning () $ aGame = nytt spel (); $ AGame-> lägga ( "Chet"); $ AGame-> lägga ( "Pat"); $ AGame-> lägga ( "Sue"); // ... //

Baserat på dessa observationer kan vi dra slutsatsen att vi använder Lägg till() två gånger borde vi kunna ta med oss Spel in i ett tillstånd med två spelare.

funktion testAfterAddingTwoPlayersToANewGameItIsPlayable () $ game = new Game (); $ game-> add ('First Player'); $ game-> add ('andra spelare'); $ This-> assertTrue ($ Spel-> isPlayable ()); 

Genom att lägga till denna andra testmetod kan vi se till isPlayable () returnerar sant, om villkoren är uppfyllda.

Men du kanske tror att det här inte är ett enhetstest. Vi använder Lägg till() metod! Vi utövar mer än det minsta lägsta koden. Vi kan istället bara lägga till elementen till $ spelare array och inte lita på Lägg till() metod alls.

Jo, svaret är ja och nej. Vi kunde göra det, ur teknisk synvinkel. Det kommer att ha fördelen av direkt kontroll över matrisen. Det kommer dock att ha nackdelen med koddubbling mellan kod och test. Så välj ett av de dåliga alternativen som du tror att du kan leva med och använda den. Jag föredrar personligen att återanvända metoder som Lägg till().

Refaktoringstest

Vi är på grön, vi refactor. Kan vi göra våra test bättre? Jo, det kan vi. Vi kunde omvandla vårt första test för att verifiera alla villkor för inte tillräckligt många spelare.

funktionstestGameWithNotEnoughPlayersIsNotPlayable () $ game = new Game (); $ This-> assertFalse ($ Spel-> isPlayable ()); $ game-> add ('En spelare'); $ This-> assertFalse ($ Spel-> isPlayable ()); 

Du kanske har hört talas om begreppet "En påstående per test". Jag håller mest med det, men om du har ett test som verifierar ett enda koncept och kräver flera påståenden att göra dess verifikation anser jag att det är acceptabelt att använda mer än en påstående. Denna uppfattning är också starkt befordrad av Robert C. Martin i hans lärdomar.

Men hur är det med vår andra testmetod? Är det bra nog? jag säger nej.

$ game-> add ('First Player'); $ game-> add ('andra spelare');

Dessa två samtal stör mig lite. De är ett detaljerat genomförande utan en uttrycklig förklaring i vår metod. Varför inte extrahera dem till en privat metod?

funktion testAfterAddingEnoughPlayersToANewGameItIsPlayable () $ game = new Game (); $ This-> addEnoughPlayers ($ spel); $ This-> assertTrue ($ Spel-> isPlayable ());  privat funktion addEnoughPlayers ($ game) $ game-> add ('First Player'); $ game-> add ('andra spelare'); 

Detta är mycket bättre och det leder oss också till ett annat koncept som vi missade. I båda testen uttryckte vi på ett eller annat sätt begreppet "tillräckligt med spelare". Men hur mycket kostar det? Är det två? Ja, för nu är det. Men vill vi att vårt test misslyckas om Spellogik kommer att kräva minst tre spelare? Vi vill inte att detta ska hända. Vi kan införa ett offentligt statiskt klassfält för det.

klassspel static $ minimumNumberOfPlayers = 2; // ... // funktion __construct () // ... // funktion isPlayable () return ($ this-> howManyPlayers ()> = själv :: $ minimumNumberOfPlayers);  // ... //

Detta gör att vi kan använda den i våra test.

privat funktion addEnoughPlayers ($ game) for ($ i = 0; $ i < Game::$minimumNumberOfPlayers; $i++)  $game->lägg till ('en spelare') 

Vår lilla hjälparmetod kommer bara att lägga till spelare tills nog har lagts till. Vi kan även skapa en annan sådan metod för vårt första test, så vi lägger till nästan tillräckligt många spelare.

funktionstestGameWithNotEnoughPlayersIsNotPlayable () $ game = new Game (); $ This-> assertFalse ($ Spel-> isPlayable ()); $ This-> addJustNothEnoughPlayers ($ spel); $ This-> assertFalse ($ Spel-> isPlayable ());  privat funktion addJustNothEnoughPlayers ($ game) for ($ i = 0; $ i < Game::$minimumNumberOfPlayers - 1; $i++)  $game->lägg till ('En spelare'); 

Men detta introducerade några dubbelarbete. Våra två hjälpar metoder är ganska lika. Kan vi inte extrahera en tredje från dem?

privat funktion addEnoughPlayers ($ game) $ this-> addManyPlayers ($ game, Game :: $ minimumNumberOfPlayers);  privat funktion addJustNothEnoughPlayers ($ game) $ this-> addManyPlayers ($ spel, spel :: $ minimumNumberOfPlayers - 1);  privat funktion addManyPlayers ($ game, $ numberOfPlayers) for ($ i = 0; $ i < $numberOfPlayers; $i++)  $game->lägg till ('en spelare') 

Det är bättre, men det introducerar ett annat problem. Vi reducerade dubbelarbete i dessa metoder, men vår $ spel objektet har nu gått ner i tre nivåer. Det blir svårt att hantera. Det är dags att initiera det i testets inrätta() metod och återanvända det.

klass GameTest utökar PHPUnit_Framework_TestCase privat $ game; funktion setUp () $ this-> game = new Game;  funktionstestGenomWithNotEnoughPlayersIsNotPlayable () $ this-> assertFalse ($ this-> game-> isPlayable ()); $ This-> addJustNothEnoughPlayers (); $ This-> assertFalse ($ this-> Spel-> isPlayable ());  funktion testAfterAddingEnoughPlayersToANewGameItIsPlayable () $ this-> addEnoughPlayers ($ this-> game); $ This-> assertTrue ($ this-> Spel-> isPlayable ());  privat funktion addEnoughPlayers () $ this-> addManyPlayers (Spel :: $ minimumNumberOfPlayers);  privat funktion addJustNothEnoughPlayers () $ this-> addManyPlayers (Spel :: $ minimumNumberOfPlayers - 1);  privat funktion addManyPlayers ($ numberOfPlayers) for ($ i = 0; $ i < $numberOfPlayers; $i++)  $this->spel-> lägg till ('En spelare'); 

Mycket bättre. All irrelevant kod är i privata metoder, $ spel initieras i inrätta() och mycket förorening avlägsnades från testmetoderna. Vi måste dock göra en kompromiss här. I vårt första test börjar vi med ett påstående. Detta förutsätter det inrätta() kommer alltid att skapa ett tomt spel. Det här är OK för tillfället. Men i slutet av dagen måste du inse att det inte finns något sådant som perfekt kod. Det finns bara kod med kompromisser som du är villig att leva med.

Den andra testbara metoden

Om vi ​​skannar vår Spel klass från toppen mot botten, är nästa metod på vår lista Lägg till(). Ja, samma metod som vi använde i våra test i föregående stycke. Men kan vi testa det?

funktion testItCanAddANewPlayer () $ this-> game-> add ('En spelare'); $ this-> assertEquals (1, count ($ this-> game-> players)); 

Nu är det här ett annat sätt att testa objekt. Vi kallar vår metod och sedan verifierar vi objektets tillstånd. Som Lägg till() returnerar alltid Sann, Det finns inget sätt att vi kan testa sin produktion. Men vi kan börja med en tom Spel objekt och kontrollera sedan om det finns en enda användare efter att vi har lagt till en. Men är det tillräckligt med kontroll?

funktion testItCanAddANewPlayer () $ this-> assertEquals (0, count ($ this-> game-> players)); $ this-> game-> add ('En spelare'); $ this-> assertEquals (1, count ($ this-> game-> players)); 

Skulle det inte vara bättre att också kontrollera om det inte finns några spelare innan vi ringer Lägg till()? Tja, det kan vara lite för mycket här, men som du kan se i koden ovan kan vi göra det. Och när du inte är säker på det ursprungliga tillståndet bör du göra ett påstående om det. Detta skyddar dig också från framtida kodändringar som kan ändra ditt objekts ursprungliga tillstånd.

Men testar vi alla saker Lägg till() metod gör? Jag säger nej. Förutom att lägga till en användare ställer den också många inställningar för den. Vi bör också kolla på dem.

funktion testItCanAddANewPlayer () $ this-> assertEquals (0, count ($ this-> game-> players)); $ this-> game-> add ('En spelare'); $ this-> assertEquals (1, count ($ this-> game-> players)); $ this-> assertEquals (0, $ this-> game-> places [1]); $ this-> assertEquals (0, $ this-> game-> purses [1]); $ This-> assertFalse ($ this-> Spel-> inPenaltyBox [1]); 

Detta är bättre. Vi verifierar varje åtgärd som Lägg till() metod gör det. Den här gången föredrog jag att direkt testa $ spelare array. Varför? Vi kunde ha använt howManyPlayers () metod som i princip gör detsamma, eller hur? Tja, i det här fallet ansåg vi att det är viktigare att beskriva våra påståenden av de effekter som Lägg till() Metoden har på objektets tillstånd. Om vi ​​behöver byta Lägg till(), vi skulle förvänta oss att det test som testar sitt strikta beteende kommer att misslyckas. Jag har haft oändliga debatter med mina kollegor i Syneto om detta. Särskilt för att denna typ av test introducerar en stark koppling mellan testet och hur Lägg till() Metoden är faktiskt implementerad. Så om du föredrar att testa det tvärtom betyder det inte att dina idéer är felaktiga.

Vi kan säkert ignorera testningen av produktionen, den echoln () rader. De skriver ut innehåll på skärmen. Vi vill inte röra dessa metoder ännu. Vår gyllene mästare bygger helt och hållet på denna produktion.

Refactoring-test (Bis)

Vi har en annan testad metod med ett helt nytt godkännande. Det är dags att refactor båda, bara en liten bit. Låt oss börja med våra tester. Är inte de sista tre påståenden lite förvirrande? De verkar inte vara relaterade strängt till att lägga till en spelare. Låt oss ändra det:

funktion testItCanAddANewPlayer () $ this-> assertEquals (0, count ($ this-> game-> players)); $ this-> game-> add ('En spelare'); $ this-> assertEquals (1, count ($ this-> game-> players)); $ This-> assertDefaultPlayerParametersAreSetFor (1); 

Det är bättre. Metoden är nu mer abstrakt, återanvändbar, uttrycklig namngiven och döljer alla obetydliga detaljer.

Refactoring the Lägg till() Metod

Vi kan göra något liknande med vår produktionskod.

funktion add ($ playerName) array_push ($ this-> spelare, $ playerName); $ This-> setDefaultPlayerParametersFor ($ this-> howManyPlayers ()); echoln ($ playerName. "var tillagt"); echoln ("De är spelarnummer". count ($ this-> players)); återvänd sant; 

Vi extraherade de obetydliga detaljerna i setDefaultPlayerParametersFor ().

privat funktion setDefaultPlayerParametersFor ($ playerId) $ this-> placerar [$ playerId] = 0; $ this-> purses [$ playerId] = 0; $ this-> inPenaltyBox [$ playerId] = false; 

Egentligen kom denna idé till mig efter att jag skrev testet. Detta är ett annat bra exempel på hur tester tvingar oss att tänka på vår kod från en annan synvinkel. Denna annorlunda vinkel på problemet är vad vi måste utnyttja och låt våra tester styra vår design av produktionskoden.

Den tredje testbara metoden

Låt oss hitta vår tredje kandidat för testning. howManyPlayers () är för enkelt och indirekt redan testat. rulla() är för komplex för att kunna testas direkt. Plus det återvänder null. Fråga frågor() verkar vara intressant vid första anblicken, men det är all presentation, inget avkastningsvärde.

currentCategory () är testbar, men det är ganska svår att testa. Det är en stor väljare med tio villkor. Vi behöver ett tio linjer långt test och då måste vi på allvar refactor denna metod och absolut testen också. Vi bör notera denna metod och komma tillbaka till den när vi är färdiga med de enklare. För oss kommer detta att vara i vår nästa handledning.

wasCorrectlyAnswered () är för komplicerat igen. Vi kommer att behöva extrahera från det, små bitar av kod som kan testas. dock, fel svar() verkar lovande. Det matar ut saker på skärmen, men det ändrar också tillståndet för vårt objekt. Låt oss se om vi kan kontrollera det och testa det.

funktionstestWhenAPlayerEntersAWrongAnswerItIsSentToThePenaltyBox () $ this-> game-> add ('En spelare'); $ this-> game-> currentPlayer = 0; $ This-> Spel-> wrongAnswer (); $ This-> assertTrue ($ this-> Spel-> inPenaltyBox [0]); 

Grrr ... Det var ganska svårt att skriva denna testmetod. fel svar() förlitar sig på $ This-> currentPlayer för dess beteendelogik, men det använder också $ This-> spelare i sin presentation del. Ett fuligt exempel på varför du inte ska blanda logik och presentation. Vi kommer att hantera detta i en framtida handledning. För närvarande har vi testat att användaren kommer in i strafflådan. Vi måste också observera att det finns en om() uttalande i metoden. Detta är ett villkor att vi ännu inte testar, eftersom vi bara har en enda spelare och sålunda uppfyller vi inte villkoret. Vi kunde testa för det slutliga värdet av $ currentPlayer fastän. Men att lägga till den här koden till testet gör att den misslyckas.

$ this-> assertEquals (1, $ this-> game-> currentPlayer);

En närmare titt på den privata metoden shouldResetCurrentPlayer () avslöjar problemet. Om indexet för den aktuella spelaren är lika med antalet spelare, återställs det till noll. Aaaahhh! Vi går faktiskt in i om()!

funktionstestWhenAPlayerEntersAWrongAnswerItIsSentToThePenaltyBox () $ this-> game-> add ('En spelare'); $ this-> game-> currentPlayer = 0; $ This-> Spel-> wrongAnswer (); $ This-> assertTrue ($ this-> Spel-> inPenaltyBox [0]); $ this-> assertEquals (0, $ this-> game-> currentPlayer);  funktionstestCurrentPlayerIsNotResetAfterWrongAnswerIfOtherPlayersDidNotYetPlay () $ this-> addManyPlayers (2); $ this-> game-> currentPlayer = 0; $ This-> Spel-> wrongAnswer (); $ this-> assertEquals (1, $ this-> game-> currentPlayer); 

Bra. Vi skapade ett andra test, för att testa det specifika fallet när det fortfarande finns spelare som inte spelade. Vi bryr oss inte om inPenaltyBox Ange för det andra testet. Vi är bara intresserade av den aktuella spelarens index.

Den slutliga testbara metoden

Den sista metoden vi kan testa och sedan refactor är didPlayerWin ().

funktion didPlayerWin () $ numberOfCoinsToWin = 6; returnera! ($ this-> purses [$ this-> currentPlayer] == $ numberOfCoinsToWin); 

Vi kan omedelbart observera att dess kodstruktur liknar isPlayable (), Metoden vi testade först. Vår lösning ska också vara något liknande. När din kod är så kort är det bara två till tre linjer, som gör mer än ett litet steg inte så stor risk. I värsta fall scenarier återgår du tre rader av kod. Så låt oss göra det i ett enda steg.

funktion testTestPlayerWinsWithTheCorrectNumberOfCoins () $ this-> game-> currentPlayer = 0; $ this-> game-> purses [0] = Spel :: $ numberOfCoinsToWin; $ This-> assertTrue ($ this-> Spel-> didPlayerWin ()); 

Men vänta! Det misslyckas. Hur är det mojligt? Ska det inte gå? Vi gav rätt antal mynt. Om vi ​​studerar vår metod kommer vi att upptäcka ett litet missvisande faktum.

returnera! ($ this-> purses [$ this-> currentPlayer] == $ numberOfCoinsToWin);

Avkastningsvärdet är faktiskt negativt. Så berättar metoden inte om en spelare vann, det berättar om en spelare inte vunnit spelet. Vi kunde gå in och hitta de platser där denna metod används och negera dess värde där. Ändra sedan sitt beteende här, för att inte felaktigt negera svaret. Men det används i wasCorrectlyAnswered (), en metod som vi ännu inte kan testa. Kanske är det för tillfället en enkel omdämning att markera rätt funktionalitet.

funktion didPlayerNotWin () return! ($ this-> purses [$ this-> currentPlayer] == själv :: $ numberOfCoinsToWin); 

Tankar och slutsatser

Så här handlar om handledning. Medan vi inte gillar negationen i namnet, är det en kompromiss vi kan göra just nu. Detta namn kommer säkert att förändras när vi börjar refactoring andra delar av koden. Dessutom, om du tittar på våra test, ser de udda nu:

funktion testTestPlayerWinsWithTheCorrectNumberOfCoins () $ this-> game-> currentPlayer = 0; $ this-> game-> purses [0] = Spel :: $ numberOfCoinsToWin; $ This-> assertFalse ($ this-> Spel-> didPlayerNotWin ()); 

Genom att testa falskt på en negerad metod, utövad med ett värde som föreslår ett sant resultat, introducerade vi ganska mycket förvirring i kodläsbarheten. Men det här är bra för nu, eftersom vi måste sluta vid något tillfälle, rätt?

I vår nästa handledning börjar vi arbeta med några av de svårare metoderna inom Spel klass. Tack för att du läste.