[CICLO 3] Test Driven Development Task 3.1.2 (cisc vs fek)

[fek]

La formulazione del task e' un po' imprecisa, usiamo la seguente formulazione:

Aggiungere a Sprite la possibilita' di specificare un area, le cui dimensioni possano non essere una potenza di due, all'interno di una Texture con dimensioni potenza di due.

cisc, a te per il test che traduca in codice questo requisito.

[cisc]

Con il primo test, fondamentalmente aggiungiamo solo la possibilità di specificare delle dimensioni aggiuntive non potenza di 2, partendo col presupposto che l'hotspot rimanga comunque in alto a sinistra, quindi il primo test, che ovviamente non compila:

public class TestFreeDimensionsSprite extends TestCase
{


    protected void testCreateFreeDimensionsSprite()
    {
        Texture texture = new Texture("diamond");
        Sprite sprite = new Sprite(100, 200, 12, 12,  texture, Bounds.createForTesting());
    }

}

dove i due parametri aggiunti alla posizione (ovvero 12, 12) sono appunto le dimensioni non potenza di 2

a te, fek

[fek]

Questo e' un inizio.
Ci sono alcuni problemi in questo test, manca un assert(), quindi non fallisce mai

E la prima regola del TDD e' per aggiungere funzionalita', partire sempre da un test che fallisce.

Poi c'e' un altro problema piu' piccolo, che deriva dalla scarsa precisione dei requisiti. In realta' vogliamo non solo specificare la dimensione dell'area all'interno della texture, ma anche il pixel di partenza. Praticamente vogliamo specificare un rettangolo all'interno della texture, mediante due coordinate.

Cosi':

    protected void testCreateSpriteFromTextureArea()
    {
        Texture texture = new Texture("diamond");
        
        Sprite sprite = new Sprite(
            100, 
            200,
            0,          // texcoord0.x
            0,          // texcoord0.y
            12,         // texcoord1.x
            12,         // texcoord1.y
            texture, 
            Bounds.createForTesting());
        
        assertEquals(12, sprite.getWidth());
        assertEquals(12, sprite.getHeight());
    }

Questo test fallisce. Ora prova a farlo passare. A te.

[cisc]

allora, prima di tutto ho aggiunto un costruttore che mi facesse compilare i test:

public Sprite(float posX, float posY, float textcoordXLow, float textcoordYLow, float textcoordXHigh, float textcoordYHigh, Texture texture, Bounds bounds)

    {

       this(posX, posY,  texture, bounds);

    }

ma con ciò, ovviamente il test non passa, quindi ho aggiunto le minime modifiche per far passare il test::

public Sprite(float posX, float posY, float textcoordXLow, float textcoordYLow, float textcoordXHigh, float textcoordYHigh, Texture texture, Bounds bounds)

    {

        if (texture == null)

        {

            throw new NullPointerException("texture can not be null");

        }

        

        this.texture = texture;

        this.bounds = bounds;



        vx = 1f;

        vy = 1f;



        this.textcoordXLow=textcoordXLow;

        this.textcoordYLow=textcoordYLow;

        this.textcoordXHigh=textcoordXHigh;

        this.textcoordYHigh=textcoordYHigh;

        

        width = textcoordXHigh - textcoordXLow;

        height = textcoordYHigh - textcoordYLow;



        setPos(posX, posY);      

    }

a te

[fek]

Ora dobbiamo passare queste informazioni all'engine per permettergli di disegnare solo l'area dello sprite che ci interessa.

Ci serve un test che ci dica che stiamo passando le informazioni giuste, uso qui un Mock Object dell'oggetto Engine, che eredita dall'interfaccia AbstractEngine (poi ti faccio vedere un modo piu' semplice di creare i mock).

L'idea e' di creare un oggetto che "faccia finta" di essere un'engine e ci serva solo per testare che lo Sprite interagisca correttamente con il vero oggetto Engine.

Ecco il test:

    public void testSpriteDrawWithArea()
    {
        Texture texture = new Texture("diamond");
        
        Sprite sprite = new Sprite(
            100, 
            200,
            0,          // texcoord0.x
            0,          // texcoord0.y
            12,         // texcoord1.x
            12,         // texcoord1.y
            texture, 
            Bounds.createForTesting());

        MockEngine engine = new MockEngine();
        
        sprite.draw(engine);
        
        assertEquals(12, engine.getTexcoordXHigh());
        assertEquals(12, engine.getTexcoordYHigh());
    }

Qui c'e' una parte della classe MockEngine:

public class MockEngine implements AbstractEngine
{
    // ...

    public void drawQuad(float posx, float posy, float width, float height, Texture texture)
    {
    }

    public float getTexcoordXHigh()
    {
        return 0;
    }
    
    public float getTexcoordYHigh()
    {
        return 0;
    }
}

Scrivi il codice minimale per far passare questo test. Ti do' un suggerimento: basta cambiare due sole righe di codice.

Vado a tagliare le cipolle per il ragu' e torno

[cisc]

ho aggiunto due variabili a MockEngine che rappresentano le ultime "High coordinate" visualizzate tramite drawQuad:

    private float highXLastBound;
    private float highYLastBound;
 
    
    public void drawQuad(float posX, float posY, float width, float height,
        Texture texture)
    {   
        highXLastBound = width;
        highYLastBound = height;
        ++numberOfQuadsDrawn;
    }

    .
    .
    .
    .
    .
    public float getTexcoordXHigh()
    {
        return highXLastBound;
    }
    
    public float getTexcoordYHigh()
    {
        return highYLastBound;
    }

quindi ho aggiunto il codice necessario a Sprite per far passare il test:

     public void draw(AbstractEngine engine)
    {
        engine.drawQuad(x + textcoordXLow, y + textcoordYLow, x + textcoordXHigh, y + textcoordYHigh, texture);
    }

se hai finito con le cipolle, possiamo andare avanti..

[cisc]

aa, ho modificato il test:

    public void testSpriteDrawWithArea()
    {
        Texture texture = new Texture("diamond");
        
        Sprite sprite = new Sprite(
            100, 
            200,
            0,          // texcoord0.x
            0,          // texcoord0.y
            12,         // texcoord1.x
            12,         // texcoord1.y
            texture, 
            Bounds.createForTesting());

        MockEngine engine = new MockEngine();
        
        sprite.draw(engine);
        
        assertEquals(112.0f, engine.getTexcoordXHigh());
        assertEquals(212.0f, engine.getTexcoordYHigh());
    }

in particolare ho modificato gli assert, a te fek

[fek]

Originariamente inviato da cisc

ho aggiunto due variabili a MockEngine che rappresentano le ultime "High coordinate" visualizzate tramite drawQuad:

Non era il codice piu' semplice possibile questo

La versione piu' semplice per far passare il test sarebbe stata questa:

    public float getTexcoordXHigh()
    {
        return 12;
    }
    
    public float getTexcoordYHigh()
    {
        return 12;
    }

Quando si inizia a fare TDD e' importante fare i passi piu' piccoli possibili, teoricamente fino a cambiare una riga di codice per volta, prima di lanciare i test.

La tua versione va comunque bene, e' il passo successivo. Ricordiamoci che scriviamo solo il codice strettamente necessario a far passare i test e aggiungiamo funzionalita' solo in presenza di un test che fallisce.

Ora abbiamo un po' di refactoring da fare, guarda i due test che abbiamo scritto, ci sono parecchie duplicazioni da eliminare, magari usando il metodo setup() come abbiamo fatto in altri test.

Test fallito, codice, test passato, refactoring, nuovo test fallito....

Dopo il refactoring, passiamo al test successivo. A te che mi bolle il ragu'

[cisc]

ho fatto il refactoring dei test e della classe Sprite, eliminando le variabili di istanza width e height, facendo ciò ho aggiunto altri due test:

public class TestFreeDimensionsSprite extends TestCase
{
    private Texture texture;

    private Sprite sprite;


    public void setUp()
    {
        texture = new Texture("diamond");
        sprite = new Sprite(100, 200, 0, // texcoord0.x
            0, // texcoord0.y
            12, // texcoord1.x
            12, // texcoord1.y
            texture, Bounds.createForTesting());
    }


    public void testCreateSpriteFromTextureArea()
    {
        assertEquals(12.0f, sprite.getWidth());
        assertEquals(12.0f, sprite.getHeight());
    }


    public void testSpriteDrawWithArea()
    {

        MockEngine engine = new MockEngine();

        sprite.draw(engine);

        assertEquals(112.0f, engine.getTexcoordXHigh());
        assertEquals(212.0f, engine.getTexcoordYHigh());
    }
    
    public void testSetSize()
    {
        sprite.setSize(14, 14);
        MockEngine engine = new MockEngine();
        sprite.draw(engine);

        assertEquals(114.0f, engine.getTexcoordXHigh());
        assertEquals(214.0f, engine.getTexcoordYHigh());
        
    }
    
    public void testSetSize2()
    {
        sprite.setSize(2, 2, 14, 14);
        assertEquals(12.0f, sprite.getWidth());
        assertEquals(12.0f, sprite.getHeight());
        
    }
}

delle modifiche in Sprite, quella più significativa è questa:

    public void setSize(float xHigh, float yHigh, float xLow, float yLow)

    {

        this.textcoordXHigh = xHigh;

        this.textcoordYHigh = yHigh;

        this.textcoordXLow = xLow;

        this.textcoordYHigh = yLow;

    }

    

    public void setSize(float width, float height)

    {

        setSize(this.textcoordXLow,this.textcoordYHigh, width, height );

    }

[fek]

Faccio il pignolo, cambia il nome di questo test:

    public void testSetSize2()
    {
        sprite.setSize(2, 2, 14, 14);
        assertEquals(12.0f, sprite.getWidth());
        assertEquals(12.0f, sprite.getHeight());
        
    }

99 su 100, quando si mette un numero al nome di un metodo o di una variabile vuol dire che non si ha chiaro il suo compito. Meglio fermarsi un attimo e trovare un nome migliore.

Il prossimo pasto non puo' essere testato, perche' si tratta di usare i valori passati a Engine.drawQuad() per impostare le coordinate uv all'interno della texture.

Lo faccio io questo

Tu nel frattempo pensa a qualche altro test da aggiungere per controllare eventuali condizioni d'errore. (Ce n'e' almeno una abbastanza ovvia)

[fek]

Ci sono stati alcuni problemi dovuti principalmente alla poca precisione delle specifiche: si e' fatta confusione fra la dimensione dello sprite su schermo e l'area occupata dallo sprite sulla texture, che sono due concetti differenti, mentre il codice li stava trattando come lo stesso concetto.

Risultato: passavano i test e non funzionava piu' il disegno sullo schermo

Ora scrivo qualche test per aggiustare il codice e fissare il problema.

[fek]

Ecco il test che mi dice che la dimensione dello sprite e' diversa (e puo' essere impostata indipendentemente) dall'area occupata dallo sprite nella texture:

    public void testSizeIsDifferentFromTextureArea()
    {
        sprite.setSize(14, 14);
        
        MockEngine engine = new MockEngine();
        sprite.draw(engine);

        assertEquals(12.0f, engine.getTexcoordXHigh());
        assertEquals(12.0f, engine.getTexcoordYHigh());        
    }

L'implementazione di Sprite riflette questo test e lo fa passare.

Infine ecco il metodo a basso livello in Engine che passa i dati a OpenGL (e che non puo' essere testato):

        float u0 = texcoord0X / texture.getWidth();
        float v0 = texcoord0Y / texture.getHeight();
        float u1 = texcoord1X / texture.getWidth();
        float v1 = texcoord1Y / texture.getHeight();

        // Front
        glTexCoord2f(u1, u0);
        glVertex3f(width, 0, 1);
        glTexCoord2f(u1, v1);
        glVertex3f(width, height, 1);
        glTexCoord2f(u0, v1);
        glVertex3f(0, height, 1);

        glTexCoord2f(u1, v0);
        glVertex3f(width, 0, 1);
        glTexCoord2f(u0, v1);
        glVertex3f(0, height, 1);
        glTexCoord2f(u0, v0);
        glVertex3f(0, 0, 1);

Non il massimo dell'efficienza ma fa il suo sporco lavoro. Se mai un profiler ci indichera' questo codice come un collo di bottiglia, interverremo per ottimizzare questo codice.

Ora i test caratteristici del task passano, il task e' concluso... non fosse che... non piace a checkstyle

C'e' un metodo con ben nove parametri:

    public void drawQuad(float posx, float posy, float width, float height,
        float texcoord0X, float texcoord0Y, float texcoord1X, float texcoord1Y, Texture texture)

C'e' da fare un po' di refactoring qui, e poi bisogna aggiungere qualche test per controllare le condizioni d'errore, giusto per dormire tranquilli.

Un passo per volta, direi che invece di passare nove parametri al metodo, se ne possono raccogliere alcuni in una classe, ad esempio una classe di nome Rectangle che rappresenta le coordinate di un rettangolo (su schermo o su una texture).

Naturalmente ci serve prima di tutto un test per la nuova classe, eccolo:

public class TestRectangle extends TestCase
{
    public void testRectangleWidth()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
     
        assertEquals(10, rectangle.getWidth());
     }
}

cisc, a te, il codice piu' semplice che fa passare il test, e questa volta scrivi davvero il piu' semplice

[cisc]

fatto, ho creato una nuova classe Rectangle che fa il minimo (ovvero passare solo il test):

public class Rectangle
{
    public Rectangle (float xLow, float yLow, float xHigh, float yHigh )
    {

    }
    
    public float getWidth()
    {
        return 10;
    }
}

a te

[fek]

Bene!

Risolviamo il problema per "Triangolazione" ora, ecco qualche altra assert:

    public void testRectangleExtents()
    {
        Rectangle rectangle1 = new Rectangle(10, 10, 20, 25);
     
        assertEquals(10.0f, rectangle.getWidth());
        assertEquals(15.0f, rectangle.getHeight());
        
        Rectangle anotherRectangle = new Rectangle(10, 10, 25, 50);
        
        assertEquals(15.0f, anotherRectangle.getWidth());        
        assertEquals(40.0f, anotherRectangle.getHeight());        
     }

[fek]

Mentre tu fai passare il test faccio una piccola divagazione.

Avete mai letto codice complesso, prolisso e illeggibile (magari volutamente)? Soluzioni piu' complicate del necessario? Avete mai pensato "che codice complicato, non ci capisco nulla, dev'essere proprio bravo chi lo ha scritto".

Non lo e', e' un principiante.

Il codice deve essere semplice e leggibile e conciso, per questo insisto cosi' tanto sullo scrivere il codice minimo e piu' semplice possibile per far passare i test.

Per non incorrere nella sindrome del "voglio fare il programmatore figo invece scrivo solo fesserie", a volte faccio leggere il mio codice alla mia ragazza e le chiedo di dirmi che cosa il codice sta facendo, soprattutto se il problema da risolvere e' complesso di per se'. Se non lo capisce, lo riscrivo.

Il codice deve esprimere solo e non piu' dei concetti necessari alla soluzione del problema.

Chi non segue questa regola e' un principiante, perche' ignora il fatto che il codice e' scritto soprattutto per altri e non solo per se' stessi, che dev'essere mantenuto, corretto, modificato, spesso e volentieri da qualcun altro. E mantenere codice costa, quindi non vogliamo spendere piu' soldi e tempo del necessario.

L'ho buttata giu' pesante perche' credo che questa sia la regola piu' importante che impareremo durante questo progetto.

Un paio di citazioni che mi piacciono sempre tanto:

"Every fool can write code that a machine a can understand, only good programmers can write code" - Martin Fowler
"Make it as simple as possible, but not simpler" - Albert Einstein

[cisc]

fatto:

private float xLow;
    private float yLow;
    private float xHigh;
    private float yHigh;
    
    public Rectangle (float xLow, float yLow, float xHigh, float yHigh )
    {
        this.xLow = xLow;
        this.yLow = yLow;
        this.xHigh = xHigh;
        this.yHigh = yHigh;
    }
    
    public float getWidth()
    {
        return xHigh - xLow;
    }
    
    public float getHeight()
    {
        return yHigh - yLow;
    }

test per la "gestione delle dimensioni" del rettangolo:

public void testRectangleLeft()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertEquals(10.0f, rectangle.left());
    }
    
    public void testRectangleRight()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertEquals(20.0f, rectangle.right());
    }
    
    public void testRectangleBottom()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertEquals(25.0f, rectangle.bottom());
    }
    
    public void testRectangleTop()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertEquals(10.0f, rectangle.top());
    }
    
    public void testTopBottom()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertTrue( rectangle.top()<rectangle.bottom());
    }
    
    public void testLeftRight()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertTrue( rectangle.left()<rectangle.right());
    }

a te

[fek]

Bene!

Un piccolo appunto su questi due test:

  public void testRectangleTop()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertEquals(10.0f, rectangle.top());
    }
    
    public void testTopBottom()
    {
        Rectangle rectangle = new Rectangle(10, 10, 20, 25);
        assertTrue( rectangle.top()<rectangle.bottom());
    }

Sostanzialmente stanno testando qualcosa che e' gia' testato dagl'altri test. Questo perche' top() e bottom(), ad esempio, sono stati gia' testati, questo test in piu' testa il fatto che 10 sia minore di 25, che e' ovvio

Quindi questi due test sono una duplicazione e vanno eliminati.

Ecco due test piu' interessanti che richiedono il lancio di un'eccezione se i left e' maggiore di right o bottom e' maggiore di left (coordinate verticali invertite).

    public void testInvalidHorizontalArgument()
    {
        try
        {
            Rectangle rectangle = new Rectangle(20, 10, 10, 25);
        }
        catch (Exception e)
        {
            return;
        }
        
        fail("Invalid argument exception not thrown");
    }
    
    public void testInvalidVerticalArgument()
    {
        try
        {
            Rectangle rectangle = new Rectangle(10, 20, 20, 15);
        }
        catch (Exception e)
        {
            return;
        }
        
        fail("Invalid argument exception not thrown");
    }

A te...

[cisc]

fatto, adesso che ne dici di far passare questo test, o pensi ci sia bisogno di qualche altre funzionalità a Rectangle?

public class TestFreeDimensionsSprite extends TestCase
{
    private Texture texture;

    private Sprite sprite;

    private Rectangle rectangle;


    public void setUp()
    {
        rectangle = new Rectangle ( 0, 0, 12, 12);
        texture = new Texture("diamond");
        sprite = new Sprite(100, 200, rectangle, texture, Bounds.createForTesting());
    }
    .
    .
    .
    .
}

[fek]

Certo, fai pure. Io nel frattempo uso la classe Rectangle per semplificare Engine.drawQuad().

[cisc]

ci sarebbe questo test da far passare, ci pensi tu fek?

public class TestTextureTwoMultiple extends TestCase

{

    public void testLoadNotTwoMultipleTexture()

    {



        try

        {

            Texture texture = new Texture("textureTest");

        }

        catch(TextureNotTwoMultipleException t)

        {

        }

        fail();



    }

}

per drawQuad e l'uso di Rectangle in Sprite me ne occupo io