Share
Cod vechi. Cod urât. Cod complicat. Codul spaghetti. Nonsens jibber. Cu două cuvinte, Codul vechi. Aceasta este o serie care vă va ajuta să lucrați și să vă ocupați de aceasta.
Am întâlnit mai întâi codul sursă vechi în lecția noastră anterioară. Apoi am rulat codul pentru a forma o opinie despre funcționalitatea sa de bază și a creat o suită de test Golden Master pentru a avea o plasă de siguranță brută pentru modificările viitoare. Vom continua să lucrăm la acest cod și îl găsiți sub codul trivia / php_start pliant. Celălalt dosar trivia / php_start este cu codul terminat al acestei lecții.
Timpul pentru primele schimbări a venit și ce modalitate mai bună de a înțelege o bază de coduri dificilă decât să începeți să extrageți constante magice și șiruri de caractere în variabile? Aceste sarcini aparent simple ne vor da perspective mai mari și uneori neașteptate asupra funcționării interioare a codului moștenit. Vom avea nevoie să dăm seama intențiile autorului codului original și să găsim numele proprii pentru fragmentele de cod pe care nu le-am mai văzut până acum.
Comenzile magice sunt șiruri de caractere folosite direct în diferite expresii, fără a fi atribuite unei variabile. Acest tip de șir a avut un înțeles deosebit pentru autorul original al codului, dar în loc să le atribuie unei variabile bine numite, autorul a crezut că sensul sirului era suficient de evident.
Să începem să ne uităm la noi Game.php și încercați să identificați șiruri de caractere. Dacă utilizați un IDE (și ar trebui să aveți) sau un editor de text mai inteligent capabil să evidențieze codul sursă, văzând șirurile vor fi ușor. Iată o imagine a modului în care arată codul pe afișajul meu.
Totul cu portocaliu este un șir. Găsirea fiecărui șir în codul sursă este foarte ușor în acest fel. Asigurați-vă că evidențierea este acceptată și activată în editorul dvs., indiferent de aplicația dvs..
Prima parte portocalie din codul nostru este imediat la linia a treia. Cu toate acestea, șirul conține doar un caracter de linie nouă. Acest lucru ar trebui să fie suficient de evident în opinia mea, ca să putem trece mai departe.
Când vine vorba de a decide ce să extragă și ce să păstreze neschimbat, există puține degete, dar în cele din urmă este de părerea dvs. profesională care trebuie să prevaleze. Pe baza acesteia, va trebui să decideți ce să faceți cu fiecare bucată de cod pe care o analizați.
Accentsconagua
pentru ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Întrebare sportivă". $ i)); array_push ($ this-> rockQuestions, $ this-> createRockQuestion ($ i)); createRockQuestion ($ index) return "Rock Question". indicele $; Deci, să analizăm liniile 32-42, fragmentul pe care îl puteți vedea mai sus. Pentru întrebările despre pop, știință și sport, există doar o simplă concatenare. Cu toate acestea, acțiunea de a compune șirul pentru o întrebare rock este extrasă într-o metodă. După părerea dvs., sunt aceste concatenări și șiruri destul de clare, astfel încât să le putem păstra pe toate în interiorul buclei noastre? Sau credeți că extragerea tuturor șirurilor în metodele lor ar justifica existența acestor metode? Dacă da, cum ați numi aceste metode?
Indiferent de răspuns, va trebui să modificăm codul. Este timpul să-i punem pe Maestrul nostru de Aur să lucreze și să scrie testul nostru care rulează și compară codul nostru cu conținutul existent.
clasa GoldenMasterTest extinde PHPUnit_Framework_TestCase private $ gmPath; funcția setUp () $ this-> gmPath = __DIR__. '/Gm.txt'; funcția testGenerateOutput () $ times = 20000; $ this-> generateMany ($ ori, $ this-> gmPath); funcția testOutputMatchesGoldenMaster () $ times = 20000; $ actualPath = '/tmp/actual.txt'; $ this-> generateMany ($ ori, $ actualPath); $ file_content_gm = file_get_contents ($ acest-> gmPath); $ file_content_actual = file_get_contents ($ actualPath); $ this-> assertTrue ($ fișier_content_gm == $ file_content_actual); funcția privată generateMany ($ times, $ fileName) ... funcția privată generateOutput ($ seed) ...
Am creat un alt test pentru a compara rezultatele, sigur testGenerateOutput () generează doar o dată ieșirea și nu face altceva. De asemenea, am mutat fișierul de ieșire master de aur "Gm.txt" în directorul curent pentru că "/ Tmp" pot fi șterse când sistemul repornește. Pentru rezultatele noastre reale, îl putem folosi în continuare. Pe cele mai multe sisteme de tip UNIX "/ Tmp" este montat în memoria RAM, deci este mult mai rapid decât sistemul de fișiere. Dacă ați făcut bine, testele ar trebui să treacă fără probleme.
Este foarte important să vă amintiți să notați testul generatorului ca fiind "sărit" pentru modificările viitoare. Dacă vă simțiți mai confortabil cu comentarea sau chiar ștergerea cu totul, vă rugăm să faceți acest lucru. Este important ca Maestrul nostru de Aur să nu se schimbe atunci când ne schimbăm codul. Acesta a fost generat o dată și nu vrem să îl modificăm vreodată, astfel încât să fim siguri că codul nostru nou generat se compară întotdeauna cu originalul. Dacă vă simțiți mai confortabil, faceți o copie de rezervă a acesteia, vă rugăm să procedați astfel.
funcția testGenerateOutput () $ this-> markTestSkipped (); $ times = 20000; $ this-> generateMany ($ ori, $ this-> gmPath);
Voi nota testul ca fiind omis. Acest lucru va pune rezultatul testului la "galben", ceea ce înseamnă că toate testele trec, dar unele sunt ignorate sau marcate ca incomplete.
Cu ajutorul testelor, putem începe să schimbăm codul. În opinia mea profesională, toate corzile pot fi ținute în interiorul pentru buclă. Vom lua codul din createRockQuestion () metodă, mutați-o înăuntru pentru buclă și ștergeți cu totul metoda. Această refactorizare este chemată Metoda inline.
"Puneți corpul metodei în corpul apelanților și eliminați metoda." ~ Martin Fowler
Există un set specific de pași pentru a face acest tip de refactorizare, așa cum este definit de Marting Fowler în Refactoring: Îmbunătățirea designului codului existent:
Nu avem extinse subclasele Joc, astfel încât primul pas validează.
Există o singură utilizare a metodei noastre, în interiorul pentru buclă.
funcția __construct () $ this-> players = array (); $ this-> places = array (0); $ this-> purses = array (0); $ this-> înPenaltyBox = array (0); $ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array (); pentru ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Întrebare sportivă". $ i)); array_push ($ this-> rockQuestions, "Întrebare Rock" $ i); createRockQuestion ($ index) return "Rock Question". indicele $; Am pus codul de la createRockQuestion () în pentru buclă în constructor. Mai avem codul nostru vechi. Acum este timpul să ne conducem testul.
Testele noastre trec. Ne putem șterge createRockQuestion () metodă.
funcția __construct () // ... // pentru ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Întrebare sportivă". $ i)); array_push ($ this-> rockQuestions, "Întrebare Rock" $ i); funcția isPlayable () return ($ this-> howManyPlayers ()> = 2); În cele din urmă ar trebui să ne facem din nou testele. Dacă am pierdut un apel la o metodă, ei vor eșua.
Ar trebui să treacă din nou. Felicitări! Am terminat cu primul nostru refactoring.
String în metodele adăuga() și roll () sunt utilizate numai pentru a le produce folosind echoln () metodă. Întreabă întrebări() compară șirurile cu categoriile. Acest lucru pare, de asemenea, acceptabil. currentCategory () pe de altă parte, returnează șiruri pe baza unui număr. În această metodă, există o mulțime de șiruri de text duplicate. Schimbarea oricărei categorii, cu excepția Rock, ar necesita modificarea numelui său în trei locuri, numai în această metodă.
function currentCategory () if ($ this-> locuri [$ this-> currentPlayer] == 0) return "Pop"; dacă ($ this-> places [$ this-> currentPlayer] == 4) return "Pop"; dacă ($ this-> places [$ this-> currentPlayer] == 8) return "Pop"; dacă ($ this-> places [$ this-> currentPlayer] == 1) return "Science"; dacă ($ this-> places [$ this-> currentPlayer] == 5) return "Science"; dacă ($ this-> places [$ this-> currentPlayer] == 9) return "Science"; dacă ($ this-> places [$ this-> currentPlayer] == 2) return "Sports"; dacă ($ this-> places [$ this-> currentPlayer] == 6) return "Sports"; dacă ($ this-> places [$ this-> currentPlayer] == 10) return "Sports"; retur "Rock";
Cred că putem face mai bine. Putem folosi o metodă de refactorizare numită Introducerea variabilei locale și să elimine duplicarea. Urmați aceste recomandări:
funcția curentCategorie () $ popCategory = "Pop"; $ scienceCategory = "Știință"; $ sportCategory = "Sport"; $ rockCategory = "Rock"; dacă ($ this-> places [$ this-> currentPlayer] == 0) return $ popCategory; dacă ($ this-> places [$ this-> currentPlayer] == 4) return $ popCategory; dacă ($ this-> places [$ this-> currentPlayer] == 8) return $ popCategory; dacă ($ this-> places [$ this-> currentPlayer] == 1) return $ scienceCategory; dacă ($ this-> places [$ this-> currentPlayer] == 5) return $ scienceCategory; dacă ($ this-> places [$ this-> currentPlayer] == 9) return $ scienceCategory; dacă ($ this-> places [$ this-> currentPlayer] == 2) return $ sportCategory; dacă ($ this-> places [$ this-> currentPlayer] == 6) return $ sportCategory; dacă ($ this-> places [$ this-> currentPlayer] == 10) return $ sportCategory; return $ rockCategory;
Acest lucru este mult mai bun. Probabil ați observat deja unele îmbunătățiri viitoare pe care le-am putea face pentru codul nostru, dar suntem doar la începutul muncii noastre. Este tentant să repari tot ce găsești imediat, dar nu te rog. De multe ori, mai ales înainte ca codul să fie bine înțeles, modificările tentante pot duce la sfârțituri sau chiar mai multe coduri rupte. Dacă credeți că există o șansă să vă uitați ideea, notați-o într-o notă lipicioasă sau creați o sarcină în software-ul de management de proiect. Acum trebuie să continuăm cu problemele legate de șir.
În restul fișierului, toate șirurile sunt legate de ieșire, trimise în echoln (). Pentru moment, le vom lăsa neatinse. Modificarea acestora ar determina imprimarea și livrarea logicii aplicației noastre. Acestea fac parte din stratul de prezentare amestecat cu logica de afaceri. Ne vom ocupa de separarea diferitelor preocupări într-o lecție viitoare.
Constantele magice sunt foarte asemănătoare cu șirurile magice, dar cu valori. Aceste valori pot fi valori sau numere booleene. Ne vom concentra mai mult pe numerele utilizate în dacă declarații sau întoarcere declarații sau alte expresii. Dacă aceste numere au un înțeles neclar, trebuie să le extragem în variabile sau metode.
include_once __DIR__. '/Game.php'; $ NotAWinner; $ aGame = Joc nou (); $ AGame-> adaugă ( "Chet"); $ AGame-> adaugă ( "Pat"); $ AGame-> add ( "Sue"); face $ aGame-> roll (rand (0, 5) + 1); dacă (rand (0, 9) == 7) $ notAWinner = $ aGame-> greșitAnswer (); altceva $ notAWinner = $ aGame-> a fost corectdescărit (); în timp ce ($ notAWinner);
Vom începe cu asta GameRunner.php de data aceasta și primul nostru accent este rola () metodă care primește numere aleatorii. Autorul precedent nu a avut grijă să dea acestor numere un sens. Putem? Dacă analizăm codul:
rand (0, 5) + 1
Va întoarce un număr între unu și șase. Partea aleatoră returnează un număr între zero și cinci, la care adăugăm întotdeauna unul. Deci este cu siguranță între unu și șase. Acum trebuie să luăm în considerare contextul cererii noastre. Dezvoltăm un joc de trivia. Știm că există un fel de tablă pe care jucătorii noștri trebuie să se miște. Și pentru a face acest lucru, trebuie să rostogoli zarurile. Un mormânt are șase fețe și poate produce numere între unu și șase. Aceasta pare a fi o deducere rezonabilă.
$ dică = rand (0, 5) + 1; $ AGame-> rola ($ zaruri);
Nu-i așa frumos? Am folosit din nou conceptul de Introducere a variabilei locale de refactorizare. Am numit noua noastră variabilă $ zaruri și reprezintă numărul aleatoriu generat între unu și șase. Acest lucru a făcut, de asemenea, următoarea afirmație sună cu adevărat natural: Joc, zaruri rola.
Ți-ai făcut testele? Nu am menționat-o, dar trebuie să le executăm cât mai des posibil. Dacă nu ați fi, ar fi un moment bun pentru a le conduce. Și ar trebui să treacă.
Deci, acesta a fost un caz de nimic mai mult decât schimbul unui număr cu o variabilă. Am luat o întreagă expresie care a reprezentat un număr și a extras-o într-o variabilă. Acest lucru poate fi considerat din punct de vedere tehnic un caz Magic Constant, dar nu un caz pur. Cum rămâne cu următoarea noastră expresie aleatorie?
dacă (rand (0, 9) == 7)
Acest lucru este mai complicat. Ce sunt zero, nouă și șapte în expresia respectivă? Poate că le putem numi. La prima vedere, nu am idei bune pentru zero și nouă, deci să încercăm șapte. Dacă numărul returnat prin funcția aleatorie este egal cu șapte, vom intra în prima ramură a lui dacă declarație care produce un răspuns greșit. Poate că șapte ar putea fi numiți $ wrongAnswerId.
$ wrongAnswerId = 7; dacă (rand (0, 9) == $ wrongAnswerId) $ notAWinner = $ aGame-> greșitAnswer (); altceva $ notAWinner = $ aGame-> a fost corectdescărit ();
Testele noastre încă se transmit și codul este oarecum mai expresiv. Acum, că am reușit să numim numărul nostru șapte, pune condițional într-un context diferit. Ne putem gândi la niște nume decente pentru zero și nouă, de asemenea. Ele sunt doar parametrii rand (), astfel încât variabilele vor fi probabil numite min-ceva și max-ceva.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; dacă (rand ($ minAnswerId, $ maxAnswerId) == $ greșitAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); altceva $ notAWinner = $ aGame-> a fost corectdescărit ();
Acum este expresiv. Avem un ID de răspuns minim, unul maxim și altul pentru un răspuns greșit. Mister rezolvat.
face $ dice = rand (0, 5) + 1; $ AGame-> rola ($ zaruri); $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; dacă (rand ($ minAnswerId, $ maxAnswerId) == $ greșitAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); altceva $ notAWinner = $ aGame-> a fost corectdescărit (); în timp ce ($ notAWinner);
Dar observați că tot codul se află în interiorul a face în timp ce buclă. Trebuie să re-atribuim variabilele ID de răspuns de fiecare dată? Nu cred. Să încercăm să le scoatem din bucla și să vedem dacă testele noastre trec.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; face $ dice = rand (0, 5) + 1; $ AGame-> rola ($ zaruri); dacă (rand ($ minAnswerId, $ maxAnswerId) == $ greșitAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); altceva $ notAWinner = $ aGame-> a fost corectdescărit (); în timp ce ($ notAWinner);
Da. Testele trec și așa.
Este timpul să treceți la Game.php și căutați Constante Magic acolo, de asemenea. Dacă aveți evidențierea codului, cu siguranță aveți constante evidențiate într-o anumită culoare. Mina mea este albastră și sunt destul de ușor de observat.
Găsirea constantei magice 50 în asta pentru buclă a fost destul de ușor. Și dacă ne uităm la ceea ce face codul, putem descoperi că în interiorul lui pentru buclă, elementele sunt împinse la mai multe tablouri. Deci avem niște liste, fiecare cu 50 de elemente. Fiecare listă reprezintă o categorie de întrebări, iar variabilele sunt de fapt câmpurile de clasă definite mai sus ca matrice.
$ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array ();
Deci, ce poate reprezenta 50? Pun pariu că ai deja niște idei. Naming-ul este una dintre cele mai dificile sarcini în programare, dacă aveți mai multe idei și vă simțiți nesigur pe care să alegeți, nu vă fie rușine. Am, de asemenea, nume diferite în capul meu și eu evaluez posibilitățile de a alege cel mai bun, chiar și în timp ce scriu acest paragraf. Cred că putem merge cu un nume conservator pentru 50 de ani$ questionsInEachCategory sau $ categorySize sau ceva similar.
$ categorySize = 50; pentru ($ i = 0; $ i < $categorySize; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Întrebare sportivă". $ i)); array_push ($ this-> rockQuestions, "Întrebare Rock" $ i); Asta pare decent. Putem să o păstrăm. Și testele sunt bineînțeles că trec.
funcția estePlayable () return ($ this-> howManyPlayers ()> = 2);
Ce sunt doi? Sunt sigur că în acest moment răspunsul este clar pentru dumneavoastră. Este usor:
este funcțiaPlayable () $ minimumNumberOfPlayers = 2; retur ($ this-> howManyPlayers ()> = $ minimNumberOfPlayers);
Ești de acord? Dacă aveți o idee mai bună, nu ezitați să comentați mai jos. Și testele tale? Sunt încă în trecere?
Acum, în rola () există și câteva numere: două, zero, 11 și 12.
dacă ($ roll% 2! = 0)
Acest lucru este destul de clar. Vom extrage expresia respectivă într-o metodă, dar nu în acest tutorial. Suntem încă în faza de înțelegere și de vânătoare pentru constante magice și șiruri de caractere. Deci, ce zici de 11 și 12? Sunt îngropați în interiorul celui de-al treilea nivel dacă declarații. Este destul de greu de înțeles ce reprezintă ei. Poate dacă ne uităm la liniile din jurul lor.
dacă ($ this-> places [$ this-> currentPlayer]> 11) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12;
Dacă locul sau poziția actuală a jucătorului este mai mare de 11, atunci poziția sa va fi redusă la cea curentă minus 12. Aceasta pare a fi cazul când un jucător ajunge la capătul bordului sau câmpul de joc și este repoziționat în original poziţie. Probabil poziția zero. Sau, dacă tabla noastră de joc este circulară, trecerea peste ultima poziție marcată va pune jucătorul în prima poziție relativă. Deci, 11 ar putea fi dimensiunea bordului.
$ boardSize = 11; dacă $ ($ this-> currentPlayer)) if ($ roll% 2! = 0) // ... // dacă ($ this-> places [$ this-> currentPlayer]> $ boardSize) $ this-> plasează [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12; // // // // altceva // ... // altceva // ... // dacă ($ this-> places [$ this-> currentPlayer]> $ boardSize) $ this-> places [$ this -> currentPlayer] = $ this-> locuri [$ this-> currentPlayer] - 12; // ... //
Nu uitați să înlocuiți 11 în ambele locuri din interiorul metodei. Aceasta ne va forța să mutăm asignarea variabilei în afara dacă declarații, chiar la primul nivel de indentare.
Dar dacă 11 este dimensiunea placii, care este 12? Remarcăm 12 din poziția actuală a jucătorului, nu 11. Și de ce nu am stabilit poziția la zero în loc să scăpăm? Pentru că asta ar face ca testele noastre să nu reușească. Previziunile noastre anterioare că jucătorul va ajunge în poziția zero după codul din interiorul lui dacă declarația este executată, a fost greșită. Să presupunem că un jucător este în poziția zece și rolurile patru. 14 este mai mare de 11, deci se va face scăderea. Jucătorul va ajunge în poziție 10 + 2 = 4-12.
Acest lucru ne conduce către o altă denumire posibilă pentru 11 și 12. Cred că este mai potrivit să sunați la 12 $ boardSize. Dar ce ne lasă asta pentru 11? Poate $ lastPositionOnTheBoard? Un pic cam lung, dar cel puțin ne spune adevărul despre constanta magică.
$ lastPositionOnTheBoard = 11; $ boardSize = 12; dacă $ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // dacă ($ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> plasează [$ this-> currentPlayer] = $ this-> plasează [$ this-> currentPlayer] - $ boardSize; // // // altfel // ... // altfel // ... // dacă ($ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this -> currentPlayer] = $ this-> locuri [$ this-> currentPlayer] - $ boardSize; // ... //
Știu, știu! Există o dublare a codului acolo. Este destul de evident, mai ales cu restul codului ascuns. Dar vă rugăm să vă amintiți că am fost după constante magice. Va exista și un timp pentru codul duplicat, dar nu chiar acum.
Am lăsat o ultimă constanță magică în cod. Poți să-l vezi? Dacă te uiți la codul final, acesta va fi înlocuit, dar, desigur, ar fi înșelăciune. Mult noroc găsindu-l și mulțumesc pentru lectură.
