Share
Vecchio codice Codice brutto. Codice complicato Codice degli spaghetti Assurdità senza senso. In due parole, Codice legacy. Questa è una serie che ti aiuterà a lavorare e ad affrontarla.
Abbiamo incontrato per la prima volta il nostro codice sorgente precedente nella lezione precedente. Quindi abbiamo eseguito il codice per esprimere un'opinione sulle sue funzionalità di base e creato una suite di test Golden Master per disporre di una rete di sicurezza per le future modifiche. Continueremo a lavorare su questo codice e potrai trovarlo sotto il trivia / php_start cartella. L'altra cartella trivia / php_start è con il codice finito di questa lezione.
È giunto il momento per i primi cambiamenti e quale modo migliore per capire un codice base difficile che iniziare a estrarre costanti e stringhe magiche in variabili? Questi compiti apparentemente semplici ci forniranno maggiori e talvolta inaspettati approfondimenti sui meccanismi interni del codice legacy. Dovremo capire le intenzioni dell'autore del codice originale e trovare i nomi appropriati per i pezzi di codice che non abbiamo mai visto prima.
Le stringhe magiche sono stringhe utilizzate direttamente in varie espressioni, senza essere assegnate a una variabile. Questo tipo di stringa aveva un significato speciale per l'autore originale del codice, ma invece di assegnarli a una variabile ben denominata, l'autore pensava che il significato della stringa fosse abbastanza ovvio.
Iniziamo guardando il nostro Game.php e prova a identificare le stringhe. Se stai usando un IDE (e dovresti) o un editor di testo più intelligente in grado di evidenziare il codice sorgente, individuare le stringhe sarà facile. Ecco un'immagine di come appare il codice sul mio display.
Tutto con l'arancione è una stringa. Trovare ogni stringa nel nostro codice sorgente è molto semplice in questo modo. Quindi assicurati che l'evidenziazione sia supportata e abilitata nell'editor, qualunque sia l'applicazione.
La prima parte arancione nel nostro codice è immediatamente alla terza riga. Tuttavia la stringa contiene solo un carattere di nuova riga. Questo dovrebbe essere abbastanza ovvio secondo me, quindi possiamo andare avanti.
Quando si tratta di decidere cosa estrarre e cosa mantenere invariato, ci sono alcuni thumb-up, ma alla fine è la tua opinione professionale che deve prevalere. Sulla base di questo, dovrai decidere cosa fare con ogni pezzo di codice che analizzi.
Accentsconagua
per ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Domande sportive". $ i)); array_push ($ this-> rockQuestions, $ this-> createRockQuestion ($ i)); function createRockQuestion ($ index) return "Rock Question". $ Indice; Analizziamo quindi le righe dalla 32 alla 42, lo snippet che puoi vedere sopra. Per domande su pop, scienza e sport, c'è solo una semplice concatenazione. Tuttavia, l'azione per comporre la stringa per una domanda rock viene estratta in un metodo. Secondo te, queste concatenazioni e stringhe sono abbastanza chiare da consentirci di mantenerle tutte all'interno del nostro ciclo for? Oppure, pensi che l'estrazione di tutte le stringhe nei loro metodi giustificherebbe l'esistenza di quei metodi? In tal caso, come chiameresti questi metodi?
Indipendentemente dalla risposta, dovremo modificare il codice. È tempo di mettere il nostro Golden Master al lavoro e scrivere il nostro test che effettivamente esegue e confronta il nostro codice con il contenuto esistente.
class GoldenMasterTest estende PHPUnit_Framework_TestCase private $ gmPath; function setUp () $ this-> gmPath = __DIR__. '/Gm.txt'; function testGenerateOutput () $ times = 20000; $ this-> generateMany ($ times, $ this-> gmPath); function testOutputMatchesGoldenMaster () $ 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); funzione privata generateMany ($ times, $ fileName) ... funzione privata generateOutput ($ seed) ...
Abbiamo creato un altro test per confrontare le uscite, reso sicuro testGenerateOutput () genera solo l'output una volta e non fa nient'altro. Abbiamo anche spostato il file di output principale dorato "Gm.txt" nella directory corrente perché "/ Tmp" può essere cancellato quando il sistema si riavvia. Per i nostri risultati attuali, possiamo ancora usarlo. Sulla maggior parte dei sistemi simili a UNIX "/ Tmp" è montato nella RAM, quindi è molto più veloce del file system. Se hai fatto bene, i test dovrebbero passare senza problemi.
È molto importante ricordare di contrassegnare il nostro test del generatore come "saltato" per modifiche future. Se ti senti più a tuo agio nel commentare o addirittura cancellarlo del tutto, per favore fallo. È importante che il nostro Golden Master non cambierà quando cambiamo il nostro codice. È stato generato una volta e non vogliamo modificarlo, mai, in modo da poter essere sicuri che il nostro codice appena generato sia sempre paragonato all'originale. Se ti senti più a tuo agio a fare un backup, per favore procedi a farlo.
function testGenerateOutput () $ this-> markTestSkipped (); $ times = 20000; $ this-> generateMany ($ times, $ this-> gmPath);
Segnerò semplicemente il test come saltato. Questo metterà il nostro risultato del test a "giallo", significa che tutti i test stanno passando ma alcuni vengono saltati o contrassegnati come incompleti.
Con i test in atto, possiamo iniziare a cambiare codice. Secondo la mia opinione professionale, tutte le stringhe possono essere mantenute all'interno del per ciclo continuo. Prenderemo il codice dal createRockQuestion () metodo, spostalo all'interno del per loop, ed eliminare del tutto il metodo. Questo refactoring è chiamato Metodo in linea.
"Metti il corpo del metodo nel corpo dei chiamanti e rimuovi il metodo." ~ Martin Fowler
Esiste una serie specifica di passaggi per eseguire questo tipo di refactoring, come definito da Marting Fowler in Refactoring: Migliorare il design del codice esistente:
Non abbiamo sottoclassi che si estendono Gioco, quindi il primo passo è valido.
C'è solo un uso del nostro metodo, all'interno del per ciclo continuo.
function __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 (); per ($ i = 0; $ i < 50; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Domande sportive". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); function createRockQuestion ($ index) return "Rock Question". $ Indice; Abbiamo messo il codice da createRockQuestion () nel per loop nel costruttore. Abbiamo ancora il nostro vecchio codice. È giunto il momento di eseguire il nostro test.
I nostri test stanno passando. Possiamo cancellare il nostro createRockQuestion () metodo.
function __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, ("Domande sportive". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); function isPlayable () return ($ this-> howManyPlayers ()> = 2); Alla fine dovremmo eseguire di nuovo i nostri test. Se abbiamo perso una chiamata a un metodo, falliranno.
Dovrebbero passare di nuovo. Congratulazioni! Abbiamo finito con il nostro primo refactoring.
Archi nei metodi Inserisci() e roll () sono usati solo per emetterli usando il echoln () metodo. fare domande() confronta stringhe in categorie. Anche questo sembra accettabile. currentCategory () d'altra parte restituisce stringhe basate su un numero. In questo metodo, ci sono molte stringhe duplicate. Cambiare qualsiasi categoria, ad eccezione di Rock, richiederebbe la modifica del suo nome in tre punti, solo in questo metodo.
function currentCategory () if ($ this-> places [$ this-> currentPlayer] == 0) return "Pop"; if ($ this-> places [$ this-> currentPlayer] == 4) return "Pop"; if ($ this-> places [$ this-> currentPlayer] == 8) return "Pop"; if ($ this-> places [$ this-> currentPlayer] == 1) return "Science"; if ($ this-> places [$ this-> currentPlayer] == 5) return "Science"; if ($ this-> places [$ this-> currentPlayer] == 9) return "Science"; if ($ this-> places [$ this-> currentPlayer] == 2) return "Sports"; if ($ this-> places [$ this-> currentPlayer] == 6) return "Sports"; if ($ this-> places [$ this-> currentPlayer] == 10) return "Sports"; return "Rock";
Penso che possiamo fare di meglio. Possiamo usare un metodo di refactoring chiamato Introduci la variabile locale ed eliminare la duplicazione. Segui queste linee guida:
function currentCategory () $ popCategory = "Pop"; $ scienceCategory = "Science"; $ sportCategory = "Sport"; $ rockCategory = "Rock"; if ($ this-> places [$ this-> currentPlayer] == 0) return $ popCategory; if ($ this-> places [$ this-> currentPlayer] == 4) return $ popCategory; if ($ this-> places [$ this-> currentPlayer] == 8) return $ popCategory; if ($ this-> places [$ this-> currentPlayer] == 1) return $ scienceCategory; if ($ this-> places [$ this-> currentPlayer] == 5) return $ scienceCategory; if ($ this-> places [$ this-> currentPlayer] == 9) return $ scienceCategory; if ($ this-> places [$ this-> currentPlayer] == 2) return $ sportCategory; if ($ this-> places [$ this-> currentPlayer] == 6) return $ sportCategory; if ($ this-> places [$ this-> currentPlayer] == 10) return $ sportCategory; return $ rockCategory;
Questo è molto meglio. Probabilmente hai già osservato alcuni miglioramenti futuri che potremmo fare per il nostro codice, ma siamo solo all'inizio del nostro lavoro. Si è tentati di sistemare tutto ciò che trovi immediatamente, ma per favore non farlo. Molte volte, soprattutto prima che il codice sia ben compreso, le modifiche allettanti possono portare a vicoli ciechi o ancora più codice non funzionante. Se pensi che ci sia una possibilità, dimenticherai la tua idea, annotala su una nota adesiva o crea un'attività nel tuo software di gestione dei progetti. Ora dobbiamo continuare con i nostri problemi relativi alle stringhe.
Nel resto del file, tutte le stringhe sono correlate all'output, inviate in echoln (). Per il momento, li lasceremo intatti. La loro modifica influirebbe sulla stampa e sulla logica di consegna della nostra applicazione. Fanno parte dello strato di presentazione combinato con la logica aziendale. Ci occuperemo di separare le diverse preoccupazioni in una lezione futura.
Le costanti magiche sono molto simili a stringhe magiche, ma con valori. Questi valori possono essere valori o numeri booleani. Ci concentreremo principalmente sui numeri utilizzati in Se dichiarazioni o ritorno dichiarazioni o altre espressioni. Se questi numeri hanno un significato non chiaro, dobbiamo estrarli in variabili o metodi.
include_once __DIR__. '/Game.php'; $ NotAWinner; $ aGame = new Game (); $ AGame-> aggiungere ( "Chet"); $ AGame-> aggiungere ( "Pat"); $ AGame-> aggiungere ( "Sue"); fai $ aGame-> roll (rand (0, 5) + 1); if (rand (0, 9) == 7) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); while ($ notAWinner);
Inizieremo con GameRunner.php questa volta e il nostro primo obiettivo è il rotolo () metodo che ottiene alcuni numeri casuali. L'autore precedente non si è preoccupato di dare un significato a quei numeri. Possiamo? Se analizziamo il codice:
rand (0, 5) + 1
Restituirà un numero compreso tra uno e sei. La parte casuale restituisce un numero compreso tra zero e cinque a cui aggiungiamo sempre uno. Quindi è sicuramente tra uno e sei. Ora dobbiamo considerare il contesto della nostra applicazione. Stiamo sviluppando un gioco a quiz. Sappiamo che esiste una sorta di consiglio su cui i nostri giocatori devono muoversi. E per farlo, dobbiamo tirare i dadi. Un dado ha sei facce e può produrre numeri compresi tra uno e sei. Sembra una deduzione ragionevole.
$ dice = rand (0, 5) + 1; $ AGame-> rotolo ($ dadi);
Non è carino? Abbiamo usato di nuovo il concetto di refactoring Introduce local Variable. Abbiamo chiamato la nostra nuova variabile $ dadi e rappresenta il numero casuale generato tra uno e sei. Ciò ha anche reso la nostra prossima affermazione davvero naturale: Game, roll dice.
Hai eseguito i tuoi test? Non l'ho menzionato, ma dobbiamo eseguirli il più frequentemente possibile. Se non lo hai, sarebbe un buon momento per eseguirli. E dovrebbero passare.
Quindi, questo è stato un caso di nulla più che il semplice scambio di un numero con una variabile. Abbiamo preso un'intera espressione che rappresentava un numero e l'ha estratto in una variabile. Questo può essere tecnicamente considerato un caso di Costante Magica, ma non un caso puro. Che dire della nostra prossima espressione casuale?
if (rand (0, 9) == 7)
Questo è più difficile. Cosa sono zero, nove e sette in quell'espressione? Forse possiamo chiamarli. A prima vista, non ho buone idee per zero e nove, quindi proviamo sette. Se il numero restituito dalla nostra funzione casuale è uguale a sette, entreremo nel primo ramo di Se affermazione che produce una risposta sbagliata. Quindi forse i nostri sette potrebbero essere nominati $ wrongAnswerId.
$ wrongAnswerId = 7; if (rand (0, 9) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();
I nostri test stanno ancora passando e il codice è un po 'più espressivo. Ora che siamo riusciti a nominare il nostro numero sette, pone il condizionale in un contesto diverso. Possiamo pensare ad alcuni nomi decenti anche per zero e nove. Sono solo parametri per rand (), quindi le variabili saranno probabilmente nominate min-qualcosa e max-qualcosa.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; if (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();
Ora questo è espressivo. Abbiamo un ID di risposta minimo, uno massimo e l'altro per la risposta sbagliata. Mistero risolto.
do $ dice = rand (0, 5) + 1; $ AGame-> rotolo ($ dadi); $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; if (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); while ($ notAWinner);
Ma nota che tutto il codice è all'interno di a fare mentre ciclo continuo. Abbiamo bisogno di riassegnare le variabili ID risposta ogni volta? Penso di no. Proviamo a spostarli fuori dal giro e vedere se i nostri test stanno passando.
$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; do $ dice = rand (0, 5) + 1; $ AGame-> rotolo ($ dadi); if (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer (); else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); while ($ notAWinner);
Sì. Anche i test passano così.
È ora di passare a Game.php e cerca anche Magic Constants. Se hai l'evidenziazione del codice, hai sicuramente delle costanti evidenziate in alcuni colori brillanti. I miei sono blu e sono abbastanza facili da individuare.
Trovare la costante magica 50 in questo per il ciclo era abbastanza facile. E se guardiamo a ciò che fa il codice, possiamo scoprire che all'interno di per loop, gli elementi vengono spostati su diversi array. Quindi abbiamo una sorta di liste, ognuna con 50 elementi. Ogni elenco rappresenta una categoria di domande e le variabili sono in realtà campi di classi definiti sopra come matrici.
$ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array ();
Quindi, cosa può rappresentare 50? Scommetto che hai già qualche idea. Il naming è uno dei compiti più difficili in programmazione, se hai più di una idea e non sei sicuro di quale scegliere, non vergognarti. Ho anche vari nomi nella mia testa e sto valutando le possibilità di scegliere il migliore, anche mentre scrivo questo paragrafo. Penso che possiamo andare con un nome prudente per 50. Qualcosa sulla falsariga di$ questionsInEachCategory o $ categorySize o qualcosa di simile.
$ categorySize = 50; per ($ i = 0; $ i < $categorySize; $i++) array_push($this->popQuestions, "Pop Question". $ I); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Domande sportive". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); Sembra decente. Possiamo tenerlo. E i test stanno ovviamente passando.
function isPlayable () return ($ this-> howManyPlayers ()> = 2);
Cosa sono due? Sono sicuro che a questo punto la risposta è chiara per te. Questo è facile:
function isPlayable () $ minimumNumberOfPlayers = 2; return ($ this-> howManyPlayers ()> = $ minimumNumberOfPlayers);
Sei d'accordo? Se hai un'idea migliore, sentiti libero di commentare qui sotto. E i tuoi test? Stanno ancora passando?
Ora, nel rotolo () metodo abbiamo anche alcuni numeri: due, zero, 11 e 12.
if ($ roll% 2! = 0)
Questo è abbastanza chiaro. Estraiamo quell'espressione in un metodo, ma non in questo tutorial. Siamo ancora nella fase della comprensione e della ricerca di costanti e archi magici. Allora, che ne è dell'11 e del 12? Sono sepolti all'interno del terzo livello di Se dichiarazioni. È abbastanza difficile capire cosa rappresentano. Forse se guardiamo le linee intorno a loro.
if ($ this-> places [$ this-> currentPlayer]> 11) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12;
Se il posto o la posizione del giocatore corrente è maggiore di 11, la sua posizione sarà ridotta a quella attuale meno 12. Questo sembra un caso in cui un giocatore raggiunge la fine della scacchiera o del campo di gioco e viene riposizionato nella sua iniziale posizione. Probabilmente posizione zero. Oppure, se il nostro tabellone di gioco è circolare, superare l'ultima posizione segnata metterà il giocatore nella relativa prima posizione. Quindi 11 potrebbe essere la dimensione della scheda.
$ boardSize = 11; if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // if ($ this-> places [$ this-> currentPlayer]> $ boardSize) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12; // ... // else // ... // else // ... // if ($ this-> places [$ this-> currentPlayer]> $ boardSize) $ this-> places [$ this -> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12; // ... //
Non dimenticare di sostituire 11 in entrambi i punti all'interno del metodo. Questo ci costringerà a spostare il compito variabile al di fuori del Se dichiarazioni, proprio al primo livello di indentazione.
Ma se 11 è la dimensione della tavola, che cosa è 12? Sottriamo 12 dalla posizione attuale del giocatore, non 11. E perché non impostiamo la posizione a zero invece di sottrarre? Perché ciò farebbe fallire i nostri test. La nostra precedente supposizione che il giocatore finirà in posizione zero dopo il codice all'interno del Se la dichiarazione è stata eseguita, era sbagliata. Diciamo che un giocatore è in posizione dieci e tira un quattro. 14 è maggiore di 11, quindi la sottrazione avverrà. Il giocatore finirà in posizione 10 + 4-12 = 2.
Questo ci spinge verso un'altra possibile denominazione per 11 e 12. Penso che sia più appropriato chiamare 12 $ boardSize. Ma cosa ci lascia per 11? Può essere $ lastPositionOnTheBoard? Un po 'lungo, ma almeno ci dice la verità sulla costante magica.
$ lastPositionOnTheBoard = 11; $ boardSize = 12; if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // if ($ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - $ boardSize; // ... // else // ... // else // ... // if ($ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this -> currentPlayer] = $ this-> places [$ this-> currentPlayer] - $ boardSize; // ... //
Lo so, lo so! C'è della duplicazione di codice lì. È abbastanza ovvio, specialmente con il resto del codice nascosto. Ma per favore ricorda che stavamo cercando costanti magiche. Ci sarà anche un tempo per il codice duplicato, ma non adesso.
Ho lasciato un'ultima costante magica nel codice. Puoi individuarlo? Se guardi il codice finale, verrà sostituito, ma ovviamente sarebbe un imbroglio. Buona fortuna a trovarlo e grazie per la lettura.
