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

[71104]

fek, quando vuoi sono pronto
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!!

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, 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

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]

Originariamente inviato da 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.

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

[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

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

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

[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
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

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]

Originariamente inviato da 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.

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

    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

    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);
    }