Ieri per risolvere un bug di Diamonds e' stato modificato il codice del Timer del gioco. Questa modifica mi ha portato a scrivere un post sul forum di sviluppo che riporto qui. I concetti dietro a questo post mi sembrano abbastanza generali e interessanti.
Ho guardato il nuovo codice di Timer a casa, ma non ho avuto il tempo di postarlo.
A parole quello che fa e' memorizzare il tempo trascorso dalla creazione del timer al momento della chiamata ad advance() e poi restituisce quel tempo in getTime().
Questo cambiamento risolve il problema evidenziato da cionci, quindi va bene, e' un comportamento del quale abbiamo bisogno. Questo cambiamento pero' rompe il contratto di Timer, quello che ha scritto cionci non e' piu' un Timer, e' qualcos'altro: abbiamo un problema fondamentale di design che va risolto subito.
Il problema nasce dal fatto che Timer non fa' piu' quello che c'e' scritto sulla scatola, non rispetta piu' il suo contratto naturale, che puo' essere scritto in questa forma:
long timeStamp1 = timer.getTime();
Sleep(timerResolution);
long timeStamp2 = timer.getTime();
assertNotEqual(timeStamp1, timeStamp2);
In parole: due chiamate al timer successive separate da un tempo pari alla risoluzione del timer tornano due time stamp differenti.
Un nuovo programmatore su Diamonds, all'oscuro di tutto, guarderebbe la classe Timer, vedrebbe un metodo getTime() (dovrebbe essere chiamato getCurrentTime() per essere ancora piu' precisi), non vedrebbe alcun test a descriverne il contratto e la userebbe in maniera naturale seguendo il contratto che ho scritto sopra. Intuitivamente e' la prima cosa alla quale si pensa vedendo un timer, perche' modella lo scorrere del tempo: due istanti di tempo diversi hanno valori diversi. Scoprirebbe pero' amaramente, magari dopo molto tempo di debugging, che il nostro Timer non rispetta quel contratto!
Rispetta invece questo:
long timeStamp1 = timer.getTime();
Sleep(timerResolution);
long timeStamp2 = timer.getTime();
assertEqual(timeStamp1, timeStamp2);
timer.advance(timerResolution);
long timeStamp3 = timer.getTime();
assertNotEqual(timeStamp1, timeStamp3);
In parole: per far avanzare il Timer si deve interporre una chiamata ad advance(). Se tutto il codice fosse scritto cosi', dove una classe non rispetta il suo contratto naturale che il suo nome e il nome dei suoi metodi suggeriscono intuitivamente, il progetto diventerebbe presto ingestibile. E ci sono molti progetti cosi'. Non Diamonds.
La soluzione al problema di design e' togliere Timer dall'imbarazzo, restituirle il suo contratto naturale e introdurre un nuovo concetto, una nuova classe, che posso chiamare al momento GameTurn magari, ma che deve uscire dall'uso che se ne fa nel codice, che modella esattamente questo nuovo comportamento e che ha il nuovo contratto, ed e' implementata in termini del Timer che rispetta il suo contratto.
Una possibile soluzione veloce che fa scrivere poco codice e' ereditare GameTurn dal Timer, magari chiamarlo GameTimer e passarlo laddove il gioco ha bisogno di un Timer. Questa soluzione continuerebbe a non essere ideale perche' il nuovo GameTimer non "e' un" Timer, non rispetta il contratto di Timer, non rispetta il principio di sostituzione, non puo' essere usato ovunque puo' essere usato Timer.
Favor Composition Over Inheritance: abbiamo bisogno di un nuovo concetto, che non abbia legami con il Timer, che si limiti ad usarlo, che rispetti il nuovo contratto e fornisca il comportamento voluto. E dobbiamo usarlo nel gioco al posto del Timer. In Eclipse sono pochi colpi di refactoring e i nuovi programmatori di Diamonds ci ringrazieranno. Va fatto subito.