În lecția noastră anterioară am învățat o nouă modalitate de a înțelege și de a face codul mai bun prin extragerea până când vom renunța. În timp ce tutorialul a fost o modalitate bună de a învăța tehnicile, nu a fost greu exemplul ideal pentru a înțelege beneficiile acestuia. În această lecție vom extrage până când vom renunța la toate codul jocului nostru legate de trivia și vom analiza rezultatul final.
Această lecție va încheia, de asemenea, seriile noastre despre refactorizare. Dacă credeți că am pierdut ceva, nu ezitați să comentați cu un subiect propus. Dacă se vor aduna idei bune, voi continua cu tutoriale suplimentare pe baza solicitărilor dumneavoastră.
Ce modalitate mai bună de a începe articolul nostru decât prin a lua cea mai lungă metodă și extrageți-o în bucăți mici. Testarea mai întâi, ca de obicei, va face această procedură nu numai eficientă, ci și distractivă.
Ca de obicei, aveți codul ca atunci când am început acest tutorial în php_start
, în timp ce rezultatul final este în php
pliant.
(); if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ This-> posete [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> jucători [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ câștigător = $ this-> didPlayerNotWin (); $ This-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; retur $ câștigător; altceva $ this-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return true; altceva $ this-> display-> correctAnswerWithTypo (); $ This-> posete [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> jucători [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ câștigător = $ this-> didPlayerNotWin (); $ This-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; retur $ câștigător;
Aceasta metoda, wasCorrectlyAnswered ()
, este prima noastră victimă.
După cum am învățat din lecțiile anterioare, primul pas în momentul modificării codului vechi este de a face testul. Acesta poate fi un proces dificil. Din fericire pentru noi, wasCorrectlyAnswered ()
metoda este destul de simplă. Este compus din mai multe if-else
declarații. Fiecare ramură a codului returnează o valoare. Când avem o valoare de returnare, putem întotdeauna specula că testarea este realizabilă. Nu este necesar ușor, dar cel puțin posibil.
funcția testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> isGettingOutOfPenaltyBox = true; $ this-> joc-> poșete [$ this-> game-> currentPlayer] = Joc :: $ numberOfCoinsToWin; $ This-> assertTrue ($ this-> joc-> wasCorrectlyAnswered ());
Nu există o regulă clară asupra testului pe care să-l scrieți mai întâi. Tocmai am ales prima cale de execuție aici. Am avut de fapt o surpriză plăcută și am reușit să folosim una dintre metodele private pe care le-am extras mai înainte de multe tutoriale. Dar nu am terminat încă. Toate verde, deci este timpul pentru refactorizare.
funcția testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ This-> currentPlayerWillLeavePenaltyBox (); $ This-> setCurrentPlayerAWinner (); $ This-> assertTrue ($ this-> joc-> wasCorrectlyAnswered ());
Acest lucru este mai ușor de citit și mult mai descriptiv. Puteți găsi metodele extrase în codul atașat.
funcția testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileNOTBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ This-> currentPlayerWillLeavePenaltyBox (); $ This-> setCurrentPlayerNotAWinner (); $ This-> assertFalse ($ this-> joc-> wasCorrectlyAnswered ()); funcția privată setCurrentPlayerNotAWinner () $ this-> joc-> poșete [$ this-> game-> currentPlayer] = 0;
Ne așteptam ca acest lucru să treacă, dar nu reușește. Motivele nu sunt deloc clare. O privire mai atentă didPlayerNotWin ()
poate fi de ajutor.
funcția didPlayerNotWin () retur! ($ this-> purses [$ this-> currentPlayer] == auto :: $ numberOfCoinsToWin);
Metoda se întoarce de fapt adevărată atunci când un jucător nu a câștigat. Poate că am putea redenumi variabila noastră, dar mai întâi, testele trebuie să treacă.
funcția privată setCurrentPlayerAWinner () $ this-> joc-> portofele [$ this-> game-> currentPlayer] = Joc :: $ numberOfCoinsToWin; funcția privată setCurrentPlayerNotAWinner () $ this-> joc-> poșete [$ this-> game-> currentPlayer] = 0;
La o inspecție mai atentă putem vedea că am amestecat valorile de aici. Confuzia dintre numele metodei și numele variabilei ne-a făcut să inversăm condițiile.
funcția privată setCurrentPlayerAWinner () $ this-> joc-> portofele [$ this-> game-> currentPlayer] = 0; funcția privată setCurrentPlayerNotAWinner () $ this-> joc-> portofele [$ this-> game-> currentPlayer] = Joc :: $ numberOfCoinsToWin - 1;
Acest lucru funcționează. În timp ce analizează didPlayerNotWin ()
am observat de asemenea că foloseste egalitatea pentru a determina câștigătorul. Trebuie să ne stabilim valoarea la o valoare mai mică, deoarece valoarea este crescută în codul de producție pe care îl testăm.
Celelalte trei teste sunt simple de scris. Ele sunt doar variații ale primelor două. Le puteți găsi în codul atașat.
Problema cea mai confuză este înșelătoare $ câștigător
numele variabilei. Ar trebui să fie $ notAWinner
.
$ notAWinner = $ aceasta-> didPlayerNotWin (); $ This-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return $ notAWinner;
Putem observa că $ notAWinner
variabila este folosită numai pentru a returna o valoare. Am putea să sunăm didPlayerNotWin ()
direct în declarația de returnare?
$ This-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return $ this-> didPlayerNotWin ();
Acest lucru trece încă testul unitar, dar dacă executăm testele Golden Master, acestea vor eșua cu eroare de "insuficientă memorie". De fapt, schimbarea face ca jocul să nu se termine niciodată.
Ce se întâmplă este faptul că actualul jucător este actualizat la următorul jucător. Pe măsură ce aveam un singur jucător, reutiliam mereu același jucător. Așa este testarea. Niciodată nu știți când descoperiți o logică ascunsă în cod dificil.
funcția testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> add ("Un alt jucător"); $ This-> currentPlayerWillLeavePenaltyBox (); $ This-> setCurrentPlayerAWinner (); $ This-> assertTrue ($ this-> joc-> wasCorrectlyAnswered ());
Prin adăugarea unui alt jucător în fiecare dintre testele noastre legate de această metodă, putem asigura că logica este acoperită. Acest test va face ca declarația de returnare modificată de mai sus să nu reușească.
funcția privată selectNextPlayer () $ this-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Putem observa imediat că selecția următorului jucător este identică pe ambele căi ale condiției. O putem muta într-o metodă proprie. Numele pe care l-am ales pentru această metodă este selectNextPlayer ()
. Acest nume ajută la evidențierea faptului că valoarea actuală a jucătorului va fi modificată. De asemenea, sugerează acest lucru didPlayerNotWin ()
ar putea fi redenumit în ceva care reflectă relația cu actualul jucător.
(); if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ This-> posete [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> jucători [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ aceasta-> didCurrentPlayerNotWin (); $ This-> selectNextPlayer (); returnați $ notAWinner; altceva $ this-> selectNextPlayer (); return true; altceva $ this-> display-> correctAnswerWithTypo (); $ This-> posete [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> jucători [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ aceasta-> didCurrentPlayerNotWin (); $ This-> selectNextPlayer (); returnați $ notAWinner;
Codul nostru devine mai scurt și mai expresiv. Ce am putea face în continuare? Am putea schimba numele ciudat al logicii "nu câștigătoare" și să schimbăm metoda într-o logică pozitivă în loc de una negativă. Sau am putea continua să extragem și să ne ocupăm de confuzia logică negativă mai târziu. Nu cred că există un mod sigur de a merge. Deci, voi lasa problema logica negativa ca un exercitiu pentru tine si vom continua cu extragerea.
funcția a fost corect repartizată () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); altceva return $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Ca regulă generală, încercați să aveți o singură linie de cod pe fiecare cale a unei logici de decizie.
Am extras întregul bloc de cod în fiecare parte a noastră dacă
afirmație. Acesta este un pas important și trebuie să vă gândiți întotdeauna la asta. Când aveți o cale de decizie sau o buclă în codul dvs., în interiorul acestuia ar trebui să existe doar o singură declarație. Persoana care citește această metodă nu va avea mare grijă de detaliile implementării. El sau ea va pasa de logica deciziei, dacă
afirmație.
funcția a fost corect repartizată () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); retur $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Și dacă putem scăpa de orice cod suplimentar, ar trebui. Scoaterea altfel
și păstrând totodată logica la fel am făcut puțină economie. Îmi place mai bine această soluție deoarece evidențiază comportamentul "implicit" al funcției. Codul care se află direct în interiorul funcției (ultima linie de cod aici). dacă
declarația este funcționalitatea excepțională adăugată la cea implicită.
Am auzit raționamentele că scrierea condițiilor în acest fel poate ascunde faptul că implicit nu se execută dacă dacă
declarația se activează. Nu pot decât să fiu de acord cu acest lucru și dacă preferați să păstrați altfel
o parte pentru claritate, vă rugăm să faceți acest lucru.
funcția privată getCorrectlyAnsweredForPlayersInPenaltyBox () if ($ this-> isGettingOutOfPenaltyBox) retur $ this-> getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox (); altceva return $ this-> getCorrectlyAnsweredForPlayerStayingInPenaltyBox ();
Putem continua să extragem în interiorul noilor metode private create. Aplicarea aceluiași principiu la următoarea declarație condiționată duce la codul de mai sus.
funcția privată giveCurrentUserACoin () $ this-> purses [$ this-> currentPlayer] ++;
Privind metodele noastre private getCorrectlyAnsweredForPlayersNotInPenaltyBox ()
și getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox ()
putem observa imediat că o atribuire simplă este dublată. Această atribuire poate fi evidentă pentru cineva ca noi care știe deja ce este cu poștalele și monedele, dar nu pentru un nou venit. Extragerea acelei singure linii într-o metodă giveCurrentUserACoin ()
este o idee bună.
Ajută și la duplicare. Dacă în viitor schimbăm modul în care dăm monedele jucătorilor, va trebui să schimbăm codul doar în cadrul acestei metode private.
funcția privată getCorrectlyAnsweredForPlayersNotInPenaltyBox () $ this-> display-> correctAnswerWithTypo (); returnați $ this-> getCorrectlyAnsweredForAPlayer (); funcția privată getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () $ this-> display-> correctAnswer (); returnați $ this-> getCorrectlyAnsweredForAPlayer (); funcția privată getCorrectlyAnsweredForAPlayer () $ this-> giveCurrentUserACoin (); $ this-> display-> playerCoins ($ this-> jucători [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ aceasta-> didCurrentPlayerNotWin (); $ This-> selectNextPlayer (); returnați $ notAWinner;
Apoi cele două metode de răspuns corect sunt identice, cu excepția faptului că unul dintre ele scoate ceva cu o tipografie. Am extras codul duplicat și am păstrat diferențele în fiecare metodă. S-ar putea să vă gândiți că am fi putut folosi metoda extrasă cu un parametru în codul apelantului și ieșirea o dată în mod normal și o singură dată cu o greșeală. Cu toate acestea, soluția propusă mai sus are un avantaj: păstrează separat cele două concepte de a nu se află în penalizare și de a ieși din caseta de penalizare.
Aceasta încheie lucrările wasCorrectlyAnswered ()
.
funcția greșitAnswer () $ this-> display-> incorectAnswer (); $ currentplayer = $ this-> jucători [$ this-> currentPlayer]; $ This-> Display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> înPenaltyBox [$ this-> currentPlayer] = adevărat; $ This-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return true;
La 11 linii această metodă nu este imensă, dar cu siguranță mare. Vă amintiți numărul magic de șapte plus minus două cercetări? Afirmă că creierul nostru poate gândi simultan despre 7 + 2 lucruri. Asta înseamnă că avem o capacitate limitată. Deci, pentru a înțelege cu ușurință și pe deplin o metodă, dorim ca logica din interiorul ei să se încadreze în acel interval. Cu un total de 11 linii și un conținut de 9 linii, această metodă este un fel de limită. Puteți susține că de fapt există o linie goală și alta cu doar o bretele. Asta ar face doar 7 linii de logică.
În timp ce brațele și spațiile sunt scurte în spațiu, ele au un sens pentru noi. Ele separă părți ale logicii, ele au un sens, așa că creierul nostru trebuie să le proceseze. Da, este mai ușor comparativ cu o logică completă de comparare, dar totuși.
De aceea, ținta noastră pentru linii de logică în cadrul unei metode este de 4 linii. Aceasta este sub minimul teoriei de mai sus, deci atât un geniu, cât și un programator mediocru ar trebui să fie capabili să înțeleagă metoda.
$ This-> currentPlayer ++; dacă ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Avem deja o metodă pentru acest cod de cod, așa că ar trebui să o folosim.
funcția greșitAnswer () $ this-> display-> incorectAnswer (); $ currentplayer = $ this-> jucători [$ this-> currentPlayer]; $ This-> Display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> înPenaltyBox [$ this-> currentPlayer] = adevărat; $ This-> selectNextPlayer (); return true;
Mai bine, dar ar trebui să renunțăm sau să continuăm?
$ currentplayer = $ this-> jucători [$ this-> currentPlayer]; $ This-> Display-> playerSentToPenaltyBox ($ currentPlayer);
Am putea introduce variabila din aceste două linii. $ This-> currentPlayer
se întoarce în mod evident jucătorul curent, deci nu este nevoie să repetați logica. Nu aflăm nimic nou sau abstract nimic nou prin utilizarea variabilei locale.
funcția greșitAnswer () $ this-> display-> incorectAnswer (); $ This-> Display-> playerSentToPenaltyBox ($ this-> jucători [$ this-> currentPlayer]); $ this-> înPenaltyBox [$ this-> currentPlayer] = adevărat; $ This-> selectNextPlayer (); return true;
Suntem la 5 linii. Orice altceva acolo?
$ this-> înPenaltyBox [$ this-> currentPlayer] = adevărat;
Putem extrage linia de mai sus în metoda proprie. Aceasta va explica ceea ce se întâmplă și va izola logica despre trimiterea jucătorului curent în caseta de penalizare în locul său.
funcția greșitAnswer () $ this-> display-> incorectAnswer (); $ This-> Display-> playerSentToPenaltyBox ($ this-> jucători [$ this-> currentPlayer]); $ This-> sendCurrentPlayerToPenaltyBox (); $ This-> selectNextPlayer (); return true;
Încă 5 linii, dar toate apelurile metodice. Primele două prezintă lucruri. Următoarele două sunt legate de logica noastră. Ultima linie returnează adevărat. Nu văd nicio modalitate de a face această metodă mai ușor de înțeles fără a introduce complexitatea extragerilor pe care le-am putea face, de exemplu prin extragerea celor două metode de afișare într-una particulară. Dacă o să facem acest lucru, unde ar trebui să meargă această metodă? În asta Joc
clasă sau în Afişa
? Cred că aceasta este deja o întrebare prea complexă care merită să fie luată în considerare în ceea ce privește simplitatea simplă a metodei noastre.
Să facem niște statistici prin verificarea acestui instrument excelent de la scriitorul PHPUnit https://github.com/sebastianbergmann/phploc.git
./ phploc ... / Refactoring \ Legacy \ Code \ - \ Part \ 1 \: \ The \ Golden \ Master / Sursa / trivia / php / phploc 2.1-gca70e70 de Sebastian Bergmann. (LCL) 232 Liniile de cod (CLOC) 0 (0.00%) Liniile codului necomentat (NCLOC) 232 (100.00%) Liniile logice ale codului (LLOC) 99 (42.67% %) Lungime medie a clasei 88 Lungime clasa minima 88 Lungime clasa maxima 88 Lungime medie metoda 7 Lungime metoda minima 1 Lungime maxima metoda 17 Functii 1 (1,01%) Lungime medie functie 1 Fara clasa sau functie 10 (10.10% per LLOC 0.26 Complexitatea medie pe clasa 25.00 Complexitatea minima a clasei 25.00 Complexitatea maxima a clasei 25.00 Complexitatea medie pe metoda 3.18 Complexitatea metodei minime 1.00 Complexitatea maxima a metodei 10.00 Dependente Accese globale 0 Constante globale 0 (0.00%) Variabile globale 0 (0.00%) Super-Global Variabilele 0 (0.00%) Accesul atributului 115 Static 0 (0.00%) Static 0 (0.00%) Apeluri metodă 21 Static 0 (0.00%) Structuri Nume 0 Interfețe 0 Trasaturi 0 Clase 1 Rezumat Clasele 0 (0.00%) Clasele de beton 1 ( 100,00%) Metode neprotetice 0 (0.00%) Metode statice 0 (0.00%) Metode statice 0 (0.00%) Vizibilitate Metode publice 11 (100.00%) Funcții 1 Funcții anonime 1 (100.00% 0 (0.00%) Constante 0 Constante globale 0 (0.00%) Clasa Constante 0 (0.00%)
./ phploc ... / Refactoring \ Legacy \ Code \ - \ Part \ 11 \: \ The \ End \? Source / trivia / php phploc 2.1-gca70e70 de Sebastian Bergmann. (LOC) 371 Linii de cod (CLOC) 0 (0.00%) Linii de cod neclare (NCLOC) 371 (100.00%) Liniile logice ale codului 151 (40.70%) %) Lungime clasa medie 36 Lungime clasa minima 8 Lungime maxima clasa 89 Metodă medie Lungime 2 Lungime minimă a metodei 1 Lungime maximă a metodei 14 Funcții 0 (0.00%) Lungime medie funcțională 0 Fără clasă sau funcție 6 (3,97%) Complexitate ciclo- per LLOC 0.15 Complexitatea medie pe clasa 6.50 Complexitatea minima a clasei 1.00 Complexitatea maxima a clasei 17.00 Complexitatea medie pe metoda 1.46 Complexitatea metodei minime 1.00 Complexitatea maxima a metodei 10.00 Dependente Accese globale 0 Constante globale 0 (0.00%) Variabile globale 0 (0.00%) Super-Global Variabilele 0 (0.00%) Accesul atributului 96 Static (94%) Static 2 (2.08%) Apeluri metodice 74 Static 0 (0.00%) Structuri Nume 0 Interfețe 1 Trasaturi 0 Clase 3 Rezumat Clase 0 (0.00%) Clase de beton 3 (100.00 %) Metode 59 Metodă non-statică 59 (100.00%) Metode statice 0 (0.00%) Vizibilitate Metode publice 35 (59.32%) Metode non-publice 24 (40.68%) Funcții 0 Funcții numite 0 (0.00%) Constante 3 Constante globale 0 (0.00%) Clasa Constante 3 (100.00%)
Datele brute sunt la fel de bune pe cât putem înțelege și le putem analiza.
Numărul de linii logice ale codului a crescut semnificativ între 99 și 151. Dar acest număr nu ar trebui să vă păcălească în a gândi că codul nostru a devenit mai complex. Aceasta este o tendință naturală de cod bine reflectat, din cauza creșterii numărului de metode și a apelurilor la acestea.
De îndată ce analizăm lungimea medie a clasei, putem vedea o scădere dramatică a liniilor de cod, de la 88 la 36.
Și este uimitor modul în care lungimea metodei a scăzut de la o medie de șapte linii la doar două linii de cod.
În timp ce numărul de linii reprezintă un bun indicator al volumului de cod pe unitate de măsură, câștigul real se regăsește în analizele complexității ciclomatice. De fiecare dată când luăm o decizie în codul nostru, creștem complexitatea ciclomică. Când lanțem dacă
afirmațiile unul în celălalt, complexitatea ciclomică a acestei metode crește exponențial. Extrasele noastre continue au condus la metode cu o singură decizie în ele, reducând astfel complexitatea lor medie pe metodă de la 3.18 la 1.00. Puteți citi acest lucru ca "metodele noastre refactate sunt de 3,18 ori mai simple decât codul original". La nivel de clasă, scăderea complexității este și mai uimitoare. A scăzut de la 25.00 la 6.50.
Bine. Asta e. Sfârșitul seriei. Vă rugăm să nu ezitați să vă exprimați opiniile și dacă credeți că am ratat orice subiect de refactorizare, cereți-le în comentariile de mai jos. Dacă sunt interesante, le voi transforma în părți suplimentare ale acestei serii.
Vă mulțumim pentru atenția acordată.