[CICLO3] Test Driven Development Task 3.2.3 (71104 vs fek THE ARMAGEDDON)

[71104]

fek, quando vuoi sono pronto :p
direi che cominciare a prendere il codice dei test che avevo fatto, eventualmente anche il codice della classe, e semplificarlo; oppure se vuoi ricominciamo dall'inizio, come vuoi. intanto questo era il codice dei test:

public class TestPlayerGrid extends TestCase
{
private Config config;

private Bounds bounds;

private int rows;
private int cols;
private PlayerGrid pg;


public void setUp()
{
config = Config.createForTesting();
bounds = new Bounds(40, 40, 256, 448, config);
pg = new PlayerGrid(config, bounds);
rows = pg.getRows();
cols = pg.getColumns();
}


public void testDimensions()
{
assertEquals(pg.getRows(), rows);
assertEquals(pg.getColumns(), cols);
assertEquals(rows, config.getIntProperty("rows"));
assertEquals(cols, config.getIntProperty("columns"));

try
{
pg.getAt(0, 0);
pg.getAt(rows - 1, 0);
pg.getAt(0, cols - 1);
pg.getAt(rows - 1, cols - 1);
}
catch(IllegalArgumentException e)
{
fail("legal arguments rejected");
}
}


private void retrievalTest(boolean empty)
{
for (int y = 0; y < rows; y++)
{
for (int x = 0; x < cols; x++)
{
if (empty)
{
assertNull(pg.getAt(y, x));
}
else
{
assertNotNull(pg.getAt(y, x));
}
}
}
}

private void insertionTest()
{
for (int y = 0; y < rows; y++)
{
for (int x = 0; x < cols; x++)
{
assertTrue(pg.addGem(y, x, "diamond"));
assertFalse(pg.addGem(y, x, "diamond"));
}
}
}

private void removalTest()
{
for (int y = 0; y < rows; y++)
{
for (int x = 0; x < cols; x++)
{
assertTrue(pg.removeGem(y, x));
assertFalse(pg.removeGem(y, x));
}
}
}


public void testCells()
{
retrievalTest(true);
insertionTest();
retrievalTest(false);
removalTest();
}


public void testAssignBounds()
{
pg.addGem(0, 0, "diamond");
assertSame(pg.getAt(0, 0).getBounds(), bounds);
pg.removeGem(0, 0);
}


public void testDraw()
{
MockEngine engine = new MockEngine();

for (int y = 0; y < rows; y++)
{
for (int x = 0; x < cols; x++)
{
pg.addGem(y, x, "diamond");
}
}

pg.draw(engine);
assertEquals(engine.getNumberOfQuadsDrawn(), rows * cols);

for (int y = 0; y < rows; y++)
{
for (int x = 0; x < cols; x++)
{
pg.removeGem(y, x);
}
}
}
}

[fek]

Come sempre non fate commit sul Server perche' la build sara' precaria mentre scriviamo il task. Spengo la build machine.

Questo e' il task:
3.2.3 Creare una matrice di sprite MxN allineate al background e disegnare i diamanti contenuti

Inziamo da zero.

Il primo test te lo do' io, ma prima la cena :)
Torno fra mezz'ora.

[fek]

Primo test. Dobbiamo creare una matrice NxM. Appena creata una matrice e'... vuota.

Ecco il test:


public void testIsEmpty()
{
Grid grid = new Grid(14, 8);

assertTrue(grid.isEmpty());
}


71104, a te. Ricorda: l'implementazione piu' semplice possibile e strettamente solo il codice che serve per passare il test. Non una riga di codice in piu'.

[71104]

pronti; è il minimo indispensabile imho:

public class Grid
{
private int rows;
private int columns;

private Gem grid[][];

public Grid(int rows, int columns)
{
this.rows = rows;
this.columns = columns;
grid = new Gem[rows][columns];
}

public boolean isEmpty()
{
boolean result = true;
for (int y = 0; result && (y < rows); y++)
{
for (int x = 0; result && (x < columns); x++)
{
if (null != grid[y][x])
{
result = false;
}
}
}
return result;
}
}

[fek]

Non e' neppure vicino a essere il minimo indispensabile!! :D

Ho chiesto il minimo indispensabile per far passare il test e non una riga di codice in piu'. In quel codice ne vedo almeno venti di troppo.


public class Grid
{
public Grid(int rows, int columns)
{
}

public boolean isEmpty()
{
return true;
}
}


Questo e' il codice piu' semplice possibile che fa passare il test.

Ora vediamo se inserendo un diamante la griglia e' ancora vuota:


public void testIsNotEmpty()
{
Grid grid = new Grid(14, 8);

grid.insertDiamond(0, 0);
assertFalse("Grid is still empty after an insert", grid.isEmpty());
}


Il codice piu' semplice possibile!
Si', ancora piu' semplice di quello che stai pensando ora. Molto piu' semplice.

Ricorda i passi:
- fai compilare prima il test
- guarda che fallisce con l'implementazione corrente
- fai la minima aggiunta che fa passare il test
- guarda il test che passa
- refactoring ed elimina il codice duplicato (ad esempio nei test)

[71104]

LOL, ok, :D allora ecco secondo me il codice MINIMALE per far passare il secondo test a dispetto di tutto quello che potrebbero intuitivamente significare i nomi assegnati alla classe e ai suoi metodi :D

public class Grid
{
private boolean empty = true;

public Grid(int rows, int columns)
{
}

public void insertDiamond(int row, int column)
{
empty = false;
}

public boolean isEmpty()
{
return empty;
}
}

[fek]

Perfetto!
Ora andiamo avanti e testiamo l'inserimento.
Scrivi tu il test questa volta, inserisci un diamante e testi che ci sia un diamante nella posizione inserita.

[71104]

Perfetto!
Ora andiamo avanti e testiamo l'inserimento.
Scrivi tu il test questa volta, inserisci un diamante e testi che ci sia un diamante nella posizione inserita. ok, allora suppongo che l'inserzione alla posizione (0, 0) si potrebbe testare così:

public void testInsertion()
{
Grid grid = new Grid(14, 8);

grid.insertDiamond(0, 0);
assertNotNull("insertion failed", grid.getAt(0, 0));
}


volendo si potrebbe perfezionare mettendoci i cicli for ed effettuando l'inserzione in tutte le caselle.

piccola nota: penso sia giunto il momento di rendere grid un campo private della classe e gi aggiungere il metodo setUp :p

[fek]

Ottimo test!
Volendo fare i pignolissimi, avrei preferito un metodo del chiamato isDiamondAt() che ritorni un boolean, per il principio di passare sempre le informazioni strettamente necessarie. A questo livello ci serve solo l'informazione se un diamante e' presente o meno ad una data posizione, non ci serve sapere quale diamante.


volendo si potrebbe perfezionare mettendoci i cicli for ed effettuando l'inserzione in tutte le caselle.

E provaci se hai il coraggio :D

piccola nota: penso sia giunto il momento di rendere grid un campo private della classe e gi aggiungere il metodo setUp :p

Ottima idea. Fai pure. Elimina la duplicazione. Modifica il test per restituire solo un boolean. Poi scrivi il codice per farlo compilare e farlo fallire e infine il codice per far passare il test. Mi raccomando solo il codice piu' semplice possibile per far passare il test.

[71104]

oplà! direi che bisogna fare così:

public class TestGrid extends TestCase
{
private Grid grid;

public void setUp()
{
grid = new Grid(14, 8);
}

public void testIsEmpty()
{
assertTrue("Grid is not empty", grid.isEmpty());
}

public void testIsNotEmpty()
{
grid.insertDiamond(0, 0);
assertFalse("Grid is still empty after an insert", grid.isEmpty());
}

public void testInsertion()
{
grid.insertDiamond(0, 0);
assertTrue("insertion failed", grid.isDiamondAt(0, 0));
}
}

eliminata la duplicazione di codice (che sarebbe diventata una quadruplicazione ^^); il test prima falliva, e per non farlo fallire ho trasformato Grid come segue:

public class Grid
{
private boolean empty = true;

private boolean grid[][];

public Grid(int rows, int columns)
{
grid = new boolean[rows][columns];
}

public void insertDiamond(int row, int column)
{
empty = false;
grid[row][column] = true;
}

public boolean isEmpty()
{
return empty;
}

public boolean isDiamondAt(int row, int column)
{
return grid[row][column];
}
}

in tal modo però c'è un'altra piccola duplicazione: il campo empty duplica l'informazione del fatto che la griglia è completamente vuota oppure no, perché la stessa informazione si può ottenere ciclando su tutta la matrice, solo che se inserisco troppo codice in una volta sola il TDD si offende :D

[fek]

Infatti si e' offeso, quella non e' il codice minimale che fa passare il test :)

Ma e' questo:


public class Grid
{
private boolean empty = true;

public Grid(int rows, int columns)
{
}

public void insertDiamond(int row, int column)
{
empty = false;
}

public boolean isEmpty()
{
return empty;
}

public boolean isDiamondAt(int row, int column)
{
return true;
}
}


Ora, continuiamo l'implementazione cambiando il test in questo:


public void testInsertion()
{
assertFalse("no diamond at (0,0)", grid.isDiamondAt(0, 0));
grid.insertDiamond(0, 0);
assertTrue("insertion failed", grid.isDiamondAt(0, 0));
}


Che ovviamente ora fallisce. Ma ci avvicina ancora di un passetto alla fine del task.
Scrivi il test che lo fa passare. La versione piu' semplice possibile!

[71104]

vabbè, ragazzi sarò onesto, questa modifica me l'ha suggerita fek :D
comunque eccola:

public boolean isDiamondAt(int row, int column)
{
return !empty;
}

[fek]

Bene. Siamo un passettino piu' vicini alla fine.
Ora andiamo avanti per triangolazione. Ti scrivo io il prossimo test:


public void testTwoInsertions()
{
grid.insertDiamond(0, 0);
assertFalse("there's a diamond at (0,1) before insertion", grid.isDiamondAt(0, 1));
grid.insertDiamond(0, 1);
assertTrue("insertion failed at (0,1) ", grid.isDiamondAt(0, 1));
}


Ora la matrice non te la toglie nessuno :)
Ma non scrivere una riga di codice in piu'.

[71104]

ok ci sono; guarda non ho neanche messo i campi privati rows e columns :D

public class Grid
{
private boolean empty = true;

private boolean grid[][];

public Grid(int rows, int columns)
{
grid = new boolean[rows][columns];
}

public void insertDiamond(int row, int column)
{
empty = false;
grid[row][column] = true;
}

public boolean isEmpty()
{
return empty;
}

public boolean isDiamondAt(int row, int column)
{
return grid[row][column];
}
}

[fek]

Bene! Ora c'e' sempre una piccola duplicazione fra empty e la griglia, ma per ora possiamo sopravvivere. Magari la togliamo piu' in la'.
Una piccola cosa mi turba: che succede se inserisco un diamante in una posizione in cui c'e' gia'?
Vorrei che si scatenasse una bella eccezione, scrivi il test relativo.

[71104]

Bene! Ora c'e' sempre una piccola duplicazione fra empty e la griglia, ma per ora possiamo sopravvivere. Magari la togliamo piu' in la'.
Una piccola cosa mi turba: che succede se inserisco un diamante in una posizione in cui c'e' gia'?
Vorrei che si scatenasse una bella eccezione, scrivi il test relativo. be', nella precedente implementazione (quella di cui ho riportato i test nel primo post) il metodo di inserzione ritornava false, e analogamente si comportava il metodo di rimozione, ma il Coach sei tu :D

public void testRedundantInsertion()
{
try {
grid.insertDiamond(0, 0);
grid.insertDiamond(0, 0);
}
catch (IllegalArgumentException e)
{
return;
}
fail("Double insertion at same position should not be allowed");
}

[fek]

Buon test che fallisce. Implementazione ora.

[71104]

ok questo era semplice :p

public void insertDiamond(int row, int column)
{
if (grid[row][column])
{
throw new IllegalArgumentException();
}

empty = false;
grid[row][column] = true;
}


ora? :)

[fek]

edit: ho scritto una scemenza.

Ora mi serve un test che mi dica che nella posizione in cui inserisco un diamante ci sia effettivamente un oggetto di tipo Gem che poi disegneremo.

Scrivi il test relativo.

[71104]

direi di procedere così:

public void testRetrieval()
{
grid.insertDiamond(0, 0);
Gem gem = grid.getGemAt(0, 0);
assertNotNull("Gem retrieval failed", gem);
}

[fek]

Bene, implementalo. Io faccio un attimo di refactoring.

Eccolo qui:


public void insertDiamond(int row, int column)
{
if (isCellFull(row, column))
{
throw new IllegalArgumentException();
}

empty = false;
grid[row][column] = true;
}

private boolean isCellFull(int row, int column)
{
return grid[row][column];
}


Si legge meglio come se fosse inglese.

[fek]

Viste le difficolta' con il test lo modifichiamo leggermente cosi':


public void testRetrieval()
{
grid.insertDiamond(0, 0, Gem.createForTesting());
Gem gem = grid.getGemAt(0, 0);
assertNotNull("Gem retrieval failed", gem);
}

[VICIUS]

private boolean isCellFull(int row, int column)
{
return grid[row][column];
}

Non fa la stessa cosa di isDiamondAt(,) ?

ciao ;)

[71104]

Non fa la stessa cosa di isDiamondAt(,) ? infatti la modifica consisteva semplicemente nel cambiare il nome a isDiamondAt.

[fek]

Non fa la stessa cosa di isDiamondAt(,) ?

ciao ;)

Si'! Duplicazione. Una delle due deve scomparire. isDiamondAt() mi piace di piu', teniamo quella.

[71104]

un po' di refactoring (accordato su MSN) e soluzione dell'ultimo test; nuova versione di TestGrid:

public class TestGrid extends TestCase
{
private Grid grid;
private Gem gem;

public void setUp()
{
grid = new Grid(14, 8);
gem = Gem.createForTesting();
}

public void testIsEmpty()
{
assertTrue("Grid is not empty", grid.isEmpty());
}

public void testIsNotEmpty()
{
grid.insertDiamond(0, 0, gem);
assertFalse("Grid is still empty after an insert", grid.isEmpty());
}

public void testInsertion()
{
assertFalse("there's a diamond at (0,0) before insertion", grid.isCellFull(0, 0));
grid.insertDiamond(0, 0, gem);
assertTrue("insertion failed", grid.isCellFull(0, 0));
}

public void testTwoInsertions()
{
grid.insertDiamond(0, 0, gem);
assertFalse("there's a diamond at (0,1) before insertion", grid.isCellFull(0, 1));
grid.insertDiamond(0, 1, gem);
assertTrue("insertion failed at (0,1) ", grid.isCellFull(0, 1));
}

public void testRedundantInsertion()
{
try {
grid.insertDiamond(0, 0, gem);
grid.insertDiamond(0, 0, gem);
}
catch (IllegalArgumentException e)
{
return;
}
fail("Double insertion at same position should not be allowed");
}

public void testRetrieval()
{
grid.insertDiamond(0, 0, gem);
assertSame("Gem retrieval failed", gem, grid.getGemAt(0, 0));
}
}

e nuova versione di Grid:

public class Grid
{
private boolean empty = true;
private Gem grid[][];

public Grid(int rows, int columns)
{
grid = new Gem[rows][columns];
}

public void insertDiamond(int row, int column, Gem gem)
{
if (isCellFull(row, column))
{
throw new IllegalArgumentException();
}

empty = false;
grid[row][column] = gem;
}

public boolean isEmpty()
{
return empty;
}

public boolean isCellFull(int row, int column)
{
return null != grid[row][column];
}

public Gem getGemAt(int row, int column)
{
return grid[row][column];
}
}

[fek]

Ci siamo quasi. Ora il test per il disegno, ci serve MockDisplay per questo.

Te la senti di scriverlo tu? Scrivilo semplice semplice.

[71104]

Ci siamo quasi. Ora il test per il disegno, ci serve MockDisplay per questo.

Te la senti di scriverlo tu? Scrivilo semplice semplice.
eccolo qua: "fallisce correttamente" :p :p :p
scusate il ritardo ma abbiamo avuto problemi di sincronizzazione causa commit in conflitto ^^

public void testDraw()
{
MockEngine engine = new MockEngine();

grid.insertDiamond(0, 0, gem);
grid.insertDiamond(0, 1, gem);

grid.draw(engine);
assertEquals(engine.getNumberOfQuadsDrawn(), 2);
}

[fek]

Perfetto. Implementa pure.

(abbiamo un filo allungato i passi ora, ci sentiamo sicuri e possiamo osare qualcosa di piu')

[71104]

implementazione di un metodo draw che risolve il test:

public void draw(AbstractEngine engine)
{
for (int y = 0; y < grid.length; y++)
{
for (int x = 0; x < grid[y].length; x++)
{
if (null != grid[y][x])
{
grid[y][x].draw(engine);
}
}
}
}

[fek]

C'e' una duplicazione, la tolgo io:


public void draw(AbstractEngine engine)
{
for (int y = 0; y < grid.length; y++)
{
for (int x = 0; x < grid[y].length; x++)
{
if (isDiamondAt(y, x))
{
grid[y][x].draw(engine);
}
}
}
}


Ci siamo quasi. L'unico problema ora e' che i diamanti sono tutti disegnati alla stessa posizione e non nella loro cella. Ci serve un test che inserisca un diamante e che controlli che la griglia lo abbia spostato alla posizione corretta della relativa cella

A te.

[71104]

public void testPositions()
{
grid.insertDiamond(0, 0, gem);
assertTrue((0 == gem.getX()) && (0 == gem.getY()));

Gem otherGem = Gem.createForTesting();
grid.insertDiamond(1, 0, otherGem);
assertTrue((0 == otherGem.getX()) && (Gem.GEM_SIZE == otherGem.getY()));
}

[fek]

Vedo un paio di problemi, uno stilistico, l'altro logico.

Il primo e' banale, invece di avere gli && all'interno dell'assert, preferisco due assert separate, cosi' e' piu' facile capire che cosa fallisce.

Il secondo invece riguarda il tipo di test, non e' molto robusto perche' si basa sulla dimensione dello sprite per definire la dimensione della cella. Funziona, e' ok, ma se possiamo scrivere qualcosa di piu' robusto ora e' preferibile.

Direi di fissare noi una dimensione della cella e poi piazzare la gemma di conseguenza. 32 pixel possono andare bene?

Un'ultima cosa, io testerei ad una posizione diversa da (1,0), qualcosa tipo (5, 6), e' piu' robusto.

[71104]

Vedo un paio di problemi, uno stilistico, l'altro logico.

Il primo e' banale, invece di avere gli && all'interno dell'assert, preferisco due assert separate, cosi' e' piu' facile capire che cosa fallisce. ok, ora modifico.

Il secondo invece riguarda il tipo di test, non e' molto robusto perche' si basa sulla dimensione dello sprite per definire la dimensione della cella. Funziona, e' ok, ma se possiamo scrivere qualcosa di piu' robusto ora e' preferibile.

Direi di fissare noi una dimensione della cella e poi piazzare la gemma di conseguenza. 32 pixel possono andare bene? ok, in effetti hai ragione, basarsi sulla dimensione della gemma è concettualmente errato; piuttosto dovremmo fissare la dimensione della cella come membro public final static di Grid e basarci su quella; inserire direttamente il numero 32 nel sorgente del test mi pare brutto :D

Un'ultima cosa, io testerei ad una posizione diversa da (1,0), qualcosa tipo (5, 6), e' piu' robusto. finora procedendo sul principio della triangolazione (se ho capito bene di cosa si tratta) abbiamo sempre testato le posizioni (0, 0) e (0, 1); io direi di uniformare tutte le posizioni a (0, 0) e (1, 1), che è più robusto di (0, 0) e (0, 1), oppure altrimenti di differenziarle tutte quante tranne la prima; cioè la prima rimane sempre (0, 0), le altre cambiano tutte con valori casuali.

[fek]

finora procedendo sul principio della triangolazione (se ho capito bene di cosa si tratta) abbiamo sempre testato le posizioni (0, 0) e (0, 1); io direi di uniformare tutte le posizioni a (0, 0) e (1, 1), che è più robusto di (0, 0) e (0, 1), oppure altrimenti di differenziarle tutte quante tranne la prima; cioè la prima rimane sempre (0, 0), le altre cambiano tutte con valori casuali.

Per triangolazione si intende usare due valori diversi per testare lo stesso code path. Ma non necessariamente gli stessi valori in tutti i test. Anzi, e' meglio testare piu' valori differenti in test differenti per esercitare il piu' possibile lo spazio di input.

[71104]

allora, ecco la correzione al test:

public void testPositions()
{
grid.insertDiamond(4, 2, gem);
assertTrue(Grid.CELL_SIZE * 2 == gem.getX());
assertTrue(Grid.CELL_SIZE * 4 == gem.getY());

Gem otherGem = Gem.createForTesting();
grid.insertDiamond(3, 5, otherGem);
assertTrue(Grid.CELL_SIZE * 5 == otherGem.getX());
assertTrue(Grid.CELL_SIZE * 3 == otherGem.getY());
}

ora lo svolgo...

[fek]

Si e' fatto tardi, ci aggiorniamo a domani :)

[71104]

ok, per svolgerlo è bastato aggiungere una linea a insertDiamond:

public void insertDiamond(int row, int column, Gem gem)
{
if (isDiamondAt(row, column))
{
throw new IllegalArgumentException();
}

empty = false;

gem.setPos(column * CELL_SIZE, row * CELL_SIZE);
grid[row][column] = gem;
}

ho aggiunto il gem.setPos(...);

ragazzi, io e fek finiremo domani, il task è quasi completo, mancano solo alcuni test:
1) l'area attualmente occupata dalla griglia non ha l'origine corretto: inizia da (0, 0), mentre invece deve iniziare ad un determinato punto (probabilmente aggiungeremo dei Bounds alla classe)
2) la classe non fa nessun controllo per verificare la correttezza dei parametri row e column che riceve in input in vari metodi (a dire il vero non se già il Java di per se' lanci delle eccezioni quando si sfondano i margini di un array, credo di si: in tal caso non abbiamo neanche bisogno di fare i controlli)
3) varie ed eventuali...

[cionci]

Direi di rinominare insertDiamond in insertGem ;)

[^TiGeRShArK^]

e visto che ci siamo allora pure isDiamondAt() in isGemAt() ;)

[fek]

ragazzi, io e fek finiremo domani, il task è quasi completo, mancano solo alcuni test:
1) l'area attualmente occupata dalla griglia non ha l'origine corretto: inizia da (0, 0), mentre invece deve iniziare ad un determinato punto (probabilmente aggiungeremo dei Bounds alla classe)
2) la classe non fa nessun controllo per verificare la correttezza dei parametri row e column che riceve in input in vari metodi (a dire il vero non se già il Java di per se' lanci delle eccezioni quando si sfondano i margini di un array, credo di si: in tal caso non abbiamo neanche bisogno di fare i controlli)
3) varie ed eventuali...

Il ragazzo ha preso il ritmo con i test adesso ;)

(Hai appena fatto una test list, e sarebbe il modo corretto di procedere prima di iniziare a scrivere il primo test)

[fek]

Direi di rinominare insertDiamond in insertGem ;)

Fate pure.

[cionci]

fek: la prossima volta la fai con me pair programming ?

[fek]

fek: la prossima volta la fai con me pair programming ?

Si', volentieri. Facciamo alla prossima Storia pero' perche' porta via un sacco di tempo.
Ma lo faccio volentieri perche' sembra che l'idea funioni bene e sia utile :)

[cionci]

Ma lo faccio volentieri perche' sembra che l'idea funioni bene e sia utile :)
Anche a me sembra che sia molto utile...

[71104]

allora, per completare il task leggo la lista che mi sono scritto ieri sera (notte :D)
1) serve un test per verificare che la posizione di base della griglia non sia (0, 0), ma (x, y); assumendo l'aggiunta di un metodo setOrigin(float x, float y) alla classe Grid, ecco come modifico la classe TestGrid:

public class TestGrid extends TestCase
{
private Point origin;
private Grid grid;
private Gem gem;

public void setUp()
{
origin = new Point(40, 40);
grid = new Grid(14, 8);
grid.setOrigin(origin.x(), origin.y());
gem = Gem.createForTesting();
}

public void testIsEmpty()
{
assertTrue("Grid is not empty", grid.isEmpty());
}

public void testIsNotEmpty()
{
grid.insertGem(0, 0, gem);
assertFalse("Grid is still empty after an insert", grid.isEmpty());
}

public void testInsertion()
{
assertFalse("there's a Gem at (0,0) before insertion", grid.isGemAt(1, 1));
grid.insertGem(1, 1, gem);
assertTrue("insertion failed", grid.isGemAt(1, 1));
}

public void testTwoInsertions()
{
grid.insertGem(1, 2, gem);
assertFalse("there's a Gem at (0,1) before insertion", grid.isGemAt(3, 0));
grid.insertGem(3, 0, gem);
assertTrue("insertion failed at (0,1) ", grid.isGemAt(3, 0));
}

public void testRedundantInsertion()
{
try
{
grid.insertGem(2, 0, gem);
grid.insertGem(2, 0, gem);
}
catch (IllegalArgumentException e)
{
return;
}
fail("Double insertion at same position should not be allowed");
}

public void testRetrieval()
{
grid.insertGem(1, 0, gem);
assertSame("Gem retrieval failed", gem, grid.getGemAt(1, 0));
}

public void testDraw()
{
MockEngine engine = new MockEngine();

grid.insertGem(3, 1, gem);
grid.insertGem(2, 1, gem);

grid.draw(engine);
assertEquals(engine.getNumberOfQuadsDrawn(), 2);
}

public void testPositions()
{
grid.insertGem(4, 2, gem);
assertTrue(origin.x() + Grid.CELL_SIZE * 2 == gem.getX());
assertTrue(origin.y() + Grid.CELL_SIZE * 4 == gem.getY());

Gem otherGem = Gem.createForTesting();
grid.insertGem(3, 5, otherGem);
assertTrue(origin.x() + Grid.CELL_SIZE * 5 == otherGem.getX());
assertTrue(origin.y() + Grid.CELL_SIZE * 3 == otherGem.getY());
}

public void testOrigin()
{
grid.insertGem(3, 6, gem);
assertEquals(origin.x() + Grid.CELL_SIZE * 6, gem.getX(), 0.001f);
assertEquals(origin.y() + Grid.CELL_SIZE * 3, gem.getY(), 0.001f);

grid.setOrigin(20, 30);
assertEquals(20 + Grid.CELL_SIZE * 6, gem.getX(), 0.001f);
assertEquals(30 + Grid.CELL_SIZE * 3, gem.getY(), 0.001f);
}
}


oltre a modificare il testPositions è stato necessario aggiungere anche un altro test (testOrigin) perché il testPositions di per se' non mi assicura che l'origine funzioni al 100%: la formula base + CELL_SIZE * index (usata per calcolare nei test le coordinate che mi aspetto che assuma la Gem) potrebbe assumere un valore corretto se entrambi i parametri base e index fossero errati; se invece utilizzo anche un test a parte (che funziona diversamente) si riduce praticamente a 0 la possibilità che il sistema di coordinate di Grid mi funzioni "per caso" ;)
impariamo dall'esperienza ragazzi :D :Prrr: :sofico:

[71104]

ecco l'ultima versione della classe Grid che mi permette di passare i nuovi test (ora faccio il commit):

public class Grid
{
public static final int CELL_SIZE = 32;

private boolean empty = true;
private Gem grid[][];

private float orgX = 0;
private float orgY = 0;

public Grid(int rows, int columns)
{
grid = new Gem[rows][columns];
}

public void insertGem(int row, int column, Gem gem)
{
if (isGemAt(row, column))
{
throw new IllegalArgumentException();
}

empty = false;

gem.setPos(orgX + column * CELL_SIZE, orgY + row * CELL_SIZE);
grid[row][column] = gem;
}


public boolean isEmpty()
{
return empty;
}

public boolean isGemAt(int row, int column)
{
return grid[row][column] != null;
}

public Gem getGemAt(int row, int column)
{
return grid[row][column];
}

public void draw(AbstractEngine engine)
{
for (int y = 0; y < grid.length; y++)
{
for (int x = 0; x < grid[y].length; x++)
{
if (isGemAt(y, x))
{
grid[y][x].draw(engine);
}
}
}
}

public void setOrigin(float newOrgX, float newOrgY)
{
for (int y = 0; y < grid.length; y++)
{
for (int x = 0; x < grid[y].length; x++)
{
if (isGemAt(y, x))
{
Gem gem = grid[y][x];
gem.setPos(gem.getX() - orgX + newOrgX,
gem.getY() - orgY + newOrgY);
}
}
}
orgX = newOrgX;
orgY = newOrgY;
}
}


come vedete ho aggiunto setOrigin e fatto una modifica a insertGem.

[71104]

adesso un'ultima cosa e poi il task dovrebbe essere completo:

public void testWrongParams()
{
try
{
grid.insertGem(-1, -1, gem); // underflow
}
catch(Exception e1)
{
try
{
grid.insertGem(100, 100, gem); // overflow
}
catch(Exception e2)
{
return;
}
}
fail("wrong parameters not rejected");
}

questo test mi permette di verificare che la classe Grid rifiuti parametri row e column out of bounds; ebbene, in genere quando faccio partire un test appena fatto questo fallisce, invece questo qui non fallisce, questo perché Java stesso effettua dei controlli con gli array; bene, con l'aiuto del debugger scopro che l'eccezione lanciata è una ArrayIndexOutOfBoundsException, quindi modifico il test in questo modo, che è più restrittivo:

public void testWrongParams()
{
try
{
grid.insertGem(-1, -1, gem); // underflow
}
catch(ArrayIndexOutOfBoundsException e1)
{
try
{
grid.insertGem(100, 100, gem); // overflow
}
catch(ArrayIndexOutOfBoundsException e2)
{
return;
}
}
fail("wrong parameters not rejected");
}

e così il task dovrebbe essere completato; se invece che ArrayOutOfBoundsException fek mi dirà di volere una IllegalArgumentsException allora farò un refactoring cambiando l'eccezione attesa nel test, verificando che il nuovo test fallisca, aggiornando l'implementazione, e verificando che il tast passi.

[fek]

e così il task dovrebbe essere completato; se invece che ArrayOutOfBoundsException fek mi dirà di volere una IllegalArgumentsException allora farò un refactoring cambiando l'eccezione attesa nel test, verificando che il nuovo test fallisca, aggiornando l'implementazione, e verificando che il tast passi.

Si', traduci ArrayOutOfBounds in IllegalArgument.

Ho creato un mostro in meno di tre ore :asd:

[71104]

Si', traduci ArrayOutOfBounds in IllegalArgument.

Ho creato un mostro in meno di tre ore :asd:
detto fatto; test fallimentare:

public void testWrongParams()
{
try
{
grid.insertGem(-1, -1, gem); // underflow
}
catch(IllegalArgumentException e1)
{
try
{
grid.insertGem(100, 100, gem); // overflow
}
catch(IllegalArgumentException e2)
{
return;
}
}
fail("wrong parameters not rejected");
}

per la correzione ho aggiunto questo metodo a Grid:

private void checkArgs(int row, int column)
{
if ((row < 0) || (row >= grid.length) ||
(column < 0) || (column >= grid[0].length))
{
throw new IllegalArgumentException();
}
}

che viene chiamato all'inizio di ogni funzione che usa come input un numero di riga e di colonna. test verde :p

[fek]

detto fatto; test fallimentare:
che viene chiamato all'inizio di ogni funzione che usa come input un numero di riga e di colonna. test verde :p

Non serve :)

Ho fatto refactoring di quel metodo con una soluzione piu' semplice che si limita solo a tradurre l'eccezione gia' generata da Java, e non ha bisogno di quel metodo. Quando arrivo a casa stasera faccio il commit.

[fek]

Al di la' della semplificazione, mi piace molto che torni sul tuo codice dopo il completamento del task per semplificarlo e renderlo piu' chiaro. Un esempio che deve essere seguito da tutti :)