Refactoring Grid con GridController TDD (cisc vs 71104)

[cisc]

Implementare il pattern Observer in modo essenziale, introducento una classe GridController che deve attendere una notifica da parte di Grid dell'avvenuta collisione, e agire di conseguenza inserendo una nuova Gem.

test list:

- se grid non ha controllore, verificare che non venga aggiunta nessuna gemma dopo una collisione

- aggiungere un controller a grid e controllare che l'aggiunta abbia avuto successo

- verificare che dopo una collisione venga effettuta la notifica al controllore

- verificare che il controllere inserisca la nuova gemma dopo una notifica

primo test:

    public void testNoGemUnderController() 
    {
        grid.setGravity(12);
        grid.insertGem(13,2,gem);
        grid.update();
        assertTrue("No Gem must be under control", grid.getGemUnderControl()==gem);
    }

[71104]

basta commentare la chiamata a insertNewGem(); all'interno di update e il test passa
a questo punto però la build non è rotta, è fracassata!!

[cisc]

la build è fracassata perchè stiamo facendo un bel cambiamento in grid, prox test:

    public void testSetController() 
    {
        grid.setController(new GridController());
        assertTrue("Grid must be under control", grid.isControlled()) ;
    }

a te

[71104]

per farlo passare basta aggiungere:

    public boolean isControlled()
    {
        return true;
    }

e poi bisogna anche mettere tutto il necessario per farlo compilare: nuova classe GridController, relativo costruttore, e metodo setController (che non fa nulla )

committato.

[cisc]

ok, prossimo test:

    public void testNotSetController() 
    {
        assertFalse("Grid must be under control", grid.isControlled()) ;
    }

[71104]

    public void setController(GridController controller)
    {
        controlled = true;
    }

    public boolean isControlled()
    {
        return controlled;
    }

oggi sono bastardo

ma in fondo c'è da chiedersi: serve realmente che Grid memorizzi l'istanza del suo controller?? lo sapremo continuando il task

ed ecco il prossimo test:

    public void testControllerNotifiedAfterDrop()
    {
        grid.setController(controller);
        grid.insertGem(13, 4, gem);
        grid.update();
        assertTrue(controller.notified());
    }

PS: si, mi sa che era necessario

[cisc]

minima modifica, aggiunto questo metodo a GridController:

    public boolean notified()
    {
        return true;
    }

[71104]

e se io ci metto questo?

    public void testControllerNotNotified()
    {
        grid.setController(controller);
        assertFalse(controller.notified());
    }

[cisc]

ho dovuto fare un po di modifiche a grid:

private GridController controller;

public void setController(GridController controller)
    {
        this.controller = controller;
        controlled = true;
    }



//in update
.
.
            //se una gemma è droppata
            if (controller != null) 
            {
                controller.notifyDrop();
            }
.
.

e in GridController:

public class GridController
{
    private boolean notified = false;
    
    public GridController()
    {
    }

    public boolean notified()
    {
        return notified;
    }

    public void notifyDrop()
    {
        notified = true;
        
    }

}

prossimo test:

    public void testControllerInsertNewGem()
    {
        grid.setController(controller);
        grid.insertGem(13, 4, gem);
        grid.update();
        assertFalse("No Gem must be under control", grid.getGemUnderControl() == gem);
    }

[71104]

ok, un piccolo refactoring prima di continuare: il flag controlled e l'istanza del controller in grid costituiscono una duplicazione di informazione, percui questo codice

    public void setController(GridController controller)
    {
        this.controller = controller;
        controlled = true;
    }

    public boolean isControlled()
    {
        return controlled;
    }

lo trasformo in quest'altro

    public void setController(GridController controller)
    {
        this.controller = controller;
    }

    public boolean isControlled()
    {
        return null != controller;
    }

eliminando il flag controlled, e i test passano ancora
(tranne quelli che abbiamo rotto lol ^^')

[71104]

ho fatto passare il test decommentando insertNewGem e spostandola all'interno dell'if; ora però siccome è un attimino tardino, direi caro cisc di ripristinare la build e andare a nanna ^^

[cisc]

per riprisrinare la build abbiamo adottato una soluzione temporanea che in definitiva non tiene conto delle modifiche che abbiamo effettuato fino ad adesso, a domani 71104

[fek]

Ragazzi, mi dispiace interrompervi, ma per favore fate il revert di questi cambiamenti.

Grid non deve sapere nulla dell'esistenza di un oggetto che la controlla. E' un punto importante del design che voglio mantenere.

Al massimo ci deve essere un solo punto di contatto per implementare l'Observer usando un meccanismo di callback.

[71104]

Originariamente inviato da fek

Al massimo ci deve essere un solo punto di contatto per implementare l'Observer usando un meccanismo di callback.

hum, ok, np: adesso tolgo GridController e TestGridController.
ma il meccanismo di callback in Java come lo realizziamo? funzioni virtuali?

[71104]

fatto il revert.

[71104]

Originariamente inviato da 71104

ma il meccanismo di callback in Java come lo realizziamo? funzioni virtuali?

mi rispondo da solo: creaiamo un'interfaccia con un metodo astratto (l'handler appunto); Grid conosce l'interfaccia ma non conosce GridController

[fek]

Originariamente inviato da 71104

mi rispondo da solo: creaiamo un'interfaccia con un metodo astratto (l'handler appunto); Grid conosce l'interfaccia ma non conosce GridController

Ti rispondo io invece, usiamo la soluzione di cionci che e' semplice e banale

Mi piace pensarla come una dimostrazione di quanto funzioni avere piu' persone che lavorano su ampie porizioni di codice: io sono partito subito con l'Observer, perche' ero entrato in quell'ordine di idee, ed ho ignorato totalmente possibili soluzioni alternative. Il mio errore e' stato corretto da cionci che guardando il problema da un lato differente ha proposto una soluzione alternativa piu' semplice.

[71104]

io e cisc abbiamo steso la nuova test list:

- verificare che grid non inserisca da sola la nuova gemma
- verificare che GridController.update chiami Grid.update
- verificare che GridController inserisca una nuova gemma
- verificare che GridController faccia tutto questo dopo un timeout
- verificare che il controller catturi l'eccezione lanciata da Grid in caso di game over e disegni lo sprite "Game Over"

sono solo gli acceptance test, sicuramente ce ne saranno altri.
allora cisc, cominciamo?

[71104]

ecco la nuova classe TestGridController:

public class TestGridController extends TestCase
{
    private Grid grid;

    private Gem gem;

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

    public void testGridDoesntInsertNewGems()
    {
        grid.insertGem(13, 4, gem);
        grid.update();
        assertEquals(gem, grid.getGemUnderControl());
    }

}

ora, questo primo test è problematico perché è in aperto contrasto con un paio di altri test già esistenti; non c'è modo di farlo passare senza rompere gli altri, e questo perché i precedenti adesso non saranno più validi: le specifiche del programma stanno cambiando e andranno aggiustati.

[fek]

Originariamente inviato da 71104

- verificare che il controller catturi l'eccezione lanciata da Grid in caso di game over e disegni lo sprite "Game Over"

Il "Game Over" non va gestito mediante un'eccezione. Non e' un design corretto, le eccezioni vanno usate solo per situazioni eccezionali. Il Game Over e' una situazione perfettamente normale alla fine del gioco.