Programma "Refactor This!" [Da leggere] - Pagina 6

thebol · 08-02-2008, 08:03

from

Codice:

    public void testNotMoveRightOnRestartWaitState() throws IOException
    {
        // TODO: REFACTOR THIS

        Droppable slaveGem;
        Droppable pivotGem;

        fillFourthColumn();

        loop.getPlayerTwoInput().notify(Event.create(Code.RIGHT, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.DOWN, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.BUTTON1, State.PRESSED));

        environment.getTimer().advance(newGemDelay);

        slaveGem = field2.getGridController().getGemsPair().getSlave();
        int slaveRow = slaveGem.getRegion().getTopRow();
        int slaveColumn = slaveGem.getRegion().getLeftColumn();

        pivotGem = field2.getGridController().getGemsPair().getPivot();
        int pivotRow = pivotGem.getRegion().getTopRow();
        int pivotColumn = pivotGem.getRegion().getLeftColumn();

        environment.getTimer().advance(restartGameDelay - 1);
        loop.doOneStep();

        assertEquals(field2.getGridController().getGemsPair().getSlave().getRegion().getLeftColumn(), slaveColumn);
        // slaveRow +1 perche la gravitï¿½ sulla gemsPair viene testata prima del check
        // del
        // gameOver(in gridController.update)
        assertEquals(field2.getGridController().getGemsPair().getSlave().getRegion().getTopRow(), slaveRow + 1);

        assertEquals(field2.getGridController().getGemsPair().getPivot().getRegion().getLeftColumn(), pivotColumn);
        // slaveRow +1 perche la gravitï¿½ sulla gemsPair viene testata prima del check
        // del
        // gameOver(in gridController.update)
        assertEquals(field2.getGridController().getGemsPair().getPivot().getRegion().getTopRow(), pivotRow + 1);

    }

to

Codice:

    public void testGemsPairOnFieldTwoMoveWhenGameOverOccour() throws IOException
    {
        Droppable slaveGem = field2.getGridController().getGemsPair().getSlave();
        Droppable pivotGem = field2.getGridController().getGemsPair().getPivot();

        fillFourthColumn();
        
        int slaveRow = slaveGem.getRegion().getTopRow();
        int slaveColumn = slaveGem.getRegion().getLeftColumn();
        int pivotRow = pivotGem.getRegion().getTopRow();
        int pivotColumn = pivotGem.getRegion().getLeftColumn();

        environment.getTimer().advance(newGemDelay);
        loop.doOneStep();

        assertEquals(field2.getGridController().getGemsPair().getSlave().getRegion().getLeftColumn(), slaveColumn);
        assertEquals(field2.getGridController().getGemsPair().getSlave().getRegion().getTopRow(), slaveRow + 1);

        assertEquals(field2.getGridController().getGemsPair().getPivot().getRegion().getLeftColumn(), pivotColumn);
        assertEquals(field2.getGridController().getGemsPair().getPivot().getRegion().getTopRow(), pivotRow + 1);
    }
    
    
    public void testGemsPairOnFieldTwoNotMoveOnWaitRestart() throws IOException
    {
        Droppable slaveGem = field2.getGridController().getGemsPair().getSlave();
        Droppable pivotGem = field2.getGridController().getGemsPair().getPivot();

        fillFourthColumn();
        
        loop.getPlayerTwoInput().notify(Event.create(Code.RIGHT, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.DOWN, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.BUTTON1, State.PRESSED));

        environment.getTimer().advance(newGemDelay);
        loop.doOneStep();

        Region slaveRegion = cloneRegion(slaveGem.getRegion());
        Region pivotRegion = cloneRegion(pivotGem.getRegion());

        environment.getTimer().advance(restartGameDelay - 1);
        loop.doOneStep();
        
        assertEquals(slaveGem.getRegion(), slaveRegion);
        assertEquals(pivotGem.getRegion(), pivotRegion);
    }

notare la creazione in ComponentHelperForTest di cloneRegion per evitare di tirarsi fuori top e left ogni volta e confrontarle una per una..

thebol · 08-02-2008, 08:06

from

Codice:

    public void testControllerDontReactForKeyPressedOnDelay() throws IOException
    {
        // TODO: REFACTOR THIS

        int inputRate = environment.getConfig().getInteger("InputRate");

        fillFourthColumn();

        loop.getPlayerTwoInput().notify(Event.create(Code.RIGHT, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.DOWN, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.BUTTON1, State.PRESSED));

        environment.getTimer().advance(newGemDelay);
        loop.doOneStep();

        environment.getTimer().advance(restartGameDelay);
        loop.doOneStep();

        environment.getTimer().advance(inputRate);
        loop.doOneStep();

        DroppablePair newGemsPairOne = loop.getPlayFieldOne().getGridController().getGemsPair();
        DroppablePair newGemsPairTwo = loop.getPlayFieldTwo().getGridController().getGemsPair();

        assertEquals("New pivot of field One is misplaced", 4, newGemsPairOne.getPivot().getRegion().getLeftColumn());
        assertEquals("New pivot of field One is misplaced", 2, newGemsPairOne.getPivot().getRegion().getTopRow());

        assertEquals("New slave of field One is misplaced", 4, newGemsPairOne.getSlave().getRegion().getLeftColumn());
        assertEquals("New slave of field One is misplaced", 1, newGemsPairOne.getSlave().getRegion().getTopRow());

        assertEquals("New pivot of field Two is misplaced", 4, newGemsPairTwo.getPivot().getRegion().getLeftColumn());
        assertEquals("New pivot of field Two is misplaced", 2, newGemsPairTwo.getPivot().getRegion().getTopRow());

        assertEquals("New slave of field Two is misplaced", 4, newGemsPairTwo.getSlave().getRegion().getLeftColumn());
        assertEquals("New slave of field Two is misplaced", 1, newGemsPairTwo.getSlave().getRegion().getTopRow());

    }

to

Codice:

    public void testControllerDontReactForKeyPressedOnDelay() throws IOException
    {
        int inputRate = environment.getConfig().getInteger("InputRate");

        fillFourthColumn();

        loop.getPlayerTwoInput().notify(Event.create(Code.RIGHT, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.DOWN, State.PRESSED));
        loop.getPlayerTwoInput().notify(Event.create(Code.BUTTON1, State.PRESSED));

        environment.getTimer().advance(newGemDelay);
        loop.doOneStep();

        environment.getTimer().advance(restartGameDelay);
        loop.doOneStep();

        environment.getTimer().advance(inputRate);
        loop.doOneStep();

        DroppablePair newGemsPairOne = loop.getPlayFieldOne().getGridController().getGemsPair();
        DroppablePair newGemsPairTwo = loop.getPlayFieldTwo().getGridController().getGemsPair();

        assertEquals("New pivot of field One is misplaced", PIVOT_START_REGION, newGemsPairOne.getPivot().getRegion());
        assertEquals("New slave of field One is misplaced", SLAVE_START_REGION, newGemsPairOne.getSlave().getRegion());

        assertEquals("New pivot of field Two is misplaced", PIVOT_START_REGION, newGemsPairTwo.getPivot().getRegion());
        assertEquals("New slave of field Two is misplaced", SLAVE_START_REGION, newGemsPairTwo.getSlave().getRegion());
    }

qua non ho potuto fare molto...oltre che creare in ComponentHelperForTest PIVOT_START_REGION e SLAVE_START_REGION per semplificare gli assert. Quando arrivo a lavoro metto in config i punti di partenza della slave e della pivot (come da TODO) e poi committo

Ufo13 · 08-02-2008, 08:34

Si ma i test non devono assolutamente dipendere dai valori nel config..

thebol · 08-02-2008, 09:31

Quote:

Originariamente inviato da Ufo13

Si ma i test non devono assolutamente dipendere dai valori nel config..

non sono d'accordo. Si rendono i test più resistenti al cambiamento. Domani cambiamo il tempo di reactToInput e se i valori sono hardcoded tutto si rompe. Usando i valori nel config invece tutto continua a funzionare(o almeno si rompe meno roba).

Poi si potrebbe anche fare un test per testare solo che la gems pair sia in una hardcoded posizione di partenza, ma secondo me è eccessivo.

ps. i valori da config per la posizione della gems pair li ho utilizzati solo in quel test. Se poi si vuol portare l'idea anche il altri test si può fare.

pps. dove si pensa che un test dipenda fortemente dal comportamento di default del config(il test ha senso solo se la gems pair è sulla colonna 4) allora si possono usare valori hardcoded nel test

Ufo13 · 08-02-2008, 10:03

Ah ok pensavo dicessi test tipo:

int updateRate = config.getInteger("updateRate");

assert(updateRate == 300);

Se e` per rendere i test a prova di cambiamento sono d'accordissimo

thebol · 08-02-2008, 10:08

Quote:

Originariamente inviato da Ufo13

Ah ok pensavo dicessi test tipo:

int updateRate = config.getInteger("updateRate");

assert(updateRate == 300);

Se e` per rendere i test a prova di cambiamento sono d'accordissimo

ah ok, sono anche io contro quel tipo di test

VICIUS · 08-02-2008, 12:06

Due cosine a cui vorrei che deste un occhiata perché mi hanno confuso non poco.

Prima di tutto non sarebbe meglio limitare l'argomento di ScoreCalculator.closeChain() a valori tra 2 e 10? In questo momento è possibile passargli qualsiasi cosa ci passi per la mente e la funzione è persino usata per resettare il tempScore usando 0.

Negli ultimi due test di TestGemCollisionSound il valore per la gravità presente nella configurazione viene moltiplicato per 2. Togliendo questa moltiplicazione i test passano comunque, quindi mi chiedevo a cosa serve? C'è qualche ragione più profonda che ho mancato?

^TiGeRShArK^ · 08-02-2008, 21:36

sono finiti tutti i refactor this?

ho fatto l'update alla head e non ne vedo...
che posso fare di breve stasera che posso?

Bonfo · 08-02-2008, 21:42

Forse ci sono i "REFACTOR THIS" che non hai visto?

In ogni caso se vai in Windows->showView->Tasks ti trovi tutti i TODO saprsi per Diamonds

^TiGeRShArK^ · 08-02-2008, 21:56

Quote:

Originariamente inviato da Bonfo

Forse ci sono i "REFACTOR THIS" che non hai visto?

In ogni caso se vai in Windows->showView->Tasks ti trovi tutti i TODO saprsi per Diamonds

no, ho guardato proprio in tasks

lo so dove si trovano i TODO

..però non ne vedo REFACTOR THIS...

P.S. mi da 26 items in tasks per la precisione....

VICIUS · 08-02-2008, 22:08

Quote:

Originariamente inviato da ^TiGeRShArK^

no, ho guardato proprio in tasks

lo so dove si trovano i TODO

..però non ne vedo REFACTOR THIS...

P.S. mi da 26 items in tasks per la precisione....

Io ne ho 41, di cui 11 sono REFACTOR THIS.

^TiGeRShArK^ · 08-02-2008, 22:16

Quote:

Originariamente inviato da VICIUS

Io ne ho 41, di cui 11 sono REFACTOR THIS.

sto rifacendo l'update....
e mi sta riupdatando tutto ora..
non ci sto capendo + niente..

P.S. il primo update l'avevo fatto con tortoise e ora questo lo sto facendo con subclipse..

^TiGeRShArK^ · 08-02-2008, 22:20

ok ci siamo..
41 todo e 11 refactor this..
sarà stato tortoise ad impallarsi

EDIT:
scoperto il motivo

in realtà ho due copie di diamonds una si I: e una in i:\workspace\eclipse

^TiGeRShArK^ · 08-02-2008, 22:25

Prima:

Codice:

// TODO: REFACTOR THIS!!! I smell a bad test. Insanely long name and 5 assert.
    public void testRepeatitionActivatedAndHandlerExecutedOnInputReaction()
    {
        setupInput();
        input.notify(Event.create(Code.ENTER, State.PRESSED));
        reactor.reactToInput(environment.getTimer().getTime());
        assertFalse(handler.isRepeated(0));
        environment.getTimer().advance(201);
        handler.resetStatus();
        reactor.reactToInput(environment.getTimer().getTime());
        assertTrue(handler.isRepeated(0));
        assertEquals(handler.getFastRepeatDelay(), handler.getCurrentRepeatDelay());
        assertTrue(handler.executed());
        assertTrue(handler.pressed());
    }

^TiGeRShArK^ · 08-02-2008, 23:24

rettifico..
Ho dovuto sistemare TUTTO dato che era qualcosa di trascendentale.
Prima:

Codice:

    private void setupInput()
    {
        input = Input.create(environment.getKeyboard(), environment.getTimer());
        input.setEventMappings(createEventMappings());
        reactor = new InputReactor(input, 200, 100);
        reactor.addHandler(Code.ENTER, handler);
    }


    public void setUp()
    {
        environment = MockEnvironment.create();
        handler = MockEventHandler.create();
    }


    public void testNormalRepeatDelaySet()
    {
        handler.setNormalRepeatDelay(100);
        assertEquals(100, handler.getNormalRepeatDelay());
    }


    public void testFastRepeatDelaySet()
    {
        handler.setFastRepeatDelay(100);
        assertEquals(100, handler.getFastRepeatDelay());
    }


    public void testPressed()
    {
        setupInput();
        handler.handleEvent(reactor, Event.create(Code.ENTER, State.PRESSED));
        assertTrue(handler.isPressed());
    }


    public void testReleased()
    {
        setupInput();
        handler.handleEvent(reactor, Event.create(Code.ENTER, State.PRESSED));
        handler.handleEvent(reactor, Event.create(Code.ENTER, State.RELEASED));
        assertFalse(handler.isPressed());
    }


    public void testRepeatitionOnTimeDelay()
    {
        setupInput();
        input.notify(Event.create(Code.ENTER, State.PRESSED));
        reactor.reactToInput(environment.getTimer().getTime());
        assertFalse(handler.isRepeated(0)); // TODO: 3 assert? why?
        assertTrue(handler.isRepeated(201));
        assertEquals(handler.getNormalRepeatDelay(), handler.getCurrentRepeatDelay());
    }


    // TODO: REFACTOR THIS!!! I smell a bad test. Insanely long name and 5 assert.
    public void testRepeatitionActivatedAndHandlerExecutedOnInputReaction()
    {
        setupInput();
        input.notify(Event.create(Code.ENTER, State.PRESSED));
        reactor.reactToInput(environment.getTimer().getTime());
        assertFalse(handler.isRepeated(0));
        environment.getTimer().advance(201);
        handler.resetStatus();
        reactor.reactToInput(environment.getTimer().getTime());
        assertTrue(handler.isRepeated(0));
        assertEquals(handler.getFastRepeatDelay(), handler.getCurrentRepeatDelay());
        assertTrue(handler.executed());
        assertTrue(handler.pressed());
    }


    // TODO: Why all this assert?
    public void testRepeatitionDisactivatedOnEventReleased()
    {
        setupInput();
        input.notify(Event.create(Code.ENTER, State.PRESSED));
        reactor.reactToInput(environment.getTimer().getTime());
        assertFalse(handler.isRepeated(0));
        environment.getTimer().advance(201);
        reactor.reactToInput(environment.getTimer().getTime());
        assertTrue(handler.isRepeated(0));
        environment.getTimer().advance(201);
        input.notify(Event.create(Code.ENTER, State.RELEASED));
        reactor.reactToInput(environment.getTimer().getTime());
        assertFalse(handler.isRepeated(0));
    }
}

Dopo il colpo dell'uccello d'acqua di nanto:

Codice:

    public void setUp()
    {
        environment = MockEnvironment.create();
        int normalRepeatDelay = environment.getConfig().getInteger("NormalRepeatDelay");
        int fastRepeatDelay = environment.getConfig().getInteger("FastRepeatDelay");
        timer = environment.getTimer();

        handler = MockEventHandler.create();
        input = Input.create(environment.getKeyboard(), environment.getTimer());
        input.setEventMappings(createEventMappings());
        reactor = new InputReactor(input, normalRepeatDelay, fastRepeatDelay);
        reactor.addHandler(Code.ENTER, handler);
        input.notify(Event.create(Code.ENTER, State.PRESSED));
    }


    public void testNormalRepeatDelaySet()
    {
        handler.setNormalRepeatDelay(100);
        assertEquals(100, handler.getNormalRepeatDelay());
    }


    public void testFastRepeatDelaySet()
    {
        handler.setFastRepeatDelay(100);
        assertEquals(100, handler.getFastRepeatDelay());
    }


    public void testPressed()
    {
        handler.handleEvent(reactor, Event.create(Code.ENTER, State.PRESSED));
        assertTrue(handler.isPressed());
    }


    public void testReleased()
    {
        handler.handleEvent(reactor, Event.create(Code.ENTER, State.PRESSED));
        handler.handleEvent(reactor, Event.create(Code.ENTER, State.RELEASED));
        assertFalse(handler.isPressed());
    }


    public void testHandlerNotRepeated()
    {
        reactToInput();
        assertFalse(handler.isRepeated(0));
    }


    private void reactToInput()
    {
        reactor.reactToInput(timer.getTime());
    }


    public void testHandlerIsRepeatedAfterDelay()
    {
        reactToInput();
        assertTrue(handler.isRepeated(handler.getNormalRepeatDelay() + 1));
    }


    public void testRepeatDelayIsNotChanging()
    {
        reactToInput();
        assertEquals(handler.getNormalRepeatDelay(), handler.getCurrentRepeatDelay());
    }


    public void testHandlerIsRepeatedAfterTwoInputReaction()
    {
        reactTwiceToInput();
        assertTrue(handler.isRepeated(0));
    }


    public void testFastRepetitionSetAfterTwoInputReaction()
    {
        reactTwiceToInput();
        assertEquals(handler.getFastRepeatDelay(), handler.getCurrentRepeatDelay());
    }


    public void testHandlerExecutedAfterTwoInputReaction()
    {
        reactTwiceToInput();
        assertTrue(handler.executed());
    }


    public void testHandlerPressedAfterTwoInputReaction()
    {
        reactTwiceToInput();
        assertTrue(handler.pressed());
    }


    public void testRepetitionDeactivatedWhenReleased()
    {
        reactTwiceToInput();
        timer.advance(handler.getNormalRepeatDelay() + 1);
        input.notify(Event.create(Code.ENTER, State.RELEASED));
        reactToInput();
        assertFalse(handler.isRepeated(0));
    }


    private void reactTwiceToInput()
    {
        reactToInput();
        timer.advance(handler.getNormalRepeatDelay() + 1);
        reactToInput();
    }
}

ebbene si

2000 anni di scuola Hokuto ci hanno insegnato la vittoria del refactoring sul bad design e sui bug

Ufo13 · 08-02-2008, 23:28

Ottimo lavoro

ma perche il setUp e` diventato piu` incasinato?

Ufo13 · 08-02-2008, 23:53

In CanMoveDownQuery ho trovato questo codice

Codice:

    protected void applyOn(Droppable gem)
    {
        if (pair != null)
        {
            if (gem == pair.getPivot() || gem == pair.getSlave())
            {
                return;
            }
        }

        result |= gem.getMovingDownObject().canMoveDown(getGrid());
    }

Anche questo direi che e` un rimasuglio delle dynamite. Cancello l'if. Se avete qualcosa in contrario dite pure

Codice:

    protected void applyOn(Droppable gem)
    {
        result |= gem.getMovingDownObject().canMoveDown(getGrid());
    }

Rimuovo i due test che falliscono che testano appunto questo comportamento (tral'altro in maniera oscura)

^TiGeRShArK^ · 08-02-2008, 23:57

Quote:

Originariamente inviato da Ufo13

Ottimo lavoro

ma perche il setUp e` diventato piu` incasinato?

perchè ho aggiunto lì tutto quello che era ripetuto nei vari metodi

EDIT: volendo si potrebbe reintrodurre l'extract method con setupInput... fai pure se ti va

fek · 09-02-2008, 08:49

Quote:

Originariamente inviato da ^TiGeRShArK^

perchè ho aggiunto lì tutto quello che era ripetuto nei vari metodi

EDIT: volendo si potrebbe reintrodurre l'extract method con setupInput... fai pure se ti va

Si' per favore

thebol · 09-02-2008, 09:22

ho committare il refactor this del mio precedente post....

ho però aggiunto un todo a questo test

Codice:

    //TODO This test check a behavior not necessary for the game. Remove it? 
    public void testGemsPairOnFieldTwoMoveWhenGameOverOccour() throws IOException
    {
        Droppable slaveGem = field2.getGridController().getGemsPair().getSlave();
        Droppable pivotGem = field2.getGridController().getGemsPair().getPivot();

        fillFourthColumn();
        
        Region slaveNextRegion = cloneRegion(slaveGem.getRegion(), 0, 1);
        Region pivotNextRegion = cloneRegion(pivotGem.getRegion(), 0, 1);

        environment.getTimer().advance(newGemDelay);
        loop.doOneStep();

        assertEquals(field2.getGridController().getGemsPair().getSlave().getRegion(), slaveNextRegion);
        assertEquals(field2.getGridController().getGemsPair().getPivot().getRegion(), pivotNextRegion);
    }

praticamente questo test verifica un comportamento di diamonds, che però non è una specifica.

Praticamente quando capita il gameover sul field1 la pair sul field2 fà un movimento verso il basso.

Questo perchè viene fatto un check se cè gameover, poi viene fatto l'update sul field 1, viene generato il gameover, poi viene fatto l'update del field2(e la pair si muove verso il basso). Al giro successivo si troverà il gameover, e la pair non si muove più(verificato da un altro test).

Questo test è da tenere?

ps.C'è un simpatico plugin per eclipse di checkstyle. Una volta caricato il file di configurazione di checkstyle, e abilitato nel progetto di diamonds, vi ritroverete i warning in eclipse. Consiglio comunque sempre una build di ant prima di committare. C'è solo una modifica da fare dopo aver caricato il file, andare in Class design->Final class e deselezionare Hide Utility Class Constructor, che genera dei warning nel codice non segnalati dal task ant(non so se dipende da diverse versioni di checkstyle).

08-02-2008, 21:36	#108
^TiGeRShArK^ Senior Member Iscritto dal: Jul 2002 Città: Reggio Calabria -> London Messaggi: 12093	sono finiti tutti i refactor this? ho fatto l'update alla head e non ne vedo... che posso fare di breve stasera che posso? __________________

08-02-2008, 21:42	#109
Bonfo Senior Member Iscritto dal: Nov 2005 Città: Bologna Messaggi: 1303	Forse ci sono i "REFACTOR THIS" che non hai visto? In ogni caso se vai in Windows->showView->Tasks ti trovi tutti i TODO saprsi per Diamonds __________________ Software engineer Bonfo's Blog

08-02-2008, 22:20	#113
^TiGeRShArK^ Senior Member Iscritto dal: Jul 2002 Città: Reggio Calabria -> London Messaggi: 12093	ok ci siamo.. 41 todo e 11 refactor this.. sarà stato tortoise ad impallarsi EDIT: scoperto il motivo in realtà ho due copie di diamonds una si I: e una in i:\workspace\eclipse __________________ Ultima modifica di ^TiGeRShArK^ : 08-02-2008 alle 22:27.

09-02-2008, 09:22	#120
thebol Senior Member Iscritto dal: Dec 2000 Città: bologna Messaggi: 1309	ho committare il refactor this del mio precedente post.... ho però aggiunto un todo a questo test Codice: //TODO This test check a behavior not necessary for the game. Remove it? public void testGemsPairOnFieldTwoMoveWhenGameOverOccour() throws IOException { Droppable slaveGem = field2.getGridController().getGemsPair().getSlave(); Droppable pivotGem = field2.getGridController().getGemsPair().getPivot(); fillFourthColumn(); Region slaveNextRegion = cloneRegion(slaveGem.getRegion(), 0, 1); Region pivotNextRegion = cloneRegion(pivotGem.getRegion(), 0, 1); environment.getTimer().advance(newGemDelay); loop.doOneStep(); assertEquals(field2.getGridController().getGemsPair().getSlave().getRegion(), slaveNextRegion); assertEquals(field2.getGridController().getGemsPair().getPivot().getRegion(), pivotNextRegion); } praticamente questo test verifica un comportamento di diamonds, che però non è una specifica. Praticamente quando capita il gameover sul field1 la pair sul field2 fà un movimento verso il basso. Questo perchè viene fatto un check se cè gameover, poi viene fatto l'update sul field 1, viene generato il gameover, poi viene fatto l'update del field2(e la pair si muove verso il basso). Al giro successivo si troverà il gameover, e la pair non si muove più(verificato da un altro test). Questo test è da tenere? ps.C'è un simpatico plugin per eclipse di checkstyle. Una volta caricato il file di configurazione di checkstyle, e abilitato nel progetto di diamonds, vi ritroverete i warning in eclipse. Consiglio comunque sempre una build di ant prima di committare. C'è solo una modifica da fare dopo aver caricato il file, andare in Class design->Final class e deselezionare Hide Utility Class Constructor, che genera dei warning nel codice non segnalati dal task ant(non so se dipende da diverse versioni di checkstyle). Ultima modifica di thebol : 09-02-2008 alle 09:28.

08-02-2008, 08:34	#103
Ufo13 Senior Member Iscritto dal: Nov 2005 Messaggi: 1545	Si ma i test non devono assolutamente dipendere dai valori nel config..

08-02-2008, 10:03	#105
Ufo13 Senior Member Iscritto dal: Nov 2005 Messaggi: 1545	Ah ok pensavo dicessi test tipo: int updateRate = config.getInteger("updateRate"); assert(updateRate == 300); Se e` per rendere i test a prova di cambiamento sono d'accordissimo

08-02-2008, 12:06	#107
VICIUS Senior Member Iscritto dal: Oct 2001 Messaggi: 11471	Due cosine a cui vorrei che deste un occhiata perché mi hanno confuso non poco. Prima di tutto non sarebbe meglio limitare l'argomento di ScoreCalculator.closeChain() a valori tra 2 e 10? In questo momento è possibile passargli qualsiasi cosa ci passi per la mente e la funzione è persino usata per resettare il tempScore usando 0. Negli ultimi due test di TestGemCollisionSound il valore per la gravità presente nella configurazione viene moltiplicato per 2. Togliendo questa moltiplicazione i test passano comunque, quindi mi chiedevo a cosa serve? C'è qualche ragione più profonda che ho mancato?

08-02-2008, 23:28	#116
Ufo13 Senior Member Iscritto dal: Nov 2005 Messaggi: 1545	Ottimo lavoro ma perche il setUp e` diventato piu` incasinato?

08-02-2008, 23:53	#117
Ufo13 Senior Member Iscritto dal: Nov 2005 Messaggi: 1545	In CanMoveDownQuery ho trovato questo codice Codice: protected void applyOn(Droppable gem) { if (pair != null) { if (gem == pair.getPivot() \|\| gem == pair.getSlave()) { return; } } result \|= gem.getMovingDownObject().canMoveDown(getGrid()); } Anche questo direi che e` un rimasuglio delle dynamite. Cancello l'if. Se avete qualcosa in contrario dite pure Codice: protected void applyOn(Droppable gem) { result \|= gem.getMovingDownObject().canMoveDown(getGrid()); } Rimuovo i due test che falliscono che testano appunto questo comportamento (tral'altro in maniera oscura)

Strumenti
Mostra una versione stampabile Invia questa pagina per email