Share
Gammal kod. Ugly code. Komplicerad kod. Spaghetti kod. Jibberish nonsens. I två ord, Legacy Code. Det här är en serie som hjälper dig att arbeta och hantera det.
Vi träffade vår äldre källkod i vår tidigare lektion. Därefter sprang vi koden för att bilda en åsikt om dess grundläggande funktionalitet och skapade en Golden Master-testpaket för att ha ett rå säkerhetsnät för framtida förändringar. Vi kommer att fortsätta arbeta med den här koden och du hittar den under trivia / php_start mapp. Den andra mappen trivia / php_start är med den här lektionen färdiga koden.
Tiden för de första ändringarna har kommit och vilket bättre sätt att förstå en svår kodbas än att börja extrahera magiska konstanter och strängar i variabler? Dessa till synes enkla uppgifter kommer att ge oss större och ibland oväntade insikter i den inre funktionen av arvskoden. Vi kommer att behöva räkna ut de ursprungliga kodförfattarnas avsikter och hitta de korrekta namnen på de kodstycken som vi aldrig har sett tidigare.
Magiska strängar är strängar som används direkt i olika uttryck, utan att tilldelas en variabel. Denna typ av sträng hade en särskild betydelse för den ursprungliga författaren av koden, men istället för att tilldela dem en välkänd variabel trodde författaren att strängens mening var uppenbar nog.
Låt oss börja med att titta på vårt Game.php och försök att identifiera strängar. Om du använder en IDE (och du borde) eller en smartare textredigerare som kan markera källkoden kommer spotting strängarna att vara enkla. Här är en bild av hur koden ser ut på min skärm.
Allt med apelsin är en sträng. Att hitta varje sträng i vår källkod är väldigt lätt så här. Så se till att markering stöds och aktiveras i din redaktör, oavsett vad din ansökan är.
Den första orange delen i vår kod är omedelbart vid rad tre. Strängen innehåller emellertid bara en nylinje tecken. Detta borde vara uppenbart nog enligt min mening, så vi kan fortsätta.
När det gäller att bestämma vad som ska extraheras och vad som ska hållas oförändrat finns det få tummen, men i slutet är det din professionella åsikt som måste råda. Baserat på det måste du bestämma vad du ska göra med varje kod du analyserar.
Accentsconagua
för ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". i $); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sportfråga". $ i)); array_push ($ this-> rockQuestions, $ this-> createRockQuestion ($ i)); funktion createRockQuestion ($ index) returnera "Rock Question". $ Index; Så låt oss analysera raderna 32 till 42, det utdrag du kan se ovan. För pop-, vetenskaps- och sportfrågor finns det bara en enkel sammanfogning. Åtgärden att komponera strängen för en bergfråga extraheras emellertid till en metod. Är det enligt dina uppfattningar tillräckligt med dessa sammanbindningar och strängar så att vi kan hålla dem alla i våra öglor? Eller tror du att extrahera alla strängar i deras metoder skulle motivera förekomsten av dessa metoder? Om så är fallet, hur skulle du namnge dessa metoder?
Oavsett svaret måste vi ändra koden. Det är dags att sätta vår Golden Master på jobbet och skriva vårt test som faktiskt kör och jämför vår kod med det befintliga innehållet.
klass GoldenMasterTest utökar PHPUnit_Framework_TestCase privat $ gmPath; funktion setUp () $ this-> gmPath = __DIR__. '/Gm.txt'; funktionstestGenerateOutput () $ times = 20000; $ this-> generateMany ($ times, $ this-> gmPath); funktionstest OutputMatchesGoldenMaster () $ times = 20000; $ actualPath = '/tmp/actual.txt'; $ this-> generateMany ($ times, $ actualPath); $ file_content_gm = file_get_contents ($ this-> gmPath); $ file_content_actual = file_get_contents ($ actualPath); $ this-> assertTrue ($ file_content_gm == $ file_content_actual); privat funktion generera många ($ gånger, $ filnamn) ... privat funktion generera utmatning ($ seed) ...
Vi skapade ett annat test för att jämföra utgångarna, såg till testGenerateOutput () genererar bara utsignalen en gång och gör inget annat. Vi flyttade också den gyllene huvudutdatafilen "Gm.txt" in i den aktuella katalogen eftersom "/ Tmp" kan rensas när systemet startas om. För våra faktiska resultat kan vi fortfarande använda det. På de flesta UNIX-liknande system "/ Tmp" är monterad i RAM så det är mycket snabbare än filsystemet. Om du gjorde det bra ska testen gå utan problem.
Det är mycket viktigt att komma ihåg att markera vårt generatortest som "hoppas över" för framtida förändringar. Om du känner dig mer bekväm med att kommentera eller till och med radera det helt, vänligen gör det. Det är viktigt att vår Guldmästare inte kommer att förändras när vi ändrar vår kod. Den genererades en gång och vi vill inte ändra det, så att vi kan vara säkra på att vår nybildade kod alltid jämförs med originalet. Om du känner dig mer bekväm gör du en säkerhetskopia av det, fortsätt att göra det.
funktion testGenerateOutput () $ this-> markTestSkipped (); $ gånger = 20000; $ this-> generateMany ($ times, $ this-> gmPath);
Jag kommer bara markera testet som hoppas över. Detta kommer att göra vårt testresultat till "gul", vilket innebär att alla tester passerar men vissa överhoppas eller markeras som ofullständiga.
Med tester på plats kan vi börja byta kod. Enligt min professionella mening kan alla strängarna hållas inne i för slinga. Vi tar koden från createRockQuestion () metod, flytta den inuti för loop och radera metoden helt och hållet. Denna refactoring kallas Inlinemetod.
"Sätt metoden kropp i kroppen av sina uppringare och ta bort metoden." ~ Martin Fowler
Det finns en specifik uppsättning åtgärder för att göra denna typ av refactoring, som definierats av Marting Fowler i Refactoring: Förbättra utformningen av befintlig kod:
Vi har inte subklasser som sträcker sig Spel, så det första steget valideras.
Det finns bara en enda användning av vår metod, inuti för slinga.
funktion __construct () $ this-> players = array (); $ this-> places = array (0); $ this-> purses = array (0); $ this-> inPenaltyBox = array (0); $ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array (); för ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". i $); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sportfråga". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); funktion createRockQuestion ($ index) returnera "Rock Question". $ Index; Vi sätter koden från createRockQuestion () in i för slinga i konstruktören. Vi har fortfarande vår gamla kod. Nu är det dags att springa vårt test.
Våra tester passerar. Vi kan ta bort vår createRockQuestion () metod.
funktion __construct () // ... // för ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". i $); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sportfråga". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); funktionen är Playable () returnera ($ this-> howManyPlayers ()> = 2); Slutligen bör vi springa våra test igen. Om vi missade ett samtal till en metod kommer de att misslyckas.
De ska passera igen. grattis! Vi är färdiga med vår första refactoring.
Strängar i metoderna Lägg till() och rulla () används bara för att mata ut dem med hjälp av echoln () metod. Fråga frågor() jämför strängar till kategorier. Detta verkar också acceptabelt. currentCategory () å andra sidan returnerar strängar baserat på ett nummer. I denna metod finns det många dubbla strängar. Ändring av någon kategori, förutom att Rock skulle behöva ändra sitt namn på tre ställen, endast i denna metod.
funktion currentCategory () if ($ this-> places [$ this-> currentPlayer] == 0) returnera "Pop"; om ($ this-> placerar [$ this-> currentPlayer] == 4) returnera "Pop"; om ($ this-> placerar [$ this-> currentPlayer] == 8) returnera "Pop"; om ($ this-> placerar [$ this-> currentPlayer] == 1) returnera "Science"; om ($ this-> placerar [$ this-> currentPlayer] == 5) returnera "Science"; om ($ this-> placerar [$ this-> currentPlayer] == 9) returnera "Science"; om ($ this-> placerar [$ this-> currentPlayer] == 2) returnera "Sports"; om ($ this-> placerar [$ this-> currentPlayer] == 6) returnera "Sports"; om ($ this-> placerar [$ this-> currentPlayer] == 10) returnera "Sports"; återgå "Rock";
Jag tror att vi kan göra bättre. Vi kan använda en refiteringsmetod som heter Introducera Local Variable och eliminera dubbelarbete. Följ dessa riktlinjer:
funktion currentCategory () $ popCategory = "Pop"; $ scienceCategory = "Science"; $ sportCategory = "Sport"; $ rockCategory = "Rock"; om ($ this-> placerar [$ this-> currentPlayer] == 0) return $ popCategory; om ($ this-> placerar [$ this-> currentPlayer] == 4) return $ popCategory; om ($ this-> placerar [$ this-> currentPlayer] == 8) returnera $ popCategory; om ($ this-> placerar [$ this-> currentPlayer] == 1) return $ scienceCategory; om ($ this-> placerar [$ this-> currentPlayer] == 5) return $ scienceCategory; om ($ this-> placerar [$ this-> currentPlayer] == 9) returnera $ scienceCategory; om ($ this-> placerar [$ this-> currentPlayer] == 2) returnera $ sportCategory; om ($ this-> placerar [$ this-> currentPlayer] == 6) return $ sportCategory; om ($ this-> placerar [$ this-> currentPlayer] == 10) return $ sportCategory; returnera $ rockCategory;
Detta är mycket bättre. Du har nog redan observerat några framtida förbättringar som vi skulle kunna göra med vår kod, men vi är bara i början av vårt arbete. Det är frestande att fixa allt du hittar omedelbart, men gör det inte. Många gånger, framförallt innan koden är väl förstådd, kan frestande förändringar leda till döda ändar eller till och med mer trasig kod. Om du tror att det finns en chans att du kommer att glömma din idé, notera den bara på en anteckningsbok eller skapa en uppgift i din projektledningssoftware. Nu måste vi fortsätta med våra strängrelaterade problem.
I resten av filen är alla strängar utgående relaterade, skickade till echoln (). För tillfället lämnar vi dem orörda. Att ändra dem skulle påverka utskriften och leverera logiken i vår ansökan. De ingår i presentationsskiktet blandat med affärslogik. Vi kommer att hantera att skilja olika problem i en framtida lektion.
Magiska konstanter är mycket lika magiska strängar, men med värden. Dessa värden kan vara booleska värden eller siffror. Vi kommer att koncentrera oss mest på nummer som används i om uttalanden eller lämna tillbaka uttalanden eller andra uttryck. Om dessa siffror har en otydlig mening måste vi extrahera dem till variabler eller metoder.
include_once __DIR__. '/Game.php'; $ NotAWinner; $ aGame = nytt spel (); $ AGame-> lägga ( "Chet"); $ AGame-> lägga ( "Pat"); $ AGame-> lägga ( "Sue"); gör $ aGame-> roll (rand (0, 5) + 1); om (rand (0, 9) == 7) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); medan ($ notAWinner);
Vi börjar med GameRunner.php den här gången och vårt första fokus är rulla() metod som får några slumpmässiga nummer. Förra författaren bryr sig inte om att ge dessa nummer en mening. Kan vi? Om vi analyserar koden:
rand (0, 5) + 1
Det kommer att returnera ett tal mellan ett och sex. Den slumpmässiga delen returnerar ett tal mellan noll och fem som vi alltid lägger till en. Så det är säkert mellan ett och sex. Nu måste vi överväga sammanhanget i vår ansökan. Vi utvecklar ett trivia-spel. Vi vet att det finns en slags styrelse där våra spelare måste flytta. Och för att göra det måste vi rulla tärningarna. En dörr har sex ansikten och det kan producera tal mellan ett och sex. Det verkar som ett rimligt avdrag.
$ tärning = rand (0, 5) + 1; $ AGame-> rulle ($ tärningar);
Är det inte så bra? Vi använde Introduce Local Variable refactoring-konceptet igen. Vi heter vår nya variabel $ dice och det representerar det slumptal som genereras mellan ett och sex. Detta gjorde också vårt nästa uttalande ljud riktigt naturligt: Spel, rulla tärningar.
Körde du dina test? Jag nämnade inte det, men vi måste springa dem så ofta som möjligt. Om du inte har det skulle det vara en bra tid att köra dem. Och de borde passera.
Så, det här var ett fall av ingenting annat än att bara utbyta ett tal med en variabel. Vi tog ett helt uttryck som representerade ett tal och extraherade det till en variabel. Detta kan tekniskt betraktas som ett Magic Constant-fall, men inte ett rent fall. Vad sägs om vårt nästa slumpmässiga uttryck?
om (rand (0, 9) == 7)
Detta är mer knepigt. Vad är noll, nio och sju i det uttrycket? Kanske kan vi namnge dem. Vid första anblicken har jag inga bra idéer för noll och nio, så låt oss försöka sju. Om numret som returneras av vår slumpmässiga funktion är lika med sju, kommer vi att ange den första grenen av om uttalande som ger ett felaktigt svar. Så kanske våra sju kan namnges $ wrongAnswerId.
$ wrongAnswerId = 7; om (rand (0, 9) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();
Våra test passerar fortfarande och koden är något mer uttrycksfull. Nu när vi lyckades namnge vårt nummer sju, ställer det villkoret i ett annat sammanhang. Vi kan tänka på några anständiga namn för noll och nio också. De är bara parametrar till rand(), så variablerna kommer förmodligen att kallas min-något och max-something.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; om (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();
Nu är det uttrycksfullt. Vi har ett minsta svar ID, högst en och ett för fel svar. Mysteriet löst.
gör $ tärning = rand (0, 5) + 1; $ AGame-> rulle ($ tärningar); $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; om (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); medan ($ notAWinner);
Men märker att all kod är inuti a göra medan slinga. Behöver vi omfördela svar-ID-variablerna varje gång? Jag tror inte det. Låt oss försöka flytta dem ur slingan och se om våra tester passerar.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; gör $ tärning = rand (0, 5) + 1; $ AGame-> rulle ($ tärningar); om (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); medan ($ notAWinner);
Ja. Testerna passerar sålunda också.
Det är dags att byta till Game.php och leta efter magiska konstanter där också. Om du har kodmarkering har du säkert konstanter markerade i någon ljus färg. Mine är blåa och de är ganska lätta att upptäcka.
Hitta den magiska konstanten 50 i det för loop var ganska lätt. Och om vi tittar på vad koden gör kan vi upptäcka det innanför för slinga, element skjuts till flera arrays. Så vi har en sorts listor, var och en med 50 element. Varje lista representerar en fråge kategori och variablerna är faktiskt klassfält definierade ovan som arrays.
$ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array ();
Så vad kan 50 representera? Jag slår vad om att du redan har några idéer. Namn är en av de svåraste uppgifterna i programmeringen, om du har mer än en idé och du är osäker på vilken du ska välja ska du inte skämmas. Jag har också olika namn i mitt huvud och jag utvärderar möjligheterna att välja den bästa, även när du skriver denna paragraf. Jag tror att vi kan gå med ett konservativt namn för 50. Något i linje med$ questionsInEachCategory eller $ categorySize eller något liknande.
$ categorySize = 50; för ($ i = 0; $ i < $categorySize; $i++) array_push($this->popQuestions, "Pop Question". i $); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sportfråga". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); Det ser anständigt ut. Vi kan behålla det. Och testen går förstås.
funktion isPlayable () return ($ this-> howManyPlayers ()> = 2);
Vad är två? Jag är säker på det här laget är svaret klart för dig. Det är lätt:
funktionen är spelbar () $ minimumNumberOfPlayers = 2; returnera ($ this-> howManyPlayers ()> = $ minimumNumberOfPlayers);
Håller du med? Om du har en bättre idé, var god att kommentera nedan. Och dina test? Passerar de fortfarande?
Nu, i rulla() metod vi har några nummer också: två, noll, 11 och 12.
om ($ roll% 2! = 0)
Det är ganska tydligt. Vi kommer att extrahera det här uttrycket i en metod, men inte i denna handledning. Vi är fortfarande i fasen för förståelse och jakt efter magiska konstanter och strängar. Så hur är det med 11 och 12? De är begravda inom den tredje nivån av om uttalanden. Det är ganska svårt att förstå vad de står för. Kanske om vi tittar på linjerna runt dem.
om ($ this-> placerar [$ this-> currentPlayer]> 11) $ this-> placerar [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12;
Om den aktuella spelarens plats eller position är större än 11, kommer dess position att minskas till den nuvarande minus 12. Detta låter som ett fall när en spelare når slutet av styrelsen eller spelplanen och placeras i sin första placera. Förmodligen placera noll. Eller om vår spelbräda är cirkulär går spelaren över den sista markerade positionen till spelaren i den relativa första positionen. Så 11 kan vara styrelsens storlek.
$ boardSize = 11; om $ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // om ($ this-> placerar [$ this-> currentPlayer]> $ boardSize) $ this-> places [$ this-> currentPlayer] = $ this-> placerar [$ this-> currentPlayer] - 12; // ... // annat // ... // annat // ... // om ($ this-> placerar [$ this-> currentPlayer]> $ boardSize) $ this-> placerar [$ this -> currentPlayer] = $ this-> placerar [$ this-> currentPlayer] - 12; // ... //
Glöm inte att byta 11 på båda ställena inom metoden. Detta kommer tvinga oss att flytta den variabla uppgiften utanför om uttalanden, precis vid första indragningsnivån.
Men om 11 är styrelsens storlek, vad är 12? Vi subtraherar 12 från spelarens nuvarande position, inte 11. Och varför ställer vi inte bara ställningen till noll istället för att subtrahera? Eftersom det skulle göra att våra test misslyckas. Vår tidigare gissning att spelaren kommer att hamna i läge noll efter koden inuti om uttalandet körs, var fel. Låt oss säga en spelare är i tio position och rullar fyra. 14 är större än 11, så subtraktionen kommer att hända. Spelaren kommer att hamna i position 10 + 4-12 = 2.
Detta driver oss mot en annan möjlig namngivning för 11 och 12. Jag tycker att det är lämpligare att ringa 12 $ boardSize. Men vad betyder det för 11? Kanske $ lastPositionOnTheBoard? Lite lång, men det säger åtminstone oss sanningen om den magiska konstanten.
$ lastPositionOnTheBoard = 11; $ boardSize = 12; om ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // om ($ this-> placerar [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this-> currentPlayer] = $ this-> platser [$ this-> currentPlayer] - $ boardSize; // ... // annat // ... // annat // ... // om ($ this-> placerar [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> placerar [$ this -> currentPlayer] = $ this-> placerar [$ this-> currentPlayer] - $ boardSize; // ... //
Jag vet jag vet! Det finns någon koddubbling där. Det är ganska uppenbart, särskilt med resten av koden dold. Men kom ihåg att vi var efter magiska konstanter. Det kommer också att finnas en tid för dubbla kod, men inte just nu.
Jag lämnade en sista magisk konstant i koden. Kan du upptäcka det? Om du tittar på den slutliga koden kommer den att bytas ut, men det skulle naturligtvis vara fusk. Lycka till att hitta det och tack för att du läste.
