Eliminazione actions e getType

[71104]

PREMESSA

il refactoring che ci apprestiamo a fare mira all'eliminazione di AbstractDroppable.getType nonché di alcuni altri metodi "getType" (cioè che effettuano dei test di runtime sul tipo di un dato oggetto) presenti in AbstractDroppableType e derivate.

poiché le actions sembrano utilizzare parecchio i metodi di cui sopra, e poiché esse non hanno più motivo di esistere in sostituzione di metodi diretti per modificare Grid (il codice di logging è stato eliminato), questo refactoring comporta anche l'eliminazione del design delle actions.

i task che cerco di riassumere nel seguito sono soggetti a possibile modifiche man mano che la situazione si evolve.


TASK LIST

1) eliminazione di AbstractDroppableType - spostare tutti i metodi di AbstractDroppableType (isXxx) in AbstractDroppable o derivate. eliminare metodo getType.

2) refactoring interfacce - controllare se i metodi spostati necessitano di essere utilizzati solamente da chi conosce gli oggetti sotto forma di interfacce specifiche (FallingObject, GridObject, ecc.), e non tramite tutta la AbstractDroppable; spostare eventualmente i metodi nelle rispettive interfacce.

3) eliminazione delle actions - ora, per ciascuna action:

3a) smistamento codice - il codice della action deve essere spostato in uno o più metodi nel chiamante della stessa: i test per quella action vanno anch'essi spostati nel test case corretto e refattorizzati di conseguenza.

3b) rimozione della action - rimuovere la action appena sostituita; i test ad essa relativi dovrebbero essere già scomparsi poiché dovrebbero essere stati spostati tutti in altri test case.

4) (ancora non è finita ) metodi polimorfici - tra i metodi spostati al punto 1, tutti quelli che cominciamo con "is" effettuano un test a runtime del tipo di gemma: devono essere eliminati ed il tutto refattorizzato di conseguenza, e non devono essere sostituiti da metodi canXxx perché sarebbe una cosa molto simile. Introdurre appositi metodi polimorfici con implementazioni specifiche per ogni tipo di droppable: vedi esempio di Jappilas, metodo crush in Abstract(Single)Droppable che nell'implementazione di default non fa nulla, mentre nell'implementazione della classe Chest effettua un crush.


NOTE AGGIUNTIVE

non iniziate ancora che non sono molto sicuro di questi passaggi, vorrei prima il parere di fek. a parer mio una volta svolto questo mega-refactoring Diamond Crush sarà molto più pulito, oserei dire al 70-80%

[thebol]

non ho la code base sotto mano, ma quante sono le action?

Non si rischia di far ingrassare troppo grid?(possibile fosse una motivazione per l'introduzione delle action?)

[71104]

Originariamente inviato da thebol

non ho la code base sotto mano, ma quante sono le action?

in it.diamonds.grid.action ne conto 10 e anche belle corpose (vedi sotto).

Non si rischia di far ingrassare troppo grid?

sono d'accordo, infatti nel frattempo tocca anche spolparla con refactoring di contorno. è faticoso, lo so, ma secondo me è ciò che va fatto per riportare la codebase ad uno stato di pulizia

[fek]

Ricordo che ad un certo punto Grid era diventato un mostro a dieci test: va tenuto il piu' possibile snello e pulito a colpi di Extract Class.

[Bonfo]

Prima di procedere vorerei ricordare che bisogna prima pulire i test !!! Se iniziamo a fare il resto prima rimaniamo di nuovo impantanati (http://www.hwupgrade.it/forum/showpo...2&postcount=7)!

[fek]

Originariamente inviato da Bonfo

Prima di procedere vorerei ricordare che bisogna prima pulire i test !!! Se iniziamo a fare il resto prima rimaniamo di nuovo impantanati (http://www.hwupgrade.it/forum/showpo...2&postcount=7)!

E abbiamo il volontario per il refactoring dei test relativi a Grid e AbstractDroppables.

[Bonfo]

Originariamente inviato da fek

E abbiamo il volontario per il refactoring dei test relativi a Grid e AbstractDroppables.

Volentieri!! Rispondi però alla domanda Agile (Grazie )

EDIT:
Ehm.. c'è da piangere !!!!

[jappilas]

Originariamente inviato da 71104

<...>
3a) introduzione metodo diretto - introdurre in Grid un metodo la cui esecuzione corrisponda ad una chiamata doIteration(xxxAction);

vedi altro thread - se pensi a "cosa" fa una action o meglio ancora una Crush in termini di operazioni base, vedi che è possibile che un metodo per ogni action (almeno non un metodo equivalente) in Grid, non serva

4) (ancora non è finita ) metodi polimorfici -

siccome alla fine alla Droppable è richiesto un comportamento autonomo in due situazioni , cioè in fase statica nella griglia e al termine della caduta ( questo vale sia per la nuova gempair sia per le Droppable che si muovono a seguito di una crush, potenzialmente innescandone un' altra )
quindi tutta l' intelligenza delle singole Droppable potrebbe in effetti condensarsi in crush() ( o touchDown() ) ed eventualmente update(timestamp)

[71104]

Originariamente inviato da jappilas

vedi altro thread - se pensi a "cosa" fa una action o meglio ancora una Crush in termini di operazioni base, vedi che è possibile che un metodo per ogni action (almeno non un metodo equivalente) in Grid, non serva

ho visto l'altro thread, avete ragione; provo a modificare la task list.

[71104]

il punto 3 è diventato da così:

3a) introduzione metodo diretto - introdurre in Grid un metodo la cui esecuzione corrisponda ad una chiamata doIteration(xxxAction); naturalmente il metodo non deve contenere quella chiamata, ma svolgere il codice della action in questione. il metodo deve essere sviluppato TDD nella maniera più semplice possibile (repetita iuvant ).

3b) refactoring dei chiamanti - individuare tutte le chiamate a doIteration con quella action e sostituirle con una chiamata al metodo diretto. non dovrebbero essere sviluppati nuovi test per questo passaggio, ma controllate che i test vecchi passino ancora.

3c) rimozione della action - rimuovere la action appena sostituita ed i test ad essa relativi.

a così:

3a) smistamento codice - il codice della action deve essere spostato in uno o più metodi nel chiamante della stessa: i test per quella action vanno anch'essi spostati nel test case corretto e refattorizzati di conseguenza.

3b) rimozione della action - rimuovere la action appena sostituita; i test ad essa relativi dovrebbero essere già scomparsi poiché dovrebbero essere stati spostati tutti in altri test case.

che ne pensate?

[71104]

mi sono preso la libertà di cominciare a spostare i metodi da AbstractDroppableType ad AbstractDroppable
tanto il punto 1) della task list è una cosa che va fatta sicuramente al 100%.

ho spostato il metodo toString, il quale ora è diventato un getTypeName implementato in ciascun tipo di droppable. inizialmente credevo fosse inutile el'ho eliminato, poi mi sono accorto che veniva utilizzato per costruire i nomi dei files delle textures, e quindi ho introdotto test-driven il metodo getTypeName.

[Ufo13]

Tutto cio` che coinvolge Grid, GridController, Actions e Stati della griglia e` una delle parti piu` delicate al momento.

Volevo occuparmi della separazione del codice di rendering dal codice che gestisce la logica ma temo che questo task dovra` aspettare per ora...

Ieri ho pensato a cosa si potrebbe fare per semplificare il tutto e mi sono venute un po' di idee:

- Grid deve essere soltanto una griglia che contiene gemme. Le operazioni permesse sarebbero qualcosa tipo: insertGemAt(Droppable, Cell) e getGemAt(Cell).

- GridController gestirebbe un po' piu` esplicitamente l'esecuzione degli stati e le transizioni tra uno stato e l'altro.

- Idealmente non ci sarebbe piu` l'idea di droppable polimorfi ma di una singola classe Droppable che puo` contenere una o piu` Strategie (tipo nello strategy pattern) a seconda dello stato. Per esempio se lo stato e` MERGING_CELLS, molti droppable non faranno nulla quando il metodo droppable.update(State currentState) viene eseguito ma le BigGem invece eseguiranno la MergingStrategy. MERGING_CELLS potrebbe benissimo essere un enumeratore cosi` da poter tenere gli stati in una mappa.


Questa e` un'idea generale che sicuramente necessita una revisione

[fek]

L'idea di uno Strategy o State dentro Droppable mi piace perche' semplifica molto la gerarchia.

Sto guardando la classe AbstractCrushAction e contiene dipendenze ovvie su Grid, Gem e usa isGem() per eseguire il suo compito. Questo e' sbagliato. Dovrebbe limitarsi a chiamare un qualche metodo crush() su un droppable, ma viste le dipendenze ha bisogno di un po' di refactoring per potere spostare la logica da questa Action al Droppable.

Questa cosa qui poi e' del tutto fuori dagli schemi:

    protected void setAddAtScore(boolean addAtScore)
    {
        this.addToScore = addAtScore;
    }

Sta cambiando il comportamento di questa classe attraverso un flag. Si fa in C, non in Java

[Ufo13]

Ah si mi sono svanito una parte. Alla fine droppable abbiamo detto che avrebbe una hashmap<State, Strategy>. A questo punto, in teoria, non dovremmo piu` avere bisogno di diverse classi per Gem, Chest etc...

Ci basterebbe avere una factory che crea gli oggetti con le strategie giuste.

[fek]

Voglio togliere quello Sprite da Droppable e creare un oggetto che sia una collezione di Sprite e relativo Droppable: qualcosa tipo RenderableDroppable o DrawableDroppable.

Ci provi tu Fede o ci provo io stasera se torno presto?

[thebol]

Originariamente inviato da Ufo13

Ah si mi sono svanito una parte. Alla fine droppable abbiamo detto che avrebbe una hashmap<State, Strategy>. A questo punto, in teoria, non dovremmo piu` avere bisogno di diverse classi per Gem, Chest etc...

Ci basterebbe avere una factory che crea gli oggetti con le strategie giuste.

non è un po la riporoposizione dell'AbstractDroppableType(magari un po più potente)?

[Ufo13]

Originariamente inviato da fek

Voglio togliere quello Sprite da Droppable e creare un oggetto che sia una collezione di Sprite e relativo Droppable: qualcosa tipo RenderableDroppable o DrawableDroppable.

Ci provi tu Fede o ci provo io stasera se torno presto?

Non sono sicuro di aver capito quello che hai in mente... La mia idea era creare un oggetto GridRenderer che prendesse tutti i Droppable da Grid e li renderizzasse.

L'informazione su come renderizzare un Droppable dovrebbe essere decoupled (scusate non mi viene un termine in italiano) il piu` possibile.

Fra possiamo vederlo insieme se ti va.

[fek]

Ok, lo guardiamo stasera assieme.