Share
I vår tidigare lektion har vi lärt oss ett nytt sätt att förstå och göra kod bättre genom att extrahera tills vi släpper. Medan den handledningen var ett bra sätt att lära sig teknikerna, var det knappast det perfekta exemplet för att förstå fördelarna med det. I denna lektion kommer vi att extrahera tills vi släpper på alla våra trivia spelrelaterade kod och vi kommer att analysera det slutliga resultatet.
Denna lektion kommer också att avsluta vår serie om refactoring. Om du tycker att vi saknat något, var god att kommentera ett föreslagna ämne. Om bra idéer samlas, fortsätter jag med extra handledning baserat på dina önskemål.
Vilket bättre sätt att starta vår artikel än genom att ta vår längsta metod och extrahera den i små bitar. Testning först, som vanligt, kommer att göra denna procedur inte bara effektiv men också kul.
Som vanligt har du koden som den var när jag började denna handledning i php_start mapp, medan slutresultatet är i php mapp.
funktionenCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ This-> plånböcker [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spelare [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ vinnare = $ this-> didPlayerNotWin (); $ This-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnera $ vinnare annars $ this-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnera sant; annat $ this-> display-> correctAnswerWithTypo (); $ This-> plånböcker [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spelare [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ vinnare = $ this-> didPlayerNotWin (); $ This-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnera $ vinnare
Den här metoden, wasCorrectlyAnswered (), är vårt första offer.
Som vi lärde oss från tidigare lektioner är det första steget när du ändrar arvskoden att få den under test. Detta kan vara en svår process. Lyckligtvis för oss, den wasCorrectlyAnswered () Metoden är ganska enkel. Den består av flera om annat uttalanden. Varje gren av koden returnerar ett värde. När vi har ett returvärde kan vi alltid spekulera på att testningen är genomförbar. Inte nödvändigt lätt, men åtminstone möjligt.
funktionstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> isGettingOutOfPenaltyBox = true; $ this-> game-> purses [$ this-> game-> currentPlayer] = Spel :: $ numberOfCoinsToWin; $ This-> assertTrue ($ this-> Spel-> wasCorrectlyAnswered ());
Det finns ingen bestämd regel om vilket test som ska skrivas först. Vi har just valt den första sökvägen här. Vi hade faktiskt en trevlig överraskning och vi återanvända en av de privata metoderna som vi tog ut många tutorials innan. Men vi är ännu inte färdiga. Alla gröna, så det är dags för refactoring.
funktionstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ This-> currentPlayerWillLeavePenaltyBox (); $ This-> setCurrentPlayerAWinner (); $ This-> assertTrue ($ this-> Spel-> wasCorrectlyAnswered ());
Detta är lättare att läsa och betydligt mer beskrivande. Du hittar de extraherade metoderna i den bifogade koden.
funktionstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileNOTBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ This-> currentPlayerWillLeavePenaltyBox (); $ This-> setCurrentPlayerNotAWinner (); $ This-> assertFalse ($ this-> Spel-> wasCorrectlyAnswered ()); privat funktion setCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0;
Vi förväntade oss att detta skulle gå, men det misslyckades. Skälen är inte klart alls. En närmare titt på didPlayerNotWin () kan vara till hjälp.
funktion didPlayerNotWin () return! ($ this-> purses [$ this-> currentPlayer] == själv :: $ numberOfCoinsToWin);
Metoden returnerar faktiskt sant när en spelare inte vann. Kanske kan vi byta namn på vår variabel men först måste testen gå över.
privat funktion setCurrentPlayerAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = Spel :: $ numberOfCoinsToWin; privat funktion setCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0;
Vid en närmare inspektion kan vi se att vi blandade upp värdena här. Vår förvirring mellan metodnamnet och det variabla namnet gjorde att vi omprövade villkoren.
privat funktion setCurrentPlayerAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0; privat funktion setCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = Spel :: $ numberOfCoinsToWin - 1;
Detta fungerar. Under analysen didPlayerNotWin () vi observerade också att det använder jämlikhet för att bestämma vinnaren. Vi måste sätta vårt värde till en mindre eftersom värdet ökas i den produktionskod vi testar.
De återstående tre testerna är enkla att skriva. De är bara variationer av de två första. Du hittar dem i den bifogade koden.
Det mest förvirrande problemet är det vilseledande $ vinnare variabelnamn. Det borde vara $ notAWinner.
$ notAWinner = $ this-> didPlayerNotWin (); $ This-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnera $ notAWinner;
Vi kan observera att $ notAWinner variabel används endast för att returnera ett värde. Kan vi ringa didPlayerNotWin () Metod direkt i returräkningen?
$ This-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnera $ this-> didPlayerNotWin ();
Detta passerar fortfarande vårt testsystem, men om vi kör våra Golden Master-tester, kommer de att misslyckas med felet "inte tillräckligt med minne". Faktum är att förändringen gör spelet aldrig klart.
Vad som händer är att nuvarande spelare är uppdaterad till nästa spelare. Eftersom vi hade en enda spelare, återanvändde vi alltid samma spelare. Så är testningen. Du vet aldrig när du upptäcker en dold logik i svår kod.
Accentsconagua
funktionstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> add ('En annan spelare'); $ This-> currentPlayerWillLeavePenaltyBox (); $ This-> setCurrentPlayerAWinner (); $ This-> assertTrue ($ this-> Spel-> wasCorrectlyAnswered ()); Genom att bara lägga till en annan spelare i var och en av våra test relaterade till denna metod kan vi se till att logiken är täckt. Detta test kommer att göra det ändrade avkastningsdeklarationen ovan för att misslyckas.
privat funktion selectNextPlayer () $ this-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Vi kan omedelbart upptäcka att valet av nästa spelare är identisk på båda vägarna av tillståndet. Vi kan flytta den till en egen metod. Namnet vi valde för den här metoden är selectNextPlayer (). Detta namn hjälper till att markera det faktum att nuvarande spelares värde kommer att ändras. Det föreslår också att didPlayerNotWin () kan omdöpa sig i något som speglar förhållandet till den aktuella spelaren.
funktionenCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ This-> plånböcker [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spelare [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ This-> selectNextPlayer (); returnera $ notAWinner; annars $ this-> selectNextPlayer (); återvänd sant; annat $ this-> display-> correctAnswerWithTypo (); $ This-> plånböcker [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spelare [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ This-> selectNextPlayer (); returnera $ notAWinner;
Vår kod blir kortare och mer uttrycksfull. Vad kan vi göra nästa? Vi kunde ändra det konstiga namnet på logotypen "inte vinnare" och ändra metoden till en positiv logik istället för en negativ. Eller vi kunde fortsätta att extrahera och hantera den negativa logiska förvirringen senare. Jag tror inte att det finns ett bestämt sätt att gå. Så jag lämnar det negativa logiska problemet som en övning för dig och vi fortsätter med utvinningen.
funktionenCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); annars returnera $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Som en tumregel, försök att ha en enda kodrad på varje väg i en beslutslogik.
Vi extraherade hela koden i varje del av vår om påstående. Detta är ett viktigt steg, och något du alltid bör tänka på. När du har en beslutsväg eller en slinga i koden, bör det bara vara ett enda uttalande på insidan av det. Den som läser den här metoden kommer sannolikt inte att bry sig om genomförandedetaljerna. Han eller hon kommer att bry sig om beslutslogiken, den om påstående.
funktionenCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); returnera $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Och om vi kan bli av med någon extra kod, borde vi. Ta bort annan och fortfarande håller logiken samma gjorde vi en liten ekonomi. Jag gillar den här lösningen bättre eftersom den lyfter fram vad som är funktionens "standard" beteende. Koden som finns i funktionens inredning direkt (den sista raden av kod här). De om uttalandet är den exceptionella funktionaliteten som läggs till som standard.
Jag hörde resonemang att skrivandet av villkoren på detta sätt kan gömma det faktum att standarden inte körs om om uttalandet aktiveras. Jag kan bara hålla med det här, och om du föredrar att behålla annan del där för tydlighet, snälla gör det.
privat funktion getCorrectlyAnsweredForPlayersInPenaltyBox () if ($ this-> isGettingOutOfPenaltyBox) return $ this-> getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox (); annars returnera $ this-> getCorrectlyAnsweredForPlayerStayingInPenaltyBox ();
Vi kan fortsätta att extrahera inuti våra nyskapade privata metoder. Att tillämpa samma princip på vårt nästa villkorliga uttalande leder till koden ovan.
privat funktion geCurrentUserACoin () $ this-> purses [$ this-> currentPlayer] ++;
Genom att titta på våra privata metoder getCorrectlyAnsweredForPlayersNotInPenaltyBox () och getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () vi kan omedelbart observera att en enkel uppgift dupliceras. Den uppgiften kan vara uppenbar för någon som oss som redan vet vad som är med plånböcker och mynt, men inte för nykomling. Extrahera den enda linjen i en metod giveCurrentUserACoin () är en bra idé.
Det hjälper också med dubbelarbete. Om vi i framtiden ändrar hur vi ger spelarmynt, behöver vi bara ändra kod bara inom denna privata metod.
privat funktion getCorrectlyAnsweredForPlayersNotInPenaltyBox () $ this-> display-> correctAnswerWithTypo (); returnera $ this-> getCorrectlyAnsweredForAPlayer (); privat funktion getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () $ this-> display-> correctAnswer (); returnera $ this-> getCorrectlyAnsweredForAPlayer (); privat funktion getCorrectlyAnsweredForAPlayer () $ this-> giveCurrentUserACoin (); $ this-> display-> playerCoins ($ this-> spelare [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ This-> selectNextPlayer (); returnera $ notAWinner;
Då är de två korrekt svarade metoderna identiska, förutom att en av dem matar ut något med ett typsnitt. Vi extraherade dubblettkoden och behöll skillnaderna i varje metod. Du kanske tror att vi kunde ha använt den extraherade metoden med en parameter i uppringningskoden och mata ut en gång normalt och en gång med ett typsnitt. Den ovan föreslagna lösningen har emellertid en fördel: det håller åtskilda de två begreppen som inte är i strafflådan och kommer ut ur strafflådan.
Detta avslutar arbetet med wasCorrectlyAnswered ().
funktionen felAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> spelare [$ this-> currentPlayer]; $ This-> display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ This-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnera sant;
På 11 linjer är den här metoden inte stor, men säkerligen stor. Kommer du ihåg det magiska numret sju plus minus två undersökningar? Det står att vår hjärna kan tänka på 7 + -2 saker samtidigt. Det vill säga, vi har en begränsad kapacitet. Så för att vi lätt och fullt förstår en metod vill vi att logiken inuti den passar in i det intervallet. Med totalt 11 linjer, och ett innehåll på 9 linjer, är denna metod snäll mot gränsen. Man kan argumentera för att det faktiskt finns en tom linje och en annan med bara en klämma. Det skulle bara göra det 7 linjer logik.
Medan bromsar och utrymmen är korta i rymden, har de en betydelse för oss. De separerar delar av logiken, de har en mening, så vår hjärna måste bearbeta dem. Ja, det är lättare jämfört med en fullständig linje o jämförelseslogik, men fortfarande.
Det är därför vårt mål för logiklinjer inom en metod är 4 linjer. Det ligger under det minsta av ovanstående teori, så både ett geni och en mediokerprogrammerare borde kunna förstå metoden.
$ This-> currentPlayer ++; om ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Vi har redan en metod för den här koden, så vi borde använda den.
funktionen felAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> spelare [$ this-> currentPlayer]; $ This-> display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ This-> selectNextPlayer (); återvänd sant;
Bättre, men ska vi släppa eller fortsätta?
$ currentPlayer = $ this-> spelare [$ this-> currentPlayer]; $ This-> display-> playerSentToPenaltyBox ($ currentPlayer);
Vi kunde inline variabeln från dessa två linjer. $ This-> currentPlayer återkommer självklart den nuvarande spelaren, så det är ingen anledning att upprepa logiken. Vi lär ingenting nytt eller abstrakt inget nytt med hjälp av den lokala variabeln.
funktionen felAnswer () $ this-> display-> incorrectAnswer (); $ This-> display-> playerSentToPenaltyBox ($ this-> spelare [$ this-> currentPlayer]); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ This-> selectNextPlayer (); återvänd sant;
Vi är nere till 5 linjer. Något annat där inne?
$ this-> inPenaltyBox [$ this-> currentPlayer] = true;
Vi kan extrahera linjen ovan till sin egen metod. Det kommer att hjälpa till att förklara vad som händer och isolera logiken om att skicka nuvarande spelare till strafflådan på sin egen plats.
funktionen felAnswer () $ this-> display-> incorrectAnswer (); $ This-> display-> playerSentToPenaltyBox ($ this-> spelare [$ this-> currentPlayer]); $ This-> sendCurrentPlayerToPenaltyBox (); $ This-> selectNextPlayer (); återvänd sant;
Fortfarande 5 linjer, men alla metoder kallar. De två första visar saker. De två följande är relaterade till vår logik. Den sista raden returnerar bara sant. Jag ser inget sätt att göra denna metod lättare att förstå utan att introducera komplexitet genom de extraktioner vi kan göra, till exempel genom att extrahera de två visningsmetoderna till en privat. Om vi skulle göra det, var ska den metoden gå? In i detta Spel klass, eller in i Visa? Jag tror att det här redan är en alltför komplicerad fråga att förtjäna att övervägas i förhållande till vår metods rena enkelhet.
Låt oss göra lite statistik genom att kolla in det här fantastiska verktyget från författaren av PHPUnit https://github.com/sebastianbergmann/phploc.git
./ phploc ... / Refactoring \ Legacy \ Code \ - \ Part \ 1 \: \ The \ Golden \ Master / Source / trivia / php / phploc 2.1-gca70e70 av Sebastian Bergmann. Kodens storlekskoder (LOC) 232 Kommentar Kodens linjer (CLOC) 0 (0.00%) Icke-kommenterad Kodslinjer (NCLOC) 232 (100.00%) Logiska linjer av kod (LLOC) 99 (42,67%) Klasser 88 %) Genomsnittlig klasslängd 88 Minsta klasslängd 88 Maximal klasslängd 88 Genomsnittlig metodlängd 7 Minsta metodlängd 1 Maximal metodlängd 17 Funktioner 1 (1.01%) Medel Funktionslängd 1 Ej i klasser eller funktioner 10 (10,10%) Cyclomatisk komplexitet Genomsnittlig komplexitet per LLOC 0,26 Genomsnittlig komplexitet per klass 25,00 Minimum klasskomplexitet 25,00 Maximal klasskomplexitet 25,00 Genomsnittlig komplexitet per metod 3.18 Minsta metakomplexitet 1,00 Maximal metodkomplexitet 10,00 Beroende Globala åtkomster 0 Globala konstanter 0 (0,00%) Globala variabler 0 (0,00%) Super globala Variabler 0 (0,00%) Attributstillträden 115 Icke-statisk 115 (100,00%) Statisk 0 (0,00%) Metodsamtal 21 Icke-statisk 21 (100,00%) Statisk 0 (0,00%) Struktur Namnrymd 0 Gränssnitt 0 Egenskaper 0 Klasser 1 Abstrakt Klasser 0 (0,00%) Betongklasser 1 ( 100.00%) Metoder 11 Omfattning Icke-statiska metoder 11 (100,00%) Statiska metoder 0 (0.00%) Synlighet Offentliga metoder 11 (100.00%) Icke-offentliga metoder 0 (0.00%) Funktioner 1 Namngivna funktioner 1 (100.00%) Anonyma funktioner 0 (0,00%) Konstanter 0 Globala konstanter 0 (0,00%) Klasskonstanter 0 (0,00%)
./ phploc ... / Refactoring \ Legacy \ Code \ - \ Part \ 11 \: \ The \ End \? / Source / trivia / php phploc 2.1-gca70e70 av Sebastian Bergmann. Kodens storlekskod (LOC) 371 Kommentar Kodens linjer (CLOC) 0 (0.00%) Icke-kommenterad kodlinje (NCLOC) 371 (100.00%) Logiska linjer av kod (LLOC) 151 (40,70%) Klasser 145 %) Medelklasslängd 36 Minsta klasslängd 8 Max klasslängd 89 Genomsnittlig metodlängd 2 Minsta metodlängd 1 Maximal metodlängd 14 Funktioner 0 (0.00%) Medel Funktionslängd 0 Ej i klasser eller funktioner 6 (3.97%) Cyclomatisk komplexitet Genomsnittlig komplexitet per LLOC 0,15 Genomsnittlig komplexitet per klass 6,50 Minimum klasskomplexitet 1,00 Högsta klasskomplexitet 17,00 Genomsnittlig komplexitet per metod 1,46 Minsta metakomplexitet 1,00 Maximal metakomplexitet 10,00 Beroende Globala åtkomst 0 Globala konstanter 0 (0,00%) Globala variabler 0 (0,00%) Super-globala Variabler 0 (0,00%) Attributstillgångar 96 Icke-statisk 94 (97,92%) Statisk 2 (2,08%) Metodsamtal 74 Icke-statisk 74 (100,00%) Statisk 0 (0.00%) Struktur Namnrymd 0 Gränssnitt 1 Egenskaper 0 Klasser 3 Abstrakt Klasser 0 (0,00%) Betongklasser 3 (100,00 %) Metoder 59 Omfattning Icke-statiska metoder 59 (100,00%) Statiska metoder 0 (0.00%) Synlighet Offentliga metoder 35 (59.32%) Icke-offentliga metoder 24 (40,68%) Funktioner 0 Namngivna funktioner 0 (0.00%) Anonyma funktioner 0 (0,00%) konstanter 3 globala konstanter 0 (0,00%) klasskonstanter 3 (100,00%)
Brutna data är lika bra som vi kan förstå och analysera.
Antalet logiska streckkod ökade ganska signifikant från 99 till 151. Men det här numret borde inte lura dig att tro att vår kod blev mer komplex. Detta är en naturlig tendens till välrefaktorerad kod, på grund av ökningen av antalet metoder och samtal till dem.
Så snart vi tittar på den genomsnittliga klasslängden kan vi se en dramatisk minskning av kodkod, från 88 till 36.
Och det är bara fantastiskt hur metodlängden gick ner från i genomsnitt sju linjer till bara två linjer kod.
Medan antal linjer är en bra indikator på kodvolymen per måttenhet, är den reala vinsten i analyserna av cyklomatisk komplexitet. Varje gång vi fattar ett beslut i vår kod ökar vi den cyklomatiska komplexiteten. När vi kedjar om uttalanden en inom den andra, den cyklomatiska komplexiteten hos den metoden stiger exponentiellt. Våra fortsatta extraktioner ledde till metoder med endast ett enda beslut i dem, vilket minskar deras genomsnittliga komplexitet per metod från 3,18 till 1,00. Du kan läsa detta som "våra refactored metoder är 3,18 gånger enklare än originalkoden". På klassnivå är droppen i komplexitet ännu mer fantastisk. Det vinner ner från 25.00 till 6.50.
Väl. Det är allt. Slut på serien. Var gärna uttrycka dina åsikter och om du tror att vi missat något refactoringämne be dem i kommentarer nedan. Om de är intressanta ska jag förvandla dem till extra delar av denna serie.
Tack för din odelade uppmärksamhet.
