Share
I vår tidligere leksjon har vi lært en ny måte å forstå og gjøre kode bedre ved å trekke ut til vi slipper. Mens den opplæringen var en god måte å lære teknikkene på, var det neppe det ideelle eksempelet for å forstå fordelene med det. I denne leksjonen vil vi trekke ut til vi slipper på alle våre trivia spill relaterte kode og vi vil analysere sluttresultatet.
Denne leksjonen vil også konkludere med serien vår om refactoring. Hvis du tror vi savnet noe, vær så snill å kommentere med et forslag til emne. Hvis gode ideer samles, vil jeg fortsette med ekstra opplæringsprogrammer basert på dine ønsker.
Hvilken bedre måte å starte vår artikkel enn ved å ta vår lengste metode og trekke den ut i små biter. Testing først, som vanlig, vil gjøre denne prosedyren ikke bare effektiv, men også morsom.
Som vanlig har du koden som den var da jeg startet denne opplæringen i php_start mappe, mens sluttresultatet er i php mappe.
funksjonen var feilaktig () hvis $ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ Dette-> vesker [$ dette-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spillere [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ winner = $ this-> didPlayerNotWin (); $ Dette-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returner $ vinneren; ellers $ this-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnere sann; ellers $ this-> display-> correctAnswerWithTypo (); $ Dette-> vesker [$ dette-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spillere [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ winner = $ this-> didPlayerNotWin (); $ Dette-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returner $ vinneren;
Denne metoden, wasCorrectlyAnswered (), er vårt første offer.
Som vi lærte fra tidligere leksjoner, er det første trinnet når du endrer arvskoden, å få det under test. Dette kan være en vanskelig prosess. Heldigvis for oss, den wasCorrectlyAnswered () metoden er ganske enkel. Den består av flere if-else uttalelser. Hver gren av koden returnerer en verdi. Når vi har en returverdi, kan vi alltid spekulere på at testingen er mulig. Ikke nødvendig lett, men i det minste mulig.
funksjonstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> isGettingOutOfPenaltyBox = true; $ this-> game-> purses [$ this-> game-> currentPlayer] = Spill :: $ numberOfCoinsToWin; $ Dette-> assertTrue ($ dette-> spill-> wasCorrectlyAnswered ());
Det er ingen bestemt regel om hvilken test som skal skrives først. Vi har nettopp valgt den første utførelsesveien her. Vi hadde faktisk en fin overraskelse, og vi brukte en av de private metodene vi brukte igjen, og vi hentet mange opplæringsprogrammer før. Men vi er ikke ferdige ennå. Alle grønne, så det er på tide å refactoring.
funksjonstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ Dette-> currentPlayerWillLeavePenaltyBox (); $ Dette-> setCurrentPlayerAWinner (); $ Dette-> assertTrue ($ dette-> spill-> wasCorrectlyAnswered ());
Dette er lettere å lese og betydelig mer beskrivende. Du finner de utviste metodene i vedlagte kode.
funksjonstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileNOTBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ Dette-> currentPlayerWillLeavePenaltyBox (); $ Dette-> setCurrentPlayerNotAWinner (); $ Dette-> assertFalse ($ dette-> spill-> wasCorrectlyAnswered ()); Private Funksjon SetCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0;
Vi ventet at dette skulle passere, men det mislykkes. Årsakene er ikke klare i det hele tatt. En nærmere titt på didPlayerNotWin () kan være nyttig.
funksjon didPlayerNotWin () return! ($ this-> purses [$ this-> currentPlayer] == selv :: $ numberOfCoinsToWin);
Metoden returnerer faktisk sant når en spiller ikke vant. Kanskje vi kunne omdøpe vår variabel, men først må tester passere.
privat funksjon settCurrentPlayerAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = Spill :: $ numberOfCoinsToWin; Private Funksjon SetCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0;
På en nærmere inspeksjon kan vi se at vi blandet opp verdiene her. Vår forvirring mellom metodenavnet og det variable navnet gjorde at vi reverserte betingelsene.
privat funksjon settCurrentPlayerAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0; privatfunksjonssettCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = Spill :: $ numberOfCoinsToWin - 1;
Dette fungerer. Mens du analyserer didPlayerNotWin () Vi har også observert at det bruker likestilling for å bestemme vinneren. Vi må sette verdien vår til en mindre fordi verdien økes i produksjonskoden vi tester.
De resterende tre tester er enkle å skrive. De er bare variasjoner av de to første. Du finner dem i vedlagte kode.
Det mest forvirrende problemet er misvisende $ vinneren variabelt navn. Det burde være $ notAWinner.
$ notAWinner = $ this-> didPlayerNotWin (); $ Dette-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnere $ notAWinner;
Vi kan observere at $ notAWinner variabel brukes kun for å returnere en verdi. Kan vi ringe didPlayerNotWin () Metode direkte i returoppgaven?
$ Dette-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnere $ this-> didPlayerNotWin ();
Dette går fortsatt vår enhetstest, men hvis vi kjører våre Golden Master tester, vil de mislykkes med feil med "ikke nok minne". Faktisk gjør endringen at spillet aldri er ferdig.
Det som skjer er at nåværende spiller er oppdatert til neste spiller. Da vi hadde en enkelt spiller, brukte vi alltid den samme spilleren. Det er slik testingen er. Du vet aldri når du oppdager en skjult logikk i vanskelig kode.
Accentsconagua
funksjonstestWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> add ('En annen spiller'); $ Dette-> currentPlayerWillLeavePenaltyBox (); $ Dette-> setCurrentPlayerAWinner (); $ Dette-> assertTrue ($ dette-> spill-> wasCorrectlyAnswered ()); Ved å bare legge til en annen spiller i hvert av våre tester relatert til denne metoden, kan vi sørge for at logikken er dekket. Denne testen vil gjøre det endrede avkastningsoppgaven ovenfor for å mislykkes.
privat funksjon selectNextPlayer () $ this-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Vi kan umiddelbart se at valget av neste spiller er identisk på begge veier av tilstanden. Vi kan flytte den til en egen metode. Navnet vi valgte for denne metoden er selectNextPlayer (). Dette navnet hjelper til med å markere det faktum at gjeldende spillerens verdi vil bli endret. Det antyder også at didPlayerNotWin () kan omdøpe seg i noe som gjenspeiler forholdet til gjeldende spiller.
funksjonen var feilaktig () hvis $ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ Dette-> vesker [$ dette-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spillere [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ Dette-> selectNextPlayer (); returnere $ notAWinner; ellers $ this-> selectNextPlayer (); returnere sant; ellers $ this-> display-> correctAnswerWithTypo (); $ Dette-> vesker [$ dette-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> spillere [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ Dette-> selectNextPlayer (); returnere $ notAWinner;
Koden vår blir kortere og mer uttrykksfulle. Hva kan vi gjøre neste? Vi kunne endre det rare navnet på logoen "ikke vinner" og endre metoden til en positiv logikk i stedet for en negativ. Eller vi kunne fortsette å utvinne og håndtere den negative logiske forvirringen senere. Jeg tror ikke det er en bestemt vei å gå. Så, jeg la det negative logiske problemet som en øvelse for deg, og vi vil fortsette med utvinningen.
funksjonen var korrektAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); ellers return $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Som en tommelfingerregel, prøv å ha en enkelt linje med kode på hver bane i en beslutningslogikk.
Vi hentet hele koden i hver del av vår hvis uttalelse. Dette er et viktig skritt, og noe du alltid bør tenke på. Når du har en beslutningsvei eller en løkke i koden din, bør det bare være en enkelt setning i innsiden av det. Personen som leser denne metoden, sannsynligvis ikke bryr seg om implementeringsdetaljer. Han eller hun vil bryr seg om avgjørelseslogikken, den hvis uttalelse.
funksjonen var korrektAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); returner $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Og hvis vi kan bli kvitt noen ekstra kode, bør vi. Fjerning av ellers og fortsatt holder logikken det samme gjorde vi en liten økonomi. Jeg liker denne løsningen bedre fordi den fremhever hva som er "standard" oppførsel av funksjonen. Koden som er i funksjonens interiør direkte (den siste koden her). De hvis erklæring er den eksepsjonelle funksjonaliteten som er lagt til standard en.
Jeg hørte tanker om at skriveforholdene på denne måten kan skjule det faktum at standard ikke utføres hvis hvis setningen aktiveres. Jeg kan bare være enig med dette, og hvis du foretrekker å beholde ellers Del det for klarhet, vær så snill.
privat funksjon getCorrectlyAnsweredForPlayersInPenaltyBox () if ($ this-> isGettingOutOfPenaltyBox) return $ this-> getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox (); ellers return $ this-> getCorrectlyAnsweredForPlayerStayingInPenaltyBox ();
Vi kan fortsette å trekke ut i våre nyopprettede private metoder. Bruk av samme prinsipp til vår neste betingede erklæring fører til koden ovenfor.
privat funksjon giCurrentUserACoin () $ this-> purses [$ this-> currentPlayer] ++;
Ved å se på våre private metoder getCorrectlyAnsweredForPlayersNotInPenaltyBox () og getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () vi kan umiddelbart observere at en enkel oppgave er duplisert. Denne oppgaven kan være åpenbar for noen som oss som allerede vet hva som er med vesker og mynter, men ikke for nybegynnere. Utvinning av den enkelte linjen i en metode giveCurrentUserACoin () er en god ide.
Det hjelper også med duplisering. Hvis vi i fremtiden endrer måten vi gir spillernes mynter, må vi bare endre kode bare i denne private metoden.
privat funksjon getCorrectlyAnsweredForPlayersNotInPenaltyBox () $ this-> display-> correctAnswerWithTypo (); returnere $ this-> getCorrectlyAnsweredForAPlayer (); privat funksjon getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () $ this-> display-> correctAnswer (); returnere $ this-> getCorrectlyAnsweredForAPlayer (); privat funksjon getCorrectlyAnsweredForAPlayer () $ this-> giveCurrentUserACoin (); $ this-> display-> playerCoins ($ this-> spillere [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ Dette-> selectNextPlayer (); returnere $ notAWinner;
Da er de to korrekt besvarte metodene identiske, bortsett fra at en av dem utdataer noe med en skrivefeil. Vi hentet duplikatkoden og holdt forskjellene i hver metode. Du kan tenke at vi kunne ha brukt den ekstraherte metoden med en parameter i oppringerkoden, og skriv ut en gang normalt og en gang med en skrivefeil. Imidlertid har løsningen foreslått ovenfor en fordel: Det holder adskilt de to konseptene som ikke er i straffeboks og kommer ut av straffeboksen.
Dette avslutter arbeidet med wasCorrectlyAnswered ().
funksjon feilAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> spillere [$ this-> currentPlayer]; $ Dette-> Skjerm> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ Dette-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; returnere sann;
På 11 linjer er denne metoden ikke stor, men absolutt stor. Husker du det magiske nummeret syv pluss minus to undersøkelser? Det står at hjernen vår samtidig kan tenke på 7 + -2 ting. Det vil si, vi har begrenset kapasitet. Så for å forstå og forstå en metode, vil vi logikken inne i den for å passe inn i dette området. Med totalt 11 linjer, og et innhold på 9 linjer, er denne metoden litt begrenset. Du kan hevde at det faktisk er en tom linje og en annen med bare en brace. Det ville bare gjøre det 7 linjer med logikk.
Mens håndtak og mellomrom er korte i rommet, har de en mening for oss. De deler deler av logikken, de har en mening, så vår hjerne må behandle dem. Ja, det er lettere sammenlignet med en komplett linje o sammenligningslogikk, men likevel.
Det er derfor vårt mål for logikklinjer innenfor en metode er 4 linjer. Det er under det minste av den ovennevnte teorien, så både et geni og en middelmådig programmør burde kunne forstå metoden.
$ Dette-> currentPlayer ++; hvis ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Vi har allerede en metode for dette koden, så vi bør bruke den.
funksjon feilAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> spillere [$ this-> currentPlayer]; $ Dette-> Skjerm> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ Dette-> selectNextPlayer (); returnere sant;
Bedre, men skal vi slippe eller fortsette?
$ currentPlayer = $ this-> spillere [$ this-> currentPlayer]; $ Dette-> Skjerm> playerSentToPenaltyBox ($ currentPlayer);
Vi kunne sette inn variabelen fra disse to linjene. $ Dette-> currentPlayer er åpenbart å returnere gjeldende spiller, så det er ikke nødvendig å gjenta logikken. Vi lærer ingenting nytt eller abstrakt ingenting nytt ved å bruke den lokale variabelen.
funksjon feilAnswer () $ this-> display-> incorrectAnswer (); $ Dette-> Skjerm> playerSentToPenaltyBox ($ dette-> spillere [$ dette-> currentPlayer]); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ Dette-> selectNextPlayer (); returnere sant;
Vi er ned til 5 linjer. Alt annet der inne?
$ this-> inPenaltyBox [$ this-> currentPlayer] = true;
Vi kan trekke ut linjen over til egen metode. Det vil bidra til å forklare hva som skjer og isolere logikken om å sende den nåværende spilleren inn i straffeboksen på sitt eget sted.
funksjon feilAnswer () $ this-> display-> incorrectAnswer (); $ Dette-> Skjerm> playerSentToPenaltyBox ($ dette-> spillere [$ dette-> currentPlayer]); $ Dette-> sendCurrentPlayerToPenaltyBox (); $ Dette-> selectNextPlayer (); returnere sant;
Fortsatt 5 linjer, men alle metoder kaller. De to første er å vise ting. De neste to er relatert til vår logikk. Den siste linjen returnerer bare sant. Jeg ser ingen måte å gjøre denne metoden lettere å forstå uten å introdusere kompleksitet av ekstraksjonene vi kan lage, for eksempel ved å trekke ut de to skjermmetodene til en privat. Hvis vi skulle gjøre det, hvor skal den metoden gå? Inn i dette Spill klasse, eller inn i Vise? Jeg tror dette er allerede et for komplekst spørsmål som fortjener å bli vurdert i forhold til vår enkle enkelhet.
La oss gjøre noen statistikk ved å sjekke ut dette flotte verktøyet fra forfatteren 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. Størrelse linjer av kode (LOC) 232 Kommentar linjer av kode (CLOC) 0 (0.00%) Ikke-kommentator Linjer av kode (NCLOC) 232 (100,00%) Logiske linjer av kode (LLOC) 99 (42,67%) Klasser 88 %) Gjennomsnittlig klasse Lengde 88 Minimum Klasse Lengde 88 Maksimal Klasselengde 88 Gjennomsnittlig Metode Lengde 7 Minste Metode Lengde 1 Maksimal Metode Lengde 17 Funksjoner 1 (1.01%) Gjennomsnittlig Funksjonslengde 1 Ikke i klasser eller funksjoner 10 (10,10%) Cyclomatic Complexity Average Complexity per LLOC 0,26 Gjennomsnittlig kompleksitet per klasse 25,00 Minimumsklassekompleksitet 25,00 Maksimal klasse-kompleksitet 25,00 Gjennomsnittlig kompleksitet per metode 3,18 Minste metakompleksitet 1,00 Maksimal metakompleksitet 10,00 Avhengigheter Globale tilganger 0 Globale konstanter 0 (0,00%) Globale variabler 0 (0,00%) Super-globalt Variabler 0 (0,00%) Egenskapstilganger 115 Ikke-statisk 115 (100,00%) Statisk 0 (0,00%) Metodeanrop 21 Ikke-statisk 21 (100,00%) Statisk 0 (0.00%) Struktur Navnegrupper 0 Grensesnitt 0 Egenskaper 0 Klasser 1 Abstrakt Klasser 0 (0,00%) Betongklasser 1 ( 100,00%) Metoder 11 Omfang Ikke-statiske metoder 11 (100,00%) Statiske metoder 0 (0.00%) Synlighet Offentlige metoder 11 (100,00%) Ikke-offentlige metoder 0 (0.00%) Funksjoner 1 Navngitte funksjoner 1 (100,00%) Anonyme funksjoner 0 (0,00%) Konstanter 0 Globale konstanter 0 (0,00%) Klassekonstanter 0 (0,00%)
./ phploc ... / Refactoring \ Legacy \ Code \ - \ Part \ 11 \: \ The \ End \? / Source / trivia / php phploc 2.1-gca70e70 av Sebastian Bergmann. Størrelse linjer av kode (LOC) 371 Kommentar linjer av kode (CLOC) 0 (0.00%) Ikke-kommentator Linjer av kode (NCLOC) 371 (100,00%) Logiske linjer av kode (LLOC) 151 (40,70%) Klasser 145 %) Gjennomsnittlig klasse lengde 36 Minimum klasse lengde 8 Maksimum klasse lengde 89 Gjennomsnittlig metode Lengde 2 Minimum metode Lengde 1 Maksimal metode Lengde 14 Funksjoner 0 (0,00%) Gjennomsnittlig Funksjonslengde 0 Ikke i klasser eller funksjoner 6 (3,97%) Syklomatisk kompleksitet Gjennomsnittlig kompleksitet per LLOC 0,15 Gjennomsnittlig kompleksitet per klasse 6,50 Minimumsklasse-kompleksitet 1,00 Maksimal klasse-kompleksitet 17,00 Gjennomsnittlig kompleksitet per metode 1,46 Minimum metodekompleksitet 1,00 Maksimal metakompleksitet 10,00 Avhengigheter Globale tilganger 0 Globale konstanter 0 (0,00%) Globale variabler 0 (0,00%) Super-globalt Variabler 0 (0,00%) Egenskapstilgang 96 Ikke-statisk 94 (97,92%) Statisk 2 (2,08%) Metodeanrop 74 Ikke-statisk 74 (100,00%) Statisk 0 (0,00%) Struktur Navnegrupper 0 Grensesnitt 1 Egenskaper 0 Klasser 3 Abstrakt Klasser 0 (0,00%) Betongklasser 3 (100,00 %) Metoder 59 Omfang Ikke-statiske metoder 59 (100,00%) Statiske metoder 0 (0.00%) Synlighet Offentlige metoder 35 (59.32%) Ikke-offentlige metoder 24 (40,68%) Funksjoner 0 Oppkalte funksjoner 0 (0.00%) Anonyme funksjoner 0 (0,00%) Konstanter 3 Globale Konstanter 0 (0,00%) Klassekonstanter 3 (100,00%)
Brute data er like god som vi kan forstå og analysere.
Antallet logiske kodelinjer økte ganske betydelig fra 99 til 151. Men dette tallet bør ikke lure deg til å tro at vår kode ble mer kompleks. Dette er en naturlig tendens til veldefinert kode, på grunn av økningen i antall metoder og samtaler til dem.
Så snart vi ser på gjennomsnittlig klasselengde kan vi se en dramatisk nedgang i kodelinjer, fra 88 til 36.
Og det er bare utrolig hvordan metodelengden gikk ned fra et gjennomsnitt på syv linjer til bare bare to linjer med kode.
Mens antall linjer er en god indikator for kodevolum per måleenhet, er den reelle gevinst i analysene av syklomatisk kompleksitet. Hver gang vi tar en beslutning i vår kode, øker vi den syklomatiske kompleksiteten. Når vi kjedet hvis uttalelser den ene i den andre, øker den syklomatiske kompleksiteten til denne metoden eksponentielt. Våre fortsatte utvinninger førte til metoder med bare en enkelt beslutning i dem, og dermed reduserte deres gjennomsnittlige kompleksitet per metode fra 3,18 til 1,00. Du kan lese dette som "våre refactored metoder er 3,18 ganger enklere enn den opprinnelige koden". På klassenivå er drop i kompleksitet enda mer fantastisk. Det vinner ned fra 25.00 til 6.50.
Vi vil. Det er det. Slutten av serien. Ta gjerne uttrykk for dine meninger, og hvis du tror vi savnet noen refactoring emner, spør etter dem i kommentarene nedenfor. Hvis de er interessante, vil jeg forvandle dem i ekstra deler av denne serien.
Takk for din udelte oppmerksomhet.
