[CICLO 6] Test Driven Development task 6.1.4 (DanieleC88 vs 71104)

[71104]

come da titolo :p

la build per ora potrebbe essere precaria, ma cercheremo di lasciarla sempre verde: chi scrive i test non committa, mentre committa solo chi li implementa ;)

dunque, fatemi pensare una test list:
- verificare che Grid si accorga se una gemma sta alla sinistra di una del suo stesso tipo
- verificare bla bla bla destra bla bla bla :D
- verificare bla bla sopra
- verificare sotto
- verificare che Grid illumini la gemma se si accorge che sopra, sotto, a destra o a sinistra c'è una gemma dello stesso tipo

[DanieleC88]

bla bla bla destra bla bla bla? come sei esplicativo... :D

[DanieleC88]

Dimenticavo: ovviamente vanno testate anche le condizioni in cui la gemma NON deve essere illuminata. ;)

[71104]

giusto, quindi la test list diventerebbe così:

- verificare che Grid si accorga se una gemma sta alla sinistra di un'altra dello stesso tipo
- verificare a destra
- verificare sopra
- verificare sotto
- verificare che Grid si accorga quando una gemma non sta a destra, sinistra, sopra o sotto una dello stesso tipo (sempre test separati: infatti mi sa che è meglio metterli in una classe apposta perché sono tanti)
- verificare che quando una gemma sta accanto ad almeno una dello stesso tipo si illumini
- verificare che questo non accada se una gemma non sta accanto a nessuna dello stesso tipo

ecco il primo test:

public void testSameAsLeft()
{
grid.insertGem(5, 3, gem1);
grid.insertGem(5, 4, gem2);
assertTrue(grid.sameAsLeft(gem2));
}


si trova in TestGrid ma è meglio che lo sposto in una nuova classe TestGemBrightnessInGrid.

[71104]

primo commit, perfetto; Daniele ha risolto questo test col seguente codice (un po' modificato da me per far felice Ant e per correggere un erroretto minimo ^^):

public boolean gemIsSameAsLeft(Gem gem)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

if(column < 1)
{
return false;
}

if(getGemAt(row, column).getName().equals(getGemAt(row, column - 1).getName()))
{
return true;
}

return false;
}

[71104]

ok, allora, ho fatto una nuova classe TestGemBrightnessInGrid e spostato il mio test in quest'ultima:

public class TestGemBrightnessInGrid extends TestCase
{
private Grid grid;

private Gem gem1;
private Gem gem2;


public void setUp()
{
grid = Grid.createForTesting();

gem1 = new Gem("diamond");
gem2 = new Gem("diamond");

}


public void testSameAsLeft()
{
grid.insertGem(5, 3, gem1);
grid.insertGem(5, 4, gem2);
assertTrue(grid.gemIsSameAsLeft(gem2));
}

}

nel metodo setUp non ho usato Gem.createForTesting perché ho preferito esplicitare il tipo di gemma nella creazione; infatti tra poco quando faremo i test opposti dovremo anche creare una terza Gem nbella classe di test che dovrà essere di tipo diverso :)

[DanieleC88]

Benissimo, ora non ci resta che testare anche gli altri lati, ci vorrà poco. ;)
public void testSameAsRight()
{
grid.insertGem(5, 3, gem1);
grid.insertGem(5, 4, gem2);
assertTrue(grid.gemIsSameAsRight(gem1));
}


public void testSameAsTop()
{
grid.insertGem(5, 4, gem1);
grid.insertGem(6, 4, gem2);
assertTrue(grid.gemIsSameAsTop(gem2));
}


public void testSameAsBottom()
{
grid.insertGem(5, 4, gem1);
grid.insertGem(6, 4, gem2);
assertTrue(grid.gemIsSameAsBottom(gem1));
}

[71104]

et voilà:

public boolean gemIsSameAsRight(Gem gem)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

if(column >= grid[0].length)
{
return false;
}

if(getGemAt(row, column).getName().equals(getGemAt(row, column + 1).getName()))
{
return true;
}

return false;
}

public boolean gemIsSameAsDown(Gem gem)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

if(row > grid.length)
{
return false;
}

if(getGemAt(row, column).getName().equals(getGemAt(row + 1, column).getName()))
{
return true;
}

return false;
}

public boolean gemIsSameAsUp(Gem gem)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

if(row < 1)
{
return false;
}

if(getGemAt(row, column).getName().equals(getGemAt(row - 1, column).getName()))
{
return true;
}

return false;
}

a questo punto uno pensa che stiamo già a metà del task e invece MANCOPENNIENTE!!! :D :D
carissimo Daniele, prima di continuare seguendo la test list ti sottopongo questo problema:

public void testAffinityCheckWithEmtpyCell()
{
grid.insertGem(5, 5, gem1);
assertFalse(grid.gemIsSameAsLeft(gem1));
}

;)

[71104]

a proposito, sarebbe bene anche riflettere su qualche modo per eliminare quella mostruosa quadruplicazione di codice, ma si tratta di un refactoring e ci penseremo a fine task.

EDIT: anzi, è talmente semplice che ci ho già pensato :p

[DanieleC88]

Per la quadruplicazione di codice non credo ci voglia molto... per il problema delle caselle vuote, be', cosa dire? Il problema è grande ma la soluzione è piccola. Un banalissimo 'if'. :D

public boolean gemIsSameAsLeft(Gem gem)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

if(column < 1)
{
return false;
}

if(getGemAt(row, column - 1) != null)
{
if(getGemAt(row, column).getName().equals(
getGemAt(row, column - 1).getName()))
{
return true;
}
}
return false;
}

E così anche per gli altri. ;)

[Ufo13]

hmmm puoi ancora eliminare un return :)

[cover]

Raga, mettete i test sotto it.diamonds.tests.ignore, almeno non ci sono problemi con la build....e fek non si arrabbia :rolleyes:

[71104]

allora, DanieleC88 si è dovuto disconnettere percui il task dovrò essere completato domani; ho eliminato la quadruplicazione sperando (come suggerito da VICIUS) in un miracoloso fix ma Spartacus mi ha detto sorry, oggi niente miracoli :cry:
ora provo ad aggiustare la build; nel frattempo ecco il codice dei 4 metodi riassunti in un unico metodo:

public boolean gemsAreSame(Gem gem, int xStep, int yStep)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

int otherColumn = column + xStep;
int otherRow = row + yStep;

if((otherColumn < 0) || (otherColumn >= grid[0].length))
{
return false;
}

if((otherRow < 1) || (otherRow >= grid.length))
{
return false;
}

if(getGemAt(otherRow, otherColumn) != null)
{
if(gem.getName().equals(getGemAt(otherRow, otherColumn).getName()))
{
return true;
}
}

return false;
}

ed ecco invece come è diventata la classe dei test:

public class TestGemBrightnessInGrid extends TestCase
{
private Grid grid;

private Gem gem1;

private Gem gem2;


public void setUp()
{
grid = Grid.createForTesting();

gem1 = new Gem("diamond");
gem2 = new Gem("diamond");

}


public void testSameAsLeft()
{
grid.insertGem(5, 3, gem1);
grid.insertGem(5, 4, gem2);
assertTrue(grid.gemsAreSame(gem2, -1, 0));
}


public void testSameAsRight()
{
grid.insertGem(5, 3, gem1);
grid.insertGem(5, 4, gem2);
assertTrue(grid.gemsAreSame(gem1, 1, 0));
}


public void testSameAsUp()
{
grid.insertGem(5, 4, gem1);
grid.insertGem(6, 4, gem2);
assertTrue(grid.gemsAreSame(gem2, 0, -1));
}


public void testSameAsDown()
{
grid.insertGem(5, 4, gem1);
grid.insertGem(6, 4, gem2);
assertTrue(grid.gemsAreSame(gem1, 0, 1));
}


public void testAffinityCheckWithEmtpyCell()
{
grid.insertGem(5, 5, gem1);
assertFalse(grid.gemsAreSame(gem1, 1, 0));
assertFalse(grid.gemsAreSame(gem1, 0, 1));
}

}

[71104]

magia, si è fixata da sola :eek:
tanto meglio :cool:

ci vediamo domani per la seconda puntata; byez :p

[fek]

In questo metodo:


public boolean gemsAroundOfSameType(Gem gem, int column, int row)
{
int otherColumn = gem.getCellColumn() + column;
int otherRow = gem.getCellRow() + row;

if((otherColumn < 0) || (otherColumn >= grid[0].length))
{
return false;
}

if((otherRow < 1) || (otherRow >= grid.length))
{
return false;
}

if(getGemAt(otherRow, otherColumn) != null)
{
if(gem.isSameType(getGemAt(otherRow, otherColumn)))
{
return true;
}
}

return false;
}


I primi due branch non sono testati da alcun test. Aggiungete i test relativi ragazzi. E, soprattutto, state scrivendo troppo codice fra un test e l'altro. Fate passi piu' piccoli.

(Ho fatto un po' di refactoring)

[Ufo13]

non è meglio unire le condizioni dei 2 if annidati con un &&?

[fek]

non è meglio unire le condizioni dei 2 if annidati con un &&?

In genere preferisco svolgere gli if per chiarezza:


public boolean gemsAroundOfSameType(Gem gem, int column, int row)
{
int otherColumn = gem.getCellColumn() + column;
int otherRow = gem.getCellRow() + row;

if((otherColumn < 0) || (otherColumn >= grid[0].length))
{
return false;
}

if((otherRow < 1) || (otherRow >= grid.length))
{
return false;
}

if(getGemAt(otherRow, otherColumn) == null)
{
return false;
}

if(gem.isSameType(getGemAt(otherRow, otherColumn)))
{
return true;
}

return false;
}

[71104]

I primi due branch non sono testati da alcun test. Aggiungete i test relativi ragazzi. hai ragione, ci sarei arrivato alla fine ^^
dobbiamo aggiungere un test che verifichi che venga lanciata eccezione se vengono passati una riga e una colonna fuori dalla griglia; per ora ritorna false, ma è un comportamento "temporaneo".

(Ho fatto un po' di refactoring) ho visto e concordo :)

[DanieleC88]

non è meglio unire le condizioni dei 2 if annidati con un &&?
Si, si potrebbe, ma in genere credo sia meglio spezzettare il codice per renderlo quanto più leggibile si può.

Colgo l'occasione per scusarmi del casino che ho fato ieri, ma purtroppo andavo di fretta, oggi spero di avere più tempo. :(

[71104]

hai ragione, ci sarei arrivato alla fine ^^
dobbiamo aggiungere un test che verifichi che venga lanciata eccezione se vengono passati una riga e una colonna fuori dalla griglia; per ora ritorna false, ma è un comportamento "temporaneo". ma cos'ero, fuso ieri?? :mbe:
non deve mica lanciare eccezione: se le coordinate vanno fuori significa semplicemente che la gemma che ho dato come parametro sta sui bordi :mbe:
per testare gli if basta semplicemente creare dei test che testino il metodo con una gemma messa ai bordi :)
Daniele, tocca provvedere ;)

[DanieleC88]

Non ho molto capito il tuo ultimo post... nel frattempo, ho scritto i test mancanti di ieri:
public void testDifferentFromLeft()
{
grid.insertGem(5, 3, gem2);
grid.insertGem(5, 4, gem3);
assertFalse(grid.gemsAroundOfSameType(gem3, -1, 0));
}


public void testDifferentFromRight()
{
grid.insertGem(5, 3, gem2);
grid.insertGem(5, 4, gem3);
assertFalse(grid.gemsAroundOfSameType(gem2, +1, 0));
}


public void testDifferentFromUp()
{
grid.insertGem(5, 4, gem2);
grid.insertGem(6, 4, gem3);
assertFalse(grid.gemsAroundOfSameType(gem3, 0, -1));
}


public void testDifferentFromDown()
{
grid.insertGem(5, 3, gem2);
grid.insertGem(6, 4, gem3);
assertFalse(grid.gemsAroundOfSameType(gem2, 0, +1));
}

[71104]

ok, quelli erano i contro-test; ora, come diceva ieri fek, i due if iniziali del metodo non sono testati, quindi ho aggiunto io i seguenti test:

public void testGemAtLeftBorder()
{
grid.insertGem(6, 0, gem1);
assertFalse(grid.gemsAroundOfSameType(gem1, -1, 0));
}


public void testGemAtRightBorder()
{
grid.insertGem(6, 7, gem1);
assertFalse(grid.gemsAroundOfSameType(gem1, 1, 0));
}


public void testGemAtUpperBorder()
{
grid.insertGem(0, 3, gem1);
assertFalse(grid.gemsAroundOfSameType(gem1, 0, -1));
}


public void testGemAtLowerBorder()
{
grid.insertGem(13, 3, gem1);
assertFalse(grid.gemsAroundOfSameType(gem1, 0, 1));
}

questi test, come anche quelli che hai fatto tu, non richiedevano la modifica di nessuna riga di codice, e questo non va bene perché significa che il codice che serviva a soddisfarli era stato già scritto; in altre parole (come diceva ieri fek) abbiamo scritto troppo codice; la prossima volta (che sarà domani visto che io sono arrivato tardi a causa dell'università e Daniele si è dovuto disconnettere) dobbiamo ricordarci di implementare solamente il minimo indispensabile a soddisfare il test di turno.

[DanieleC88]

questi test, come anche quelli che hai fatto tu, non richiedevano la modifica di nessuna riga di codice, e questo non va bene perché significa che il codice che serviva a soddisfarli era stato già scritto; in altre parole (come diceva ieri fek) abbiamo scritto troppo codice; la prossima volta (che sarà domani visto che io sono arrivato tardi a causa dell'università e Daniele si è dovuto disconnettere) dobbiamo ricordarci di implementare solamente il minimo indispensabile a soddisfare il test di turno.
Eh, lo so. Il problema sono i nostri orari, per due volte consecutive siamo andati di fretta, e ciò non va bene. Spero che oggi tornerai prima... :(

[fek]

Ragazzi, siamo notevolmente in ritardo con questo task.

[Ufo13]

Non so se e` permesso farlo ma se uno di voi e` troppo occupato posso provare a sostituirlo... La storia 2 e` rimasta bloccata...

[DanieleC88]

Mi spiace per il ritardo che stiamo causando, abbiamo avuto problemi per l'incompatibilità con gli orari e ieri non abbiamo potuto fare proprio niente. Comunque oggi io sono totalmente libero e so che anche Alberto tornerà prima dalle lezioni, quindi potremo dedicarci completamente a questo task, e credo proprio che lo finiremo.

[71104]

dunque, a quanto pare rimane da seguire solo l'ultimo punto della test list; è stato arduo ma alla fine io e Daniele siamo riusciti a trovare il modo di fare il drop manuale di più gemme in Grid:

public void testDropAndBrighten()
{
grid.insertGem(13, 3, gem1);
grid.update();

grid.removeGemFromGrid(grid.getGemUnderControl());

grid.insertGem(13, 4, gem2);
grid.update();
assertTrue(gem2.isBrightened());
}

in questo test io voglio che al drop di una certa gemma venga verificato se questa si trovi o meno accanto a una del suo stesso tipo; se la verifica risulta positiva, voglio che questa si illumini.

a te Daniele, e ricorda che stavolta dobbiamo scrivere solamente il minimo codice necessario a soddisfare il test ;)

[DanieleC88]

Be', il minimo indispensabile è:
getGemUnderControl().setBrightenedStatus(true);
All'interno di Grid.update. ;)

A te la palla.

[71104]

LOL ma che spiritoso :rotfl:
qui passiamo da un eccesso all'altro :D
e va bene, hai ragione tu d'altronde: se quello è il minimo indispensabile vuol dire che il test che avevo fatto non era sufficiente a documentare gli obiettivi richiesti dal task, perciò adesso a fregarti ci penso io:

public void testDropAndDontBrighten()
{
grid.insertGem(13, 3, gem1);
grid.update();

grid.removeGemFromGrid(grid.getGemUnderControl());

grid.insertGem(13, 4, gem3);
grid.update();
assertFalse(gem3.isBrightened());
}

mo non scappi :D

[DanieleC88]

Argh, colpo basso... :D
E va bene, ora ci tocca testare davvero che nelle vicinanze ci siano gemme dello stesso tipo:
private boolean gemTestForLightening(Gem gem)
{
boolean down = gemsAroundOfSameType(gem, 0, +1);
boolean left = gemsAroundOfSameType(gem, -1, 0);
boolean right = gemsAroundOfSameType(gem,+1, 0);

return (down || left || right);
}
E in Grid.update:
if(gemTestForLightening(getGemUnderControl()))
{
getGemUnderControl().setBrightenedStatus(true);
}

Bene bene bene... ora mi tocca scrivere qualche test... vediamo...

[71104]

Bene bene bene... ora mi tocca scrivere qualche test... vediamo... già già, infatti i test passano tutti e il gioco è molto carino, ma avviandolo non possiamo fare a meno di notare che quando una gemma cade accanto a una del suo stesso tipo, si deve illuminare anche l'altra!! :D

[DanieleC88]

Altro colpo basso! Ma così non vale!...
E va bene, l'hai voluta tu... beccati questa! :D
public void testMultipleBrightening()
{
grid.insertGem(13, 3, gem1);
grid.update();
grid.removeGemFromGrid(grid.getGemUnderControl());

grid.insertGem(13, 4, gem2);
grid.update();
grid.removeGemFromGrid(grid.getGemUnderControl());

assertTrue(gem1.isBrightened() && gem2.isBrightened());
}

[71104]

ahiahi ^^
ok, **IMMANE SFORZO DI MENINGI**
ce l'ho fatta!! :)
ecco qua il codice che lo soddisfa:

private boolean gemTestForLightening(Gem gem)
{
int column = gem.getCellColumn();
int row = gem.getCellRow();

boolean down = gemsAroundOfSameType(gem, 0, +1);
boolean left = gemsAroundOfSameType(gem, -1, 0);
boolean right = gemsAroundOfSameType(gem, +1, 0);

if (down)
{
getGemAt(row + 1, column).setBrightenedStatus(true);
}

if (left)
{
getGemAt(row, column - 1).setBrightenedStatus(true);
}

if (right)
{
getGemAt(row, column + 1).setBrightenedStatus(true);
}

return down || left || right;
}

[Jocchan]

Il task è completo?

[71104]

Il task è completo? a meno di refactoring direi proprio di si: la richiesta del task è soddisfatta. però a me rimane un dubbio: ma questo gioco come funziona? se più gemme dello stesso tipo stanno accanto non si doveva formare la power gem? perché le abbiamo fatte illuminare? :mbe:

[Jocchan]

a meno di refactoring direi proprio di si: la richiesta del task è soddisfatta. però a me rimane un dubbio: ma questo gioco come funziona? se più gemme dello stesso tipo stanno accanto non si doveva formare la power gem? perché le abbiamo fatte illuminare? :mbe:

Perchè intanto dobbiamo avere la conferma del fatto che le gemme riconoscano se, accanto a loro, ce ne sono altre di uguale colore.
Le maxigemme si formeranno quando queste saranno disposte in modo da formare quadrati o rettangoli, ma per esempio in una situazione di questo tipo:

XXXXO
XXOOO
XXOXO

Le gemme che ho segnato con "O", pur non formando maxigemme, sono a contatto tra di loro e, se una di queste venisse a contatto con un baule, dovrebbero ugualmente sparire tutte.

L'illuminazione, insomma, per ora serve solo a noi per assicurarci che sia tutto a posto. In seguito, magari la useremo appena prima della fusione in maxigemma (prima si illuminano e poi si fondono), ma per decidere i dettagli c'è ancora tempo ;)

[71104]

L'illuminazione, insomma, per ora serve solo a noi per assicurarci che sia tutto a posto. In seguito, magari la useremo appena prima della fusione in maxigemma (prima si illuminano e poi si fondono), ma per decidere i dettagli c'è ancora tempo ;) be', allora io direi di usare l'illuminazione quando le gemme sono attaccate ma non formano una power gem :)

[cover]

public void testDropAndBrighten()
{
grid.insertGem(13, 3, gem1);
grid.update();

grid.removeGemFromGrid(grid.getGemUnderControl());

grid.insertGem(13, 4, gem2);
grid.update();
assertTrue(gem2.isBrightened());
}



Uhm...non ho capito una cosa :stordita:
Utilizzando removeGemFromGrid, non rimuovi completamente la gemma(se non sbaglio si vuole far restare la gemma in griglia e non toglierla completamente) ?

public void removeGemFromGrid(Gem gem)

{

grid[gem.getCellRow()][gem.getCellColumn()] = null;



bagOfGems.removeGem(gem);

}


Con il risultato che gem1 viene rimossa completamente dalla sua posizione con:


grid.insertGem(13, 3, gem1);
grid.update();

grid.removeGemFromGrid(grid.getGemUnderControl());

e quindi non avendo fisicamente la gem1 vicino a gem2 poichè rimossa?
Con la conseguente che vicino a gem2 in realtà non c'è nessun'altra gemma...

Spero di non aver detto una ca***ta :Prrr:

[cover]

be', allora io direi di usare l'illuminazione quando le gemme sono attaccate ma non formano una power gem :)

In questo caso imho bisogna contare anche una cosa, se non sbaglio una volta che si è formata una power gem questa non si stacca se sotto non c'è spazio disponibile per tutta la "gemmona", quindi in teoria potrebbe capitare che sotto la gemma si sia "liberato" lo spazio di una sola gemma e l'utente và a posizionare una gemma dello stesso tipo della power gem proprio sotto ad essa, in quel caso si deve illuminare?
Spero si sia capito... :stordita:

[fek]

a meno di refactoring direi proprio di si: la richiesta del task è soddisfatta. però a me rimane un dubbio: ma questo gioco come funziona? se più gemme dello stesso tipo stanno accanto non si doveva formare la power gem? perché le abbiamo fatte illuminare? :mbe:

Bene. Bravi :)

[DanieleC88]

Spero di non aver detto una ca***ta :Prrr:
Non è proprio una cazzata, ma hai dimenticato che quando la gemma tocca il fondo (in Grid.update), nella griglia viene inserita una nuova gemma, che va per i ca**i suoi e che non è la gem2 che noi dovevamo inserire. Quindi, prima id inserire gem2, abbiamo dovuto eliminare la nuova gemma creata. ;)

[cover]

Vero... :doh: :D