Refactoring Legacy Code Part 9 - Analizzare le preoccupazioni

In questo tutorial, continueremo a concentrarci sulla nostra logica di business. Valuteremo se RunnerFunctions.php appartiene a una classe e se sì, a quale classe? Penseremo alle preoccupazioni e ai metodi a cui appartengono. Infine, impareremo un po 'di più sul concetto di derisione. Allora, cosa stai aspettando? Continuare a leggere.


RunnerFunctions - Da procedurale a object oriented

Anche se abbiamo la maggior parte del nostro codice in forma orientata agli oggetti, ben organizzata in classi, alcune funzioni sono semplicemente sedute in un file. Abbiamo bisogno di prendere un po 'per dare le funzioni RunnerFunctions.php in un aspetto più orientato agli oggetti.

const WRONG_ANSWER_ID = 7; const MIN_ANSWER_ID = 0; const MAX_ANSWER_ID = 9; function isCurrentAnswerCorrect ($ minAnswerId = MIN_ANSWER_ID, $ maxAnswerId = MAX_ANSWER_ID) return rand ($ minAnswerId, $ maxAnswerId)! = WRONG_ANSWER_ID;  function run () $ display = new CLIDisplay (); $ aGame = nuovo gioco ($ display); $ AGame-> aggiungere ( "Chet"); $ AGame-> aggiungere ( "Pat"); $ AGame-> aggiungere ( "Sue"); do $ dice = rand (0, 5) + 1; $ AGame-> rotolo ($ dadi);  while (! didSomebodyWin ($ aGame, isCurrentAnswerCorrect ()));  function didomebodyWin ($ aGame, $ isCurrentAnswerCorrect) if ($ isCurrentAnswerCorrect) return! $ AGame-> wasCorrectlyAnswered ();  else return! $ AGame-> wrongAnswer (); 

Il mio primo istinto è quello di avvolgerli in una classe. Questo non è geniale, ma è qualcosa che ci fa iniziare a cambiare le cose. Vediamo se l'idea può davvero funzionare.

const WRONG_ANSWER_ID = 7; const MIN_ANSWER_ID = 0; const MAX_ANSWER_ID = 9; class RunnerFunctions function isCurrentAnswerCorrect ($ minAnswerId = MIN_ANSWER_ID, $ maxAnswerId = MAX_ANSWER_ID) return rand ($ minAnswerId, $ maxAnswerId)! = WRONG_ANSWER_ID;  function run () // ... // function didomebodyWin ($ aGame, $ isCurrentAnswerCorrect) // ... //

Se lo facciamo, dobbiamo modificare i nostri test e i nostri GameRunner.php usare la nuova classe. Abbiamo chiamato la classe qualcosa di generico per il momento, rinominandolo sarà facile quando necessario. Non sappiamo nemmeno se questa classe esisterà da sola o sarà assimilata a Gioco. Quindi non preoccuparti di nominare ancora.

funzione privata generateOutput ($ seed) ob_start (); srand ($ seme); (nuovo RunnerFunctions ()) -> run (); $ output = ob_get_contents (); ob_end_clean (); restituire $ output; 

Nel nostro GoldenMasterTest.php file, dobbiamo modificare il modo in cui eseguiamo il nostro codice. La funzione è generateOutput () e la sua terza riga deve essere modificata per creare un nuovo oggetto e chiamare correre() su di esso. Ma questo fallisce.

PHP Errore irreversibile: chiamata a funzione non definita didomebodyWin () in ... 

Ora dobbiamo modificare ulteriormente la nostra nuova classe.

do $ dice = rand (0, 5) + 1; $ AGame-> rotolo ($ dadi);  while (! $ this-> didSomebodyWin ($ aGame, $ this-> isCurrentAnswerCorrect ()));

Abbiamo solo bisogno di cambiare la condizione del mentre dichiarazione nel correre() metodo. Il nuovo codice chiama didSomebodyWin () e isCurrentAnswerCorrect () dalla classe attuale, anticipando $ This-> a loro.

Questo fa passare il maestro d'oro, ma frena i test runner.

PHP Errore irreversibile: chiamata alla funzione non definita isCurrentAnswerCorrect () in / ... /RunnerFunctionsTest.php sulla riga 25

Il problema è dentro assertAnswersAreCorrectFor (), ma facilmente risolvibile creando prima un oggetto runner.

funzione privata assertAnswersAreCorrectFor ($ correctAnserIDs) $ runner = new RunnerFunctions (); foreach ($ correctAnserIDs come $ id) $ this-> assertTrue ($ runner-> isCurrentAnswerCorrect ($ id, $ id)); 

Questo stesso problema deve essere risolto anche in altre tre funzioni.

function testItCanFindWrongAnswer () $ runner = new RunnerFunctions (); $ this-> assertFalse ($ runner-> isCurrentAnswerCorrect (WRONG_ANSWER_ID, WRONG_ANSWER_ID));  function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided () $ runner = new RunnerFunctions (); $ this-> assertTrue ($ runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aCorrectAnswer ()));  function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided () $ runner = new RunnerFunctions (); $ this-> assertFalse ($ runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aWrongAnswer ())); 

Mentre questo fa passare il codice, introduce un po 'di duplicazione del codice. Dato che siamo ora con tutti i test sul verde, possiamo estrarre la creazione del corridore in a impostare() metodo.

$ runner privato; function setUp () $ this-> runner = new Runner ();  function testItCanFindCorrectAnswer () $ this-> assertAnswersAreCorrectFor ($ this-> getCorrectAnswerIDs ());  function testItCanFindWrongAnswer () $ this-> assertFalse ($ this-> runner-> isCurrentAnswerCorrect (WRONG_ANSWER_ID, WRONG_ANSWER_ID));  function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided () $ this-> assertTrue ($ this-> runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aCorrectAnswer ()));  function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided () $ this-> assertFalse ($ this-> runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aWrongAnswer ()));  funzione privata assertAnswersAreCorrectFor ($ correctAnserIDs) foreach ($ correctAnserIDs come $ id) $ this-> assertTrue ($ this-> runner-> isCurrentAnswerCorrect ($ id, $ id)); 

Bello. Tutte queste nuove creazioni e rifacimenti mi hanno fatto riflettere. Abbiamo chiamato la nostra variabile corridore. Forse la nostra classe potrebbe essere chiamata la stessa. Facciamo il refactoring. Dovrebbe essere facile.

Se non hai controllato "Cerca le occorrenze di testo"nella casella in alto, non dimenticare di modificare manualmente gli include, poiché il refactoring rinominerà anche il file.

Ora abbiamo un file chiamato GameRunner.php, un altro chiamato Runner.php e un terzo chiamato Game.php. Non so voi, ma questo mi sembra estremamente confuso. Se dovessi vedere questi tre file per la prima volta nella mia vita, non avrei idea di chi fa cosa. Dobbiamo liberarci di almeno uno di loro.

Il motivo per cui abbiamo creato il RunnerFunctions.php file nelle prime fasi del nostro refactoring, era quello di creare un modo per includere tutti i metodi e i file per il test. Avevamo bisogno di accedere a tutto, ma non eseguire tutto se non in un ambiente preparato nel nostro maestro d'oro. Possiamo ancora fare la stessa cosa, basta che non eseguiamo il nostro codice GameRunner.php. Dobbiamo aggiornare gli include e creare una classe all'interno, prima di continuare.

require_once __DIR__. '/Display.php'; require_once __DIR__. '/Runner.php'; (nuovo Runner ()) -> run ();

Questo lo farà. Dobbiamo includere Display.php esplicitamente, quindi quando Corridore prova a creare un nuovo CLIDisplay, saprà cosa implementare.


Analizzando le preoccupazioni

Credo che una delle caratteristiche più importanti della programmazione orientata agli oggetti stia definendo le preoccupazioni. Mi pongo sempre domande del tipo: "questa classe fa quello che dice il suo nome?", "Questo metodo è preoccupante per questo oggetto?", "Il mio oggetto dovrebbe interessarsi a quel valore specifico?"

Sorprendentemente, questi tipi di domande hanno un grande potere nel chiarire sia il dominio del business che l'architettura del software. Stiamo chiedendo e rispondendo a questi tipi di domande in un gruppo di Syneto. Molte volte, quando un programmatore ha un dilemma, lui o lei si alza, chiede due minuti di attenzione dal team per trovare la nostra opinione su un argomento. Coloro che hanno familiarità con l'architettura del codice risponderanno dal punto di vista del software, mentre altri, più familiarizzati con il dominio aziendale, potranno far luce su alcune informazioni essenziali sugli aspetti commerciali.

Proviamo a pensare alle preoccupazioni nel nostro caso. Possiamo continuare a concentrarci sul Corridore classe. È molto più probabile eliminare o trasformare questa classe, piuttosto che Gioco.

In primo luogo, un corridore dovrebbe preoccuparsi di come isCurrentAnswerCorrect () lavoro? Un corridore dovrebbe avere qualche conoscenza su domande e risposte?

Sembra davvero che questo metodo sarebbe meglio in Gioco. Credo fermamente che a Gioco sulle curiosità dovrebbe importare se una risposta è corretta o meno. Credo davvero a Gioco deve preoccuparsi di fornire il risultato della risposta per la domanda corrente.

È ora di agire. Faremo a metodo di spostamento refactoring. Come abbiamo già visto prima dai miei tutorial precedenti, ti mostrerò solo il risultato finale.

require_once __DIR__. '/CLIDisplay.php'; include_once __DIR__. '/Game.php'; class Runner function run () // ... // function didomebodyWin ($ aGame, $ isCurrentAnswerCorrect) // ... //

È essenziale notare che non solo il metodo è andato via, ma anche la costante che definisce i limiti della risposta.

Ma per quanto riguarda didSomebodyWin ()? Un corridore dovrebbe decidere quando qualcuno ha vinto? Se osserviamo il corpo del metodo, possiamo vedere un problema che si evidenzia come una torcia nel buio.

function didSomebodyWin ($ aGame, $ isCurrentAnswerCorrect) if ($ isCurrentAnswerCorrect) return! $ aGame-> wasCorrectlyAnswered ();  else return! $ aGame-> wrongAnswer (); 

Qualunque cosa faccia questo metodo, lo fa su a Gioco solo oggetto. Verifica la risposta corrente restituita dal gioco. Quindi restituisce qualunque oggetto di gioco ritorni nel suo wasCorrectlyAnswered () o risposta sbagliata() metodi. Questo metodo non fa nulla da solo. Tutto ciò che importa è Gioco. Questo è un classico esempio di odore di codice chiamato Caratteristica invidia. Una classe fa qualcosa che un'altra classe dovrebbe fare. È ora di spostarlo.

class RunnerFunctionsTest estende PHPUnit_Framework_TestCase private $ runner; function setUp () $ this-> runner = new Runner (); 

Come al solito, abbiamo prima spostato i test. TDD? Chiunque?

Questo ci lascia senza più test da eseguire, quindi questo file può andare ora. Cancellare è la mia parte preferita della programmazione.

E quando eseguiamo i nostri test, otteniamo un bel errore.

Errore irreversibile: chiamata al metodo non definito Game :: didSomebodyWin ()

Adesso è il momento di cambiare anche il codice. Copia e incolla il metodo in Gioco magicamente farà passare tutti i test. Entrambi i vecchi e quelli trasferiti a GameTest. Ma mentre questo mette il metodo nel posto giusto, ha due problemi: anche il corridore deve essere cambiato e noi mandiamo un falso Gioco oggetto che non abbiamo più bisogno di fare da quando fa parte di Gioco.

do $ dice = rand (0, 5) + 1; $ AGame-> rotolo ($ dadi);  while (! $ aGame-> didomebodyWin ($ aGame, $ this-> isCurrentAnswerCorrect ()));

Riparare il corridore è molto semplice. Cambiamo solo $ this-> didSomebodyWin (...) in $ aGame-> didSomebodyWin (...). Dovremo tornare qui e cambiarlo di nuovo, dopo il nostro prossimo passo. Il refactoring del test.

function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided () $ aGame = \ Mockery :: mock ('Game [wasCorrectlyAnswered]'); $ AGame-> shouldReceive ( 'wasCorrectlyAnswered') -> una volta () -> andReturn (false); $ This-> assertTrue ($ aGame-> didSomebodyWin ($ this-> aCorrectAnswer ())); 

È tempo di deriderlo! Invece di usare la nostra classe finta, definita alla fine dei nostri test, useremo Mockery. Ci consente di sovrascrivere facilmente un metodo Gioco, aspettati che venga chiamato e restituisca il valore che vogliamo. Certo, potremmo farlo rendendo la nostra classe finta estesa Gioco e sovrascriviamo il metodo da soli. Ma perché fare un lavoro per il quale esiste uno strumento?

function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided () $ aGame = \ Mockery :: mock ('Game [wrongAnswer]'); $ AGame-> shouldReceive ( 'wrongAnswer') -> una volta () -> andReturn (true); $ This-> assertFalse ($ aGame-> didSomebodyWin ($ this-> aWrongAnswer ())); 

Dopo che il nostro secondo metodo è stato riscritto, possiamo sbarazzarci della falsa classe di gioco e di tutti i metodi che l'hanno inizializzata. Problemi risolti!

Pensieri finali

Anche se siamo riusciti a pensare solo al Corridore, abbiamo fatto grandi progressi oggi. Abbiamo imparato a conoscere le responsabilità, abbiamo identificato metodi e variabili che appartengono a un'altra classe. Abbiamo pensato a un livello più alto e ci siamo evoluti verso una soluzione migliore. Nel team di Syneto, c'è una forte convinzione che ci siano modi per scrivere bene il codice e non commettere mai una modifica a meno che non abbia reso il codice almeno un po 'più pulito. Questa è una tecnica che nel tempo può portare a un codebase molto più carino, con meno dipendenze, più test e alla fine meno bug.

Grazie per il tuo tempo.