PDA

View Full Version : [CICLO 4] Test Driven Development task 4.2.2 (cdimauro vs 71104)


71104
10-11-2005, 19:29
come al solito i commit saranno precari per un po'.

dunque, test list:
- test per coda vuota
- test per coda non vuota
- test per rimozione di un elemento dalla coda (anche più d'uno)
- test per ordine corretto degli elementi della coda (anche più d'uno)
- test per la correttezza del tipo di evento (pressione o rilascio)
- test per i timestamp
- varie ed eventuali...

allora, il test per la coda vuota è molto semplice:

public void testQueueEmpty()
{
assertTrue(input.isQueueEmpty());
}

che è talmente semplice che faccio anche quello per la coda non vuota :D

public void testQueueNotEmpty()
{
input.generateKey(KeyCode.vk_Left);
assertFalse(input.isQueueEmpty());
}

cdimauro, questi due come li risolveresti? :)

cdimauro
10-11-2005, 20:09
Per il primo test avrei risolto semplicemente aggiungendo il seguente codice alla classe input:


public boolean isQueueEmpty()
{
return true;
}


Aggiungendo il secondo test, mi vedo costretto a introdurre la variabile d'istanza empty a Input e a modificare un costruttore un po' di methodi:


private boolean empty;

private Input(AbstractKeyboard keyboard)
{
this.keyboard = keyboard;
keys = new BitSet();
empty = true;
}

public boolean isQueueEmpty()
{
return empty;
}

public void generateKey(KeyCode key)
{
keys.set(key.value(), true);
empty = false;
}

Il minimo indispensabile per far passare i test... ;)

A te la palla Alberto. :)

71104
10-11-2005, 20:19
ok, a questo punto già mi rendo conto di aver dimenticato un'aspetto importante della gestione della coda: la rimozione di elementi. l'inserzione avviene quando vengono ricevuti eventi dalla tastiera, ma la rimozione? quindi siccome l'operazione di rimozione ci serve, aggiungo questo test:

public void testQueueItemRemoval()
{
input.generateKey(KeyCode.vk_Right);
assertTrue(input.extractKey() == KeyCode.vk_Right);
}

l'operazione di peek casomai la aggiungeremo dopo (non è neanche detto che serva).

PS: ho editato il post iniziale

cdimauro
10-11-2005, 21:05
Per far passare questo test introduco un'altra variabile, che mi serve per ricordare l'ultimo tasto "premuto", e aggiungo anche il metodo extractKey() :


private KeyCode lastKey;

public void generateKey(KeyCode key)
{
keys.set(key.value(), true);
empty = false;
lastKey = key;
}


public KeyCode extractKey()
{
empty = true;
return lastKey;
}
A questo punto scrivo io il test per il punto seguente, che è:
" test per ordine corretto degli elementi della coda (anche più d'uno)".


public void testQueueSequenceOrder()
{
input.generateKey(KeyCode.vk_Right);
input.generateKey(KeyCode.vk_Up);
assertTrue(input.extractKey() == KeyCode.vk_Right);
assertTrue(input.extractKey() == KeyCode.vk_Up);
}

A te Alberto :)

71104
10-11-2005, 21:05
ok, scusate torno tra una mezz'ora, vado a cena ^^

cionci
10-11-2005, 21:09
Posso muovere un dubbio ? Siamo sicuri che serva la rimozione degli eventi dalla coda ? Perchè invece non realizzate una coda circolare di N elementi...
Anche perchè...chi si mette a rimuovere gli eventi dalla coda ? Nel senso che gli utilizzatori di input non possono sapere se lo stesso input servirà anche ad altri quindi la politica di rimozione diverrebbe molto complessa...

cionci
10-11-2005, 21:16
Ah...muovete i test che state scrivendo in it.diamonds.tests.ignore altrimenti la build fallisce ed il repository non deve mai stare con la build rotta...

71104
10-11-2005, 22:02
allora, l'ultimo test di cdimauro introduce la triangolazione sul campo dell'ultimo tasto premuto, nel senso che potrei risolverlo aggiungendo un'altro membro analogo a lastKey, ma per triangolazione ne aggiungo un numero virtualmente illimitato creando la coda :p
per risolvere il test ho aggiunto un campo next a KeyCode (inizializzato a null) e ho modificato il codice di generateKey ed extractKey come segue:

public void generateKey(KeyCode key)
{
keys.set(key.value(), true);
empty = false;
if (null != queue)
{
queue.next = key;
}
else {
queue = key;
}
}


public KeyCode extractKey()
{
KeyCode result = queue;
if (null != queue)
{
queue = queue.next;
result.next = null;
}
empty = (null != queue);
return result;
}

(tra parentesi il campo lastKey è ora diventato queue, nome più significativo in questo caso).
cionci, la coda circolare è inutile da implementare, per noi sarebbe solo codice in più, potenzialmente sbagliato, e la rimozione serve per forza: la dovrà usare reactToInput per lavorare sugli input (senza contare che serve necessariamente per testare il corretto ordine degli elementi nella coda).

PS: ovviamente poi dopo aver sistemato reactToInput dovremo anche eliminare la gestione degli input tramite bitset, e questo sarà un refactoring che faremo alla fine.
dal momento che si tratta quasi esclusivamente di rimuovere codice, non sarà necessario usare altri test durante il refactoring, l'importante è controllare ad ogni piccolo cambiamento che i test attuali continuino a passare tutti.

cdimauro
10-11-2005, 22:24
Posso muovere un dubbio ? Siamo sicuri che serva la rimozione degli eventi dalla coda ? Perchè invece non realizzate una coda circolare di N elementi...
Anche perchè...chi si mette a rimuovere gli eventi dalla coda ? Nel senso che gli utilizzatori di input non possono sapere se lo stesso input servirà anche ad altri quindi la politica di rimozione diverrebbe molto complessa...
L'input dovrebbe essere associato a un ben determinato oggetto, che lo "consumerà". E quindi non dovrebbe essere accessibile ad altri oggetti.

Per quanto riguarda la coda circolare, se c'è già una classe Java disponibile non c'è problema. Io non conosco la libreria di Java, quindi non so. Altrimenti è meglio prendere un Vector e implementare la coda in questo modo.

Il tutto IMHO, chiaramente.

71104
10-11-2005, 22:27
ragazzi, tra imprevisti rotture di OO ed errori nostri nel codice questo task sta durando una vita!!! :cry: :cry:
allora, la mia implementazione dei nodi della queue non va bene, e per mettere in evidenza l'errore concettuale basta questo test:

input.generateKey(KeyCode.vk_Right);
input.generateKey(KeyCode.vk_Up);
input.generateKey(KeyCode.vk_Left);
input.generateKey(KeyCode.vk_Up);
assertTrue(input.extractKey() == KeyCode.vk_Right);
assertTrue(input.extractKey() == KeyCode.vk_Up);
assertTrue(input.extractKey() == KeyCode.vk_Left);
assertTrue(input.extractKey() == KeyCode.vk_Up);

il problema è che il Java non gestisce le enum con istanze e puntatori come gli oggetti, cioè se io inserisco due volte un vk_Right io di fatto non ho inserito due oggetti diversi, ma ho inserito due volte la stessa enum, quindi il campo next di vk_Right all'ultima inserzione viene modificato in maniera errata.
ora stiamo cercando la soluzione: sarà necessario implementare la coda diversamente.

71104
10-11-2005, 22:54
ok, ho risolto il problema grazie a cdimauro; scusate il ritardo ma ho avuto un ulteriore contrattempo, come se non bastassero... -_-'
ecco qua le ultime modifiche:

public void generateKey(KeyCode key)
{
keys.set(key.value(), true);
empty = false;
queue.add(key);
}


public KeyCode extractKey()
{
KeyCode result = queue.remove();
empty = queue.isEmpty();
return result;
}

ed inoltre il campo queue è diventato di tipo LinkedList<KeyCode>.

a questo punto la prima cosa che vedo è che c'è una duplicazione perché l'informazione di lista vuota è data sia dal campo empty sia da queue.isEmpty()...

Jocchan
10-11-2005, 22:58
a questo punto la prima cosa che vedo è che c'è una duplicazione perché l'informazione di lista vuota è data sia dal campo empty sia da queue.isEmpty()...

Una volta accertato che si tratta di una duplicazione, e che l'eliminazione di uno dei due elementi non comporta nessun danno, sai bene cosa bisogna fare ;)

cdimauro
10-11-2005, 23:09
Ho pensato io al refactoring. :)

Ho eliminato empty da tutte le parti e l'ho rimpiazzato soltanto in isQueueEmpty:

public boolean isQueueEmpty()
{
return queue.size() == 0;
}

71104
10-11-2005, 23:16
ok, adesso il prossimo test (che testa il tipo di evento, cioè pressione o rilascio del tasto) richiede un po' di refactoring dei test precedenti:

public void testQueueItemRemoval()
{
input.generateKey(KeyCode.vk_Right);
assertTrue(input.extractKey().key() == KeyCode.vk_Right);
}


public void testQueueSequenceOrder()
{
input.generateKey(KeyCode.vk_Right);
input.generateKey(KeyCode.vk_Up);
assertTrue(input.extractKey().key() == KeyCode.vk_Right);
assertTrue(input.extractKey().key() == KeyCode.vk_Up);
}


public void testEventType()
{
input.generateKey(KeyCode.vk_Left, true);
input.generateKey(KeyCode.vk_Left, false);
assertTrue(input.extractKey().state());
assertFalse(input.extractKey().state());
}

testEventType è il nuovo test; ho assunto che la nuova classe KeyEvent che costituisce un nodo della lista abbia un metodo public state() che restituisce il tipo di evento (boolean quindi) e un metodo key() che restituisce il KeyCode.
la sequenza generata in questo test è semplice, pressione e rilascio di un tasto, ma poi si potrebbe fare un ulteriore test più robusto per testare combinazioni più complesse (casomai dopo però).

EDIT: ho cambiato il nome del metodo type() in state().

Jocchan
10-11-2005, 23:18
la sequenza generata in questo test è semplice, pressione e rilascio di un tasto, ma poi si potrebbe fare un ulteriore test più robusto per testare combinazioni più complesse (casomai dopo però).

Mi raccomando facciamolo, stiamo implementando questa coda proprio per le situazioni più concitate quindi meglio assicurarci che tutto vada per il meglio ;)

71104
10-11-2005, 23:21
Mi raccomando facciamolo, stiamo implementando questa coda proprio per le situazioni più concitate quindi meglio assicurarci che tutto vada per il meglio ;) hai ragione ;)

cdimauro
10-11-2005, 23:56
Per far passare il test ho implementato una classe KeyEvent in input, che serve a contenere l'informazione relativa al tasto premuto e al suo stato, modificando le parti di input che prima usavano KeyCode:

public class KeyEvent
{
private KeyCode keyCode;

private boolean keyState;


public KeyEvent(KeyCode keyCode, boolean keyState)
{
this.keyCode = keyCode;
this.keyState = keyState;
}


public KeyCode key()
{
return keyCode;
}


public boolean state()
{
return keyState;
}

}


private LinkedList<KeyEvent> queue;


private Input(AbstractKeyboard keyboard)
{
this.keyboard = keyboard;
keys = new BitSet();
queue = new LinkedList<KeyEvent>();
}


public void generateKey(KeyCode key, boolean state)
{
keys.set(key.value(), state);
queue.add(new KeyEvent(key, state));
}


public void generateKey(KeyCode key)
{
generateKey(key, true);
}


public KeyEvent extractKey()
{
return queue.remove();
}
Inoltre adesso generateKey(KeyCode key) utilizza generateKey(KeyCode key, boolean state), dove ho "centralizzato" tutto il codice.

Qui e in qualche altra parte si potrà effettuare del refactoring, ma per adesso m'interessa finire: il letto mi aspetta. :p

cdimauro
11-11-2005, 00:04
Rimane da associate un timestamp a ogni evento generato. Il che vuol dire accertarsi che un tasto premuto prima NON possa avere un timestamp
maggiore di un tasto premuto dopo:

public void testEventTimestamp()
{
input.generateKey(KeyCode.vk_Left, true);
input.generateKey(KeyCode.vk_Left, false);
assertTrue(input.extractKey().getTimestamp() < input.extractKey().getTimestamp());
}
A te Alberto. :)

P.S. Spero che Java non sia come il C, per cui un compilatore può processare liberamente gli argomenti di una funzione/metodo, altrimenti bisognerà modificare l'assert mettendo il primo timestamp in una variabile temporanea. :p

71104
11-11-2005, 00:22
allora, ho risolto il test mettendo un campo timestamp in KeyEvent con relativo getter (il "setter" in questo caso sarebbe il costruttore, al quale ho aggiunto un parametro); ma c'è un piccolo dettaglio da notare: pur essendo il codice di implementazione corretto, il test inizialmente non passava perché i due timestamp risultavano identici, e non strettamente l'uno minore dell'altro; ho inserito tra le due inserzioni una piccola sleep da 100 ms, soluzione secondo me migliore del cambiare il simbolo < con <= (come proponeva cdimauro) in quanto il simbolo <= potrebbe rendere inutile il test (non testiamo realmente il caso, più realistico, in cui il primo timestamp è minore del secondo; cambiando le cose in teoria potrebbe pure saltare fuori l'errore che in realtà avevo sbagliato a scrivere il codice e quindi mettendo la sleep in realtà i due timestamp risultavano invertiti, o cose del genere, ma il test non lo evidenziava).
ecco come è diventato il test:

public void testEventTimestamp() throws InterruptedException
{
input.generateKey(KeyCode.vk_Left, true);
Thread.sleep(100);
input.generateKey(KeyCode.vk_Left, false);
assertTrue(input.extractKey().getTimestamp() < input.extractKey().getTimestamp());
}

ed ecco il codice della classe KeyEvent:

public class KeyEvent
{
private KeyCode keyCode;

private boolean keyState;

private long timestamp;


public KeyEvent(KeyCode keyCode, boolean keyState, long timestamp)
{
this.keyCode = keyCode;
this.keyState = keyState;
this.timestamp = timestamp;
}


public KeyCode key()
{
return keyCode;
}


public boolean state()
{
return keyState;
}

public long getTimestamp()
{
return timestamp;
}

}

a nanna :D
anzi, io non ancora, voglio aggiungere quell'ultimo test della sequenza di input complessa... :p

cdimauro
11-11-2005, 00:25
OK. Per adesso va bene: vado a nanna che il letto mi aspetta da almeno 3 ore :-D

Ci sono un po' di refactoring da fare, un po' di rename di metodi / attributi, e dovremmo discutere di qualche dettaglio come faceva notare Alberto.

A domani. Buonanotte a tutti :)

71104
11-11-2005, 00:30
piccola modifica all'ultimo test:

public void testEventTimestamp() throws InterruptedException
{
input.generateKey(KeyCode.vk_Left, true);
Thread.sleep(100);
input.generateKey(KeyCode.vk_Left, false);
long t1 = input.extractKey().getTimestamp();
assertTrue(t1 < input.extractKey().getTimestamp());
}

in questo modo il primo timestamp viene estratto sicuramente prima del secondo, e ci mettiamo al riparo da problemi che di fatto non dovrebbero esistere, ma come dicevo anche a cdimauro non è mai bene affidarsi a quel tipo di dettagli.

71104
11-11-2005, 00:39
e per finire:

public void testComplexInputSequence()
{
input.generateKey(KeyCode.vk_Up, true);
input.generateKey(KeyCode.vk_Down, true);
input.generateKey(KeyCode.vk_Up, false);
input.generateKey(KeyCode.vk_Down, false);

Input.KeyEvent ke;

ke = input.extractKey();
assertEquals(KeyCode.vk_Up, ke.key());
assertTrue(ke.state());

ke = input.extractKey();
assertEquals(KeyCode.vk_Down, ke.key());
assertTrue(ke.state());

ke = input.extractKey();
assertEquals(KeyCode.vk_Up, ke.key());
assertFalse(ke.state());

ke = input.extractKey();
assertEquals(KeyCode.vk_Down, ke.key());
assertFalse(ke.state());
}

è un test decisamente brutto ma ci vuole (e il Customer è della stessa opinione :Prrr: ); testa una sequenza di input un po' più lunga delle altre (con pressione contemporanea di due tasti) e assicura praticamente senza possibilità di errore che la queue sia effettivamente FIFO.

a domani la rimozione del bitset ;)
buonanotte a tutto il team :D

71104
11-11-2005, 00:43
buonanotte a tutto il team :D unmomentofermitutti!!!
che cos'è questo obbrobbrio?? :mad:

public boolean isQueueEmpty()
{
return queue.size() == 0;
}

molto meglio così:

public boolean isQueueEmpty()
{
return queue.isEmpty();
}

buona notte ;)

^TiGeRShArK^
11-11-2005, 01:12
ragazzi, tra imprevisti rotture di OO ed errori nostri nel codice questo task sta durando una vita!!! :cry: :cry:
allora, la mia implementazione dei nodi della queue non va bene, e per mettere in evidenza l'errore concettuale basta questo test:

input.generateKey(KeyCode.vk_Right);
input.generateKey(KeyCode.vk_Up);
input.generateKey(KeyCode.vk_Left);
input.generateKey(KeyCode.vk_Up);
assertTrue(input.extractKey() == KeyCode.vk_Right);
assertTrue(input.extractKey() == KeyCode.vk_Up);
assertTrue(input.extractKey() == KeyCode.vk_Left);
assertTrue(input.extractKey() == KeyCode.vk_Up);

il problema è che il Java non gestisce le enum con istanze e puntatori come gli oggetti, cioè se io inserisco due volte un vk_Right io di fatto non ho inserito due oggetti diversi, ma ho inserito due volte la stessa enum, quindi il campo next di vk_Right all'ultima inserzione viene modificato in maniera errata.
ora stiamo cercando la soluzione: sarà necessario implementare la coda diversamente.

mi sa ke il prob è ke giava passa gli oggetti x riferimento :Prrr:
in effetti il suo è un comportamento corretto in questo caso..
se vuoi mettere due istanze dello stesso oggetto te lo devi reinstanziare con una new..
nn hai idea d quanto tempo ho perso x la mia tesi prima d scoprire questa SIMPATICA caratteristica di java ke passa i tipi primitivi (string, booelan, int, floa, ecc.. )x valore e gli oggetti complessi x riferimento...:muro:

PFed
11-11-2005, 11:57
mi sa ke il prob è ke giava passa gli oggetti x
nn hai idea d quanto tempo ho perso x la mia tesi prima d scoprire questa SIMPATICA caratteristica di java ke passa i tipi primitivi (string, booelan, int, floa, ecc.. )x valore e gli oggetti complessi x riferimento...:muro:

Per la precisione Java passa tutti i parametri per valore.

Come ben sai una variabile oggetto contiene il riferimento ad un'oggetto di un determinato tipo, che si trova nello Heap.

Quando il parametro di un metodo è un'oggetto, stai passando PER VALORE il RIFERIMENTO all'oggetto.

Dici, e che cambia ?
Cambia invece, perchè è importante conoscere la differenza per capire i meccanismi:

PSEUDOCODICE JAVA:

class Persona {
public String nome;
}

void parameterTestAssegnaProprietaDentroUnMetodo(Persona pers) {
pers.nome = "Pietropaolo";
}

void parameterTestAssegnaNulDentroUnMetodo(Persona pers) {
pers = null;
}

void Main() {
Persona mario = new Persona();
mario.nome = "Mario";

parameterTestAssegnaProprietaDentroUnMetodo(mario);

System.out.println(mario.nome); // qui mario.nome è diventato "Pietropaolo", quindi potresti pensare che gli oggetti sono passati per riferimento.

parameterTestAssegnaNullDentroUnMetodo(mario);

//dopo questo metodo pero mario non è Null, ed ecco qui perchè è importante capire la differenza!!
}



In pratica stai lavorando con due copie distinte (passaggio per vaolre) del riferimento ad un'oggetto.

cdimauro
11-11-2005, 14:46
unmomentofermitutti!!!
che cos'è questo obbrobbrio?? :mad:

public boolean isQueueEmpty()
{
return queue.size() == 0;
}

molto meglio così:

public boolean isQueueEmpty()
{
return queue.isEmpty();
}

buona notte ;)
Non puoi lamentarti se non ho visto quel metodo fra quelli elencati: a quell'ora normalmente ho già almeno 3 ore di sonno!!! :p

fek
11-11-2005, 19:40
Ho letto il topic, ragazzi, complimenti per lo svolgimento, bravissimi :D

Una piccola nota:

Input.KeyEvent ke;

ke = input.extractKey();

'ke' non vuol dire nulla, 71104, se non inizi a dare nomi significativi alle variabili ti strozzo! :p
quello e' un 'keyEvent'.

Complimenti ancora. Bravi.

BlueDragon
12-11-2005, 13:58
Ma non vi siete scordati qualcosa? :D
La coda risulta sempre vuota...mi pare che gli unici metodi che aggiungano elementi siano generateKey e generateKeyEvent..ma non vengono chiamati nel gioco...sono utilizzati solo dai test :)

71104
12-11-2005, 14:21
Ma non vi siete scordati qualcosa? :D
La coda risulta sempre vuota...mi pare che gli unici metodi che aggiungano elementi siano generateKey e generateKeyEvent..ma non vengono chiamati nel gioco...sono utilizzati solo dai test :) hai ragione, la coda c'è e non viene usata, ma non ci siamo scordati di nulla: quello sarebbe stato oggetto del refactoring finale e del task 4.2.3 ;)

cionci
12-11-2005, 15:16
Così ad occhio l'avreste dovuto fare voi:

4.2.2:
Registrare in una coda all'interno di Input gli eventi "pressione" e "rilascio" dei vari tasti. Ad ogni evento deve essere associato il timestamp in cui l'evento è stato scatenato.

Per evitare di rompere la build bastava lasciare il BitSet e le funzioni che usava reactToInput...

Il 4.2.3 doveva solo fare il refactoring di Grid per usare le funzioni messe a dispozione dalla coda...