Problemi vari

[paolo-chiappini]

Nell'utilizzare le classi Board, PersonalGoalCardX, Player sono sorti diversi problemi di natura concettuale e di struttura del codice. Re-itero che il codice deve essere URGENTEMENTE testato in quanto è impossibile proseguire senza le basi del modello!

Di seguito sono riportati gli errori che ho trovato, potrebbero seguire altre Issue nel caso ne trovassi altri...

• Inconsistenza con gli indici: nel momento in cui si dichiarano array bidimensionali o si usano metodi su di essi è buona cosa essere consistenti con l'ordine degli indici. Dall'algebra, siccome stiamo parlando di matrici, lo standard conviene che l'ordine sia [righe][colonne] (il quale è lo stesso anche per i metodi che accettano come parametri (row, col)). Ho notato che in alcuni metodi come evaluatePoints di PersonalGoalCard essi sono invertiti causando problemi come Index out of bound (errori che sono facilmente individuabili eseguendo il codice almeno una volta).

• Uso scorretto degli attributi protected, static, di abstract e dell'ereditarietà:

  • nel caso delle PersonalGoalCard mi sono sfuggite un po' di cose dall'ultima pull request, ma che sono immediatamente evidenti durante l'esecuzione: pattern viene ridefinito 3 volte differenti in 3 classi differenti quando dovrebbe bastare 1 sola definizione. pattern è privato quando invece dovrebbe essere protected, specialmente se definito dalle classi che estendono. PersonalGoalCard dovrebbe essere abstract dato che se si usano delle sottoclassi, solo queste dovrebbero essere istanziabili.
  • Nella classe Player, score è stato cambiato a static, questo è un errore FATALE perchè mentre testavo altri pezzi di codice, per qualche motivo non veniva mai modificato. Dopo aver perso parecchio tempo e diversi tentativi a capire se ci fossero errori nella logica del calcolo dei punteggi ho notato che questo era il problema dietro tutto. Ricordo che static indica che l'attributo appartiene al NOME DELLA CLASSE e NON alle ISTANZE, quindi in questo modo la modifica di score su uno dei player andava a modificarla su TUTTI!!! (Tanto è vero che un attibuto static non richiede istanze, es: System.out)

• Errori/typo nella scrittura del codice: IntelliJ è estremamente potente e riesce ad individuare errori anche molto complessi, ma...

  • for(int j = 0 ; i < bookshelf.getHeight(); j++) (condizione su "i" in un ciclo con "j") bisogna fare attenzione a cose simili! Sono errori "banali" che però hanno conseguenze disastrose sul resto del codice per quanto possono essere difficili da trovare. Se la scelta è giustificata, invece, andrebbe documentata da un commento. (in ogni caso TEST, TEST, TEST! Sono errori che saltano fuori dall'esecuzione!).
  • Attenzione alla logica! if(bookshelf.getTileAt(i,j).getType().equals(pattern[i][j]) && (bookshelf.getTileAt(i,j) != null) è al contrario perchè, per il modo in cui sono valutati gli operatori logici, se getTileAt ritornasse null, il programma crasherebbe con una NullPointerException. La verifica dovrebbe essere fatta a priori: if(bookshelf.getTileAt(i,j) != null && bookshelf.getTileAt(i,j).getType().equals(pattern[i][j])). Anche in linguaggio umano la prima non ha senso: "Verifico che la tile sia dello stesso tipo, se lo è ALLORA verifico che non sia anche null", invece di "Verifico che la tile non sia null, se non lo è ALLORA verifico che sia anche dello stesso tipo".

• Mancanza di attributi specificati da progetto: per quanto riguarda la classe Player (non ricordo di altre occorrenze al momento), manca l'attributo bookshelf. (Per queste cose fare riferimento all'UML ed eventualmente al file su Discord che dettaglia le interfacce).

• Attenzione ai test aleatori: MAI usare random nei test. I Test DEVONO essere DETERMINISTICI! (in questo è possibile che ne sia colpevole anche io, pertanto possibile che vengano cambiati in futuro).

Col rischio di finire in un manicomio, queste saranno le mie ultime parole: TEST, TEST, TEST, TEST, TEST, TEST, TEST, TEST, TEST... Se non ci sono i test siamo tutti bloccati! Piuttosto, se serve una mano, sono il primo ad offrirmi per aiutare... non ci conosceremo da tanto tempo, ma siamo pur sempre un team!

[Andrea01Fraschini]

Approfitto di questa issue per dare consigli vari su come fare/utilizzare i test:

1. Scrivete PRIMA i test e poi le implementazioni:

Pensate a come devono essere utilizzate le classi e alle specifiche dei metodi (se avete dubbi chiedete please) e scrivete test che definiscano il comportamento che ci si aspetta, solo dopo iniziate a pensare all'implementazione. Se usate metodologie di sviluppo iterative (codice >> test >> codice >> test >> ...) consiglio semplicemente di provare a cambiare l'ordine (quindi: test >> codice >> test >> codice >> ...)

2. I test devono essere indipententi:

Un test non deve dipendere dagli altri nè dall'ordine d'esecuzione. A seconda dell'ambiente d'esecuzione, i test possono essere eseguiti in ordine diverso e se un test utilizzasse, per esempio, il risultato di un test precedente, ci potrebbero essere problemi. Quindi, tendenzialmente, non ci devono essere attributi statici nei test.

3. I test dovrebbero essere i più semplici possibili:

Abbastanza autoesplicativo: preferibile avere un'asserzione per test (tecnicamente ogni metodo di test dovrebbe testare UNA sola cosa) e, nel caso se ne utilizzino di multiple, utilizzate assertAll per eseguire tutte le asserzioni anche in caso di fallimento di una delle prime (altrimenti il test si fermerebbe alla prima asserzione fallita).

4. (repetita iuvant) I test devono essere deterministici:

Se non è fattibile testare ogni possibile scenario (difficilmente si riesce), sceglietene un po' (possibilmente casi limite) e testate quelli. Un test casuale passato non dà alcuna informazione sul corretto funzionamento del codice, in quanto, a parità di implementazione, potrebbe casualmente fallire e, soprattutto, non è facilmente riproducibile.

5. Scrivete test per debuggare:

Nel caso si presentino dei bug (o magari dovrei scrivere: "quando si presenteranno dei bug"), prima ancora di toccare codice, scrivete un test che replichi il bug per essere sicuri d'individuarne la causa e, una volta che il test sarà fallito, potrete andare a modificare il codice. L'ultima cosa che ci serve è andare a modificare codice funzionante per una nostra ipotesi sbagliata.

6. Utilizzate correttamente le asserzioni:

Le asserzioni sono essenzialmente tutte del tipo: assertEquals(valore_atteso, valore_ottenuto). Questo significa che il primo valore deve essere un parametro costante del test e il secondo, invece, sarà un valore buttato fuori da qualunque cosa stiate testando. È essenzialmente l'unico modo corretto di utilizzare le asserzioni. Utilizzarle per verificare che due variabili d'implementazione siano uguali serve solo a testare la coerenza interna della logica ma non la sua correttezza.