Share
Gammel kode. Ugyldig kode. Komplisert kode. Spaghetti kode. Jibberish tull. I to ord, Legacy Code. Dette er en serie som vil hjelpe deg med å jobbe og håndtere det.
Vi møtte vår eldre kildekode først i vår tidligere leksjon. Så kjørte vi koden for å danne en mening om sin grunnleggende funksjonalitet og opprettet en Golden Master testpakke for å ha et sikkert sikkerhetsnett for fremtidige endringer. Vi vil fortsette å jobbe med denne koden, og du finner den under trivia / php_start mappe. Den andre mappen trivia / php_start er med denne leksjonens ferdige kode.
Tiden for de første endringene har kommet, og hvilken bedre måte å forstå en vanskelig kodebase enn å begynne å trekke ut magiske konstanter og strenger i variabler? Disse tilsynelatende enkle oppgaver vil gi oss større og noen ganger uventede innsikt i den indre virkningen av arvskoden. Vi må finne ut hva hensikten med den opprinnelige kodeforfatteren er og finne de riktige navnene til kodene som vi aldri har sett før.
Magiske strenger er strenger brukt direkte i forskjellige uttrykk, uten å bli tilordnet en variabel. Denne typen streng hadde en spesiell betydning for den opprinnelige forfatteren av koden, men i stedet for å tilordne dem til en velkalt variabel, syntes forfatteren at betydningen av strengen var åpenbar nok.
La oss begynne med å se på vår Game.php og prøv å identifisere strenge. Hvis du bruker en IDE (og du burde) eller et smartere tekstredigeringsprogram som er i stand til å markere kildekoden, vil det være enkelt å spore strenger. Her er et bilde av hvordan koden ser ut på skjermen min.
Alt med oransje er en streng. Å finne hver streng i kildekoden vår er veldig enkel på denne måten. Så sørg for at utheving støttes og aktiveres i redigeringsprogrammet, uansett hva søknaden din er.
Den første oransje delen i vår kode er umiddelbart på linje tre. Strengen inneholder imidlertid bare en nylinjetegn. Dette burde være åpenbart nok til min mening, så vi kan fortsette.
Når det gjelder å bestemme hva som skal trekkes ut og hva som skal holdes uendret, er det få tommelfingerbilder, men på slutten er det din faglige mening som må seire. Basert på det, må du bestemme hva du skal gjøre med hvert stykke kode du analyserer.
Accentsconagua
for ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, $ this-> createRockQuestion ($ i)); funksjon createRockQuestion ($ index) return "Rock Question". $ Index; Så la oss analysere linjer 32 til 42, koden du kan se ovenfor. For pop-, vitenskaps- og sportsspørsmål er det bare en enkel sammenkobling. Handlingen til å komponere strengen for et bergspørsmål blir imidlertid hentet inn i en metode. Er disse konkatenasjoner og strenger etter din mening klart nok, slik at vi kan holde dem alle inni vår forløp? Eller tror du å utvinne alle strengene i deres metoder, ville rettferdiggjøre eksistensen av disse metodene? Hvis ja, hvordan vil du nevne disse metodene?
Uansett svaret, må vi endre koden. Det er på tide å sette vår Golden Master på jobb og skrive vår test som faktisk kjører og sammenligner vår kode med eksisterende innhold.
klassen GoldenMasterTest utvider PHPUnit_Framework_TestCase private $ gmPath; funksjon setUp () $ this-> gmPath = __DIR__. '/Gm.txt'; funksjonstestGenerateOutput () $ times = 20000; $ this-> generateMany ($ ganger, $ this-> gmPath); funksjonstestOutputMatchesGoldenMaster () $ times = 20000; $ actualPath = '/tmp/actual.txt'; $ this-> generateMany ($ ganger, $ 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 funksjon generere mange ($ ganger, $ filnavn) ... privat funksjon generereutgang ($ seed) ...
Vi opprettet en annen test for å sammenligne utgangene, sørget for testGenerateOutput () bare genererer utgangen en gang og gjør ingenting annet. Vi flyttet også den gyldne masterutdatafilen "Gm.txt" inn i gjeldende katalog fordi "/ Tmp" kan bli slettet når systemet starter opp igjen. For våre faktiske resultater kan vi fortsatt bruke det. På de fleste UNIX-lignende systemer "/ Tmp" er montert i RAM, så det er mye raskere enn filsystemet. Hvis du gjorde det bra, bør testene passere uten problemer.
Det er veldig viktig å huske å merke vår generatortest som "hoppet over" for fremtidige endringer. Hvis du føler deg mer komfortabel med å kommentere eller til og med slette den helt, vennligst gjør det. Det er viktig at vår Gyldne Mester ikke vil endre seg når vi bytter kode. Det ble generert en gang, og vi ønsker ikke å endre det, noensinne, slik at vi kan være sikre på at vår nylig genererte kode alltid sammenligner med originalen. Hvis du føler deg mer komfortabel gjør en sikkerhetskopi av det, vennligst fortsett å gjøre det.
funksjon testGenerateOutput () $ this-> markTestSkipped (); $ ganger = 20000; $ this-> generateMany ($ ganger, $ this-> gmPath);
Jeg vil bare markere testen som hoppet over. Dette vil sette vårt testresultat til "gul", som betyr at alle tester passerer, men noen er hoppet over eller merket som ufullstendige.
Med tester på plass, kan vi begynne å endre kode. I min faglige mening kan alle strengene holdes inne i til sløyfe. Vi tar koden fra createRockQuestion () metode, flytte den inne i til sløyfe, og slett metoden helt. Denne refactoring kalles Inline Metode.
"Sett metoden kropp inn i kroppen til sine innringere og fjern metoden." ~ Martin Fowler
Det er et bestemt sett av trinn for å gjøre denne typen refactoring, som definert av Marting Fowler i Refactoring: Forbedre utformingen av eksisterende kode:
Vi har ikke underklasser som strekker seg Spill, så det første trinnet validerer.
Det er bare en eneste bruk av metoden vår, inne i til sløyfe.
funksjon __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 (); for ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); funksjon createRockQuestion ($ index) return "Rock Question". $ Index; Vi legger koden fra createRockQuestion () inn i det til sløyfe i konstruktøren. Vi har fortsatt vår gamle kode. Det er nå på tide å kjøre testen vår.
Våre tester går forbi. Vi kan slette vår createRockQuestion () metode.
funksjon __construct () // ... // for ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); funksjon isPlayable () return ($ this-> howManyPlayers ()> = 2); Til slutt bør vi kjøre testene våre igjen. Hvis vi savnet et kall til en metode, vil de mislykkes.
De skal passere igjen. Gratulerer! Vi er ferdige med vår første refactoring.
Strenger i metodene Legg til() og rulle () brukes kun til å sende dem ut ved hjelp av echoln () metode. stille spørsmål() Sammenligner strenge til kategorier. Dette virker også akseptabelt. currentCategory () På den annen side returneres strenger basert på et tall. I denne metoden er det mange dupliserte strenger. Endring av en kategori, bortsett fra at Rock ville kreve å endre navnet på tre steder, bare i denne metoden.
funksjon currentCategory () if ($ this-> plasserer [$ this-> currentPlayer] == 0) return "Pop"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 4) return "Pop"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 8) return "Pop"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 1) return "Science"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 5) returner "Science"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 9) return "Science"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 2) return "Sports"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 6) return "Sports"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 10) return "Sports"; returnere "rock";
Jeg tror vi kan gjøre det bedre. Vi kan bruke en refactoringmetode som heter Sett inn lokal variabel og eliminere duplisering. Følg disse retningslinjene:
funksjon currentCategory () $ popCategory = "Pop"; $ scienceCategory = "Science"; $ sportCategory = "Sports"; $ rockCategory = "Rock"; hvis ($ this-> plasserer [$ this-> currentPlayer] == 0) return $ popCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 4) return $ popCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 8) return $ popCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 1) return $ scienceCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 5) return $ scienceCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 9) return $ scienceCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 2) return $ sportCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 6) return $ sportCategory; hvis ($ this-> plasserer [$ this-> currentPlayer] == 10) return $ sportCategory; returner $ rockCategory;
Dette er mye bedre. Du har sikkert allerede observert noen fremtidige forbedringer som vi kunne gjøre med vår kode, men vi er bare i begynnelsen av arbeidet vårt. Det er fristende å fikse alt du finner umiddelbart, men vær så snill å ikke. Mange ganger, spesielt før koden er godt forstått, kan fristende endringer føre til døde ender eller enda mer ødelagt kode. Hvis du tror det er en sjanse, vil du glemme ideen din, bare noter den på et notat eller opprett en oppgave i prosjektstyringsprogramvaren. Nå må vi fortsette med våre strengrelaterte problemer.
I resten av filen er alle strenger utdatarelaterte, sendt inn echoln (). For tiden vil vi forlate dem uberørt. Endring av dem vil påvirke utskrift og leveranse av logikken i søknaden vår. De er en del av presentasjonslaget blandet med forretningslogikk. Vi vil håndtere å skille forskjellige bekymringer i en fremtidig leksjon.
Magiske konstanter er veldig mye som magiske strenger, men med verdier. Disse verdiene kan være boolske verdier eller tall. Vi vil konsentrere mest på tall som brukes i hvis uttalelser eller komme tilbake uttalelser eller andre uttrykk. Hvis disse tallene har en uklar betydning, må vi trekke dem ut i variabler eller metoder.
include_once __DIR__. '/Game.php'; $ NotAWinner; $ aGame = nytt spill (); $ AGame-> legge til ( "Chet"); $ AGame-> legge til ( "Pat"); $ AGame-> legge til ( "Sue"); gjør $ aGame-> rull (rand (0, 5) + 1); hvis (rand (0, 9) == 7) $ notAWinner = $ aGame-> wrongAnswer (); ellers $ notAWinner = $ aGame-> wasCorrectlyAnswered (); mens ($ notAWinner);
Vi begynner med GameRunner.php denne gangen og vårt første fokus er rull() metode som får noen tilfeldige tall. Forrige forfatter bryr seg ikke om å gi disse tallene en mening. Kan vi? Hvis vi analyserer koden:
rand (0, 5) + 1
Det vil returnere et tall mellom ett og seks. Den tilfeldige delen returnerer et tall mellom null og fem som vi alltid legger til en. Så det er sikkert mellom ett og seks. Nå må vi vurdere konteksten i søknaden vår. Vi utvikler et trivia spill. Vi vet at det er en slags brett som våre spillere må bevege seg på. Og for å gjøre det, må vi rulle terningene. En dør har seks ansikter, og den kan produsere tall mellom ett og seks. Det virker som et rimelig fradrag.
$ terning = rand (0, 5) + 1; $ AGame-> roll ($ terninger);
Er det ikke bra? Vi brukte igjen Introduce Local Variable refactoring konseptet. Vi heter vår nye variabel $ terninger og det representerer tilfeldig tall som genereres mellom ett og seks. Dette gjorde også vår neste setning lyden veldig naturlig: Spill, kast terninger.
Kjørte du testene dine? Jeg nevnte det ikke, men vi må kjøre dem så ofte som mulig. Hvis du ikke har det, ville dette være en god tid å løpe dem. Og de skal passere.
Så dette var et tilfelle av ingenting annet enn å bare utveksle et tall med en variabel. Vi tok et helt uttrykk som representerte et tall og hentet det inn i en variabel. Dette kan teknisk sett betraktes som et Magic Constant-tilfelle, men ikke et rent tilfelle. Hva med vårt neste tilfeldige uttrykk?
hvis (rand (0, 9) == 7)
Dette er vanskeligere. Hva er null, ni og syv i det uttrykket? Kanskje vi kan nevne dem. Ved første øyekast har jeg ingen gode ideer for null og ni, så la oss prøve syv. Hvis tallet som returneres av vår tilfeldige funksjon er lik syv, kommer vi inn i den første grenen av hvis uttalelse som gir et feil svar. Så kanskje våre syv kunne bli navngitt $ wrongAnswerId.
$ wrongAnswerId = 7; hvis (rand (0, 9) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); ellers $ notAWinner = $ aGame-> wasCorrectlyAnswered ();
Våre tester går fortsatt forbi og koden er noe mer uttrykksdyktig. Nå som vi klarte å nevne nummeret vårt sju, setter det betingelsene i en annen sammenheng. Vi kan tenke på noen anstendige navn for null og ni også. De er bare parametere til rand (), så variablene vil sannsynligvis bli kalt min-noe og maks-noe.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; hvis (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> feilAnswer (); ellers $ notAWinner = $ aGame-> wasCorrectlyAnswered ();
Nå er det uttrykksfulle. Vi har et minimum svar ID, maksimalt ett og annet for feil svar. Mysteriet løst.
gjør $ terning = rand (0, 5) + 1; $ AGame-> roll ($ terninger); $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; hvis (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> feilAnswer (); ellers $ notAWinner = $ aGame-> wasCorrectlyAnswered (); mens ($ notAWinner);
Men vær oppmerksom på at all koden er inne i a gjør mens sløyfe. Trenger vi å omfordele svar-ID-variablene hver gang? Jeg tror ikke. La oss prøve å flytte dem ut av løkken og se om våre tester går forbi.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; gjør $ terning = rand (0, 5) + 1; $ AGame-> roll ($ terninger); hvis (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> feilAnswer (); ellers $ notAWinner = $ aGame-> wasCorrectlyAnswered (); mens ($ notAWinner);
Ja. Tester passerer som dette også.
Det er på tide å bytte til Game.php og se etter Magic Constants der også. Hvis du har kodeutheving, har du sikkert konstanter fremhevet i noen lys farger. Mine er blå og de er ganske enkle å få øye på.
Finne den magiske konstanten 50 i det til loop var ganske enkelt. Og hvis vi ser på hva koden gjør, kan vi oppdage det inni til sløyfe, elementer blir presset til flere arrays. Så vi har en slags lister, hver med 50 elementer. Hver liste representerer en spørrekategori og variablene er faktisk klassefeltene definert ovenfor som arrayer.
$ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array ();
Så, hva kan 50 representere? Jeg vedder på at du allerede har noen ideer. Navngivning er en av de vanskeligste oppgavene i programmering, hvis du har mer enn en ide, og du er usikker på hvilken du skal velge, ikke skam deg. Jeg har også forskjellige navn i hodet mitt, og jeg vurderer mulighetene for å velge det beste, selv når du skriver dette avsnittet. Jeg tror vi kan gå med et konservativt navn for 50. Noe i tråd med$ questionsInEachCategory eller $ categorySize eller noe lignende.
$ categorySize = 50; for ($ i = 0; $ i < $categorySize; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); Det ser anstendig ut. Vi kan beholde det. Og testene går selvfølgelig forbi.
funksjon isPlayable () return ($ this-> howManyPlayers ()> = 2);
Hva er to? Jeg er sikker på at dette svaret er klart for deg. Det er enkelt:
funksjon isPlayable () $ minimumNumberOfPlayers = 2; returnere ($ this-> howManyPlayers ()> = $ minimumNumberOfPlayers);
Er du enig? Hvis du har en bedre ide, gjerne kommentere nedenfor. Og dine tester? Går de fortsatt forbi?
Nå, i rull() metode vi har noen tall også: to, null, 11 og 12.
hvis ($ roll% 2! = 0)
Det er ganske klart. Vi vil trekke ut uttrykket i en metode, men ikke i denne opplæringen. Vi er fortsatt i fasen av forståelse og jakt etter magiske konstanter og strenger. Så hva med 11 og 12? De er begravet inne i det tredje nivået av hvis uttalelser. Det er ganske vanskelig å forstå hva de står for. Kanskje hvis vi ser på linjene rundt dem.
hvis ($ this-> plasserer [$ this-> currentPlayer]> 11) $ this-> plasserer [$ this-> currentPlayer] = $ this-> plasserer [$ this-> currentPlayer] - 12;
Hvis den nåværende spillers plass eller posisjon er større enn 11, blir posisjonen redusert til den nåværende en minus 12. Dette høres ut som et tilfelle av når en spiller når slutten av brettet eller spillefeltet, og den blir reposisjonert i sin første stilling. Sannsynligvis posisjon null. Eller, hvis vårt spillbrett er sirkulært, går den siste merkede posisjonen til spilleren i den relative første posisjonen. Så 11 kan være styrets størrelse.
$ boardSize = 11; hvis ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // hvis ($ this-> plasserer [$ this-> currentPlayer]> $ boardSize) $ this-> plasserer [$ this-> currentPlayer] = $ this-> plasserer [$ this-> currentPlayer] - 12; // ... // else // ... // else // ... // if ($ this-> plasserer [$ this-> currentPlayer]> $ boardSize) $ this-> plasserer [$ dette -> currentPlayer] = $ this-> plasserer [$ this-> currentPlayer] - 12; // // //
Ikke glem å bytte 11 på begge steder i metoden. Dette vil tvinge oss til å flytte den variable oppgaven utenfor hvis uttalelser, rett ved første inntrengningsnivå.
Men hvis 11 er styrets størrelse, hva er 12? Vi trekker 12 fra spillerens nåværende stilling, ikke 11. Og hvorfor stiller vi ikke stillingen til null i stedet for å trekke? Fordi det ville gjøre at våre tester mislykkes. Vår forrige gjetning at spilleren vil ende opp i posisjon null etter koden inne i hvis setningen er kjørt, var feil. La oss si at en spiller er i posisjon ti og ruller en fire. 14 er større enn 11, så subtraksjonen vil skje. Spilleren vil ende opp i posisjon 10 + 4-12 = 2.
Dette driver oss mot en annen mulig navngiving for 11 og 12. Jeg synes det er mer hensiktsmessig å ringe 12 $ boardSize. Men hva forlater det for 11? Kan være $ lastPositionOnTheBoard? Litt lenge, men i det minste forteller det oss sannheten om den magiske konstanten.
$ lastPositionOnTheBoard = 11; $ boardSize = 12; hvis ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // hvis ($ this-> plasserer [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this-> currentPlayer] = $ this-> steder [$ this-> currentPlayer] - $ boardSize; // ... // else // ... // else // ... // if ($ this-> plasserer [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> plasserer [$ dette -> currentPlayer] = $ this-> plasserer [$ this-> currentPlayer] - $ boardSize; // // //
Jeg vet jeg vet! Det er noen kode duplisering der. Det er ganske opplagt, spesielt med resten av koden skjult. Men vær så snill å huske at vi var etter magiske konstanter. Det vil være en tid for duplikatkode også, men ikke akkurat nå.
Jeg forlot en siste magisk konstant i koden. Kan du få øye på det? Hvis du ser på den endelige koden, vil den bli erstattet, men selvfølgelig ville det bli utro. Lykke til å finne det og takk for å lese.
