View Full Version : Orrore, abbiamo una GetType!!!
AbstractDroppable.getType. se ancora non mi sono rincoglionito del tutto e ricordo qualcosa di quanto mi ha insegnato fek, quella "cosa" va eliminata.
osservazione numero 1: cercando in Eclipse tutti i riferimenti a quella getType ne trova molti... :fagiano:
osservazione numero 2: ...molti dei quali sono delle cose disgustose :Puke: vi faccio un esempio: la getType viene utilizzata da un metodo toString per ottenere un oggetto "tipo" che deve a sua volta restituire il nome del tipo (tramite un metodo getName tra l'altro; toString ci faceva proprio schifo eh?) utilizzato poi per costruire la stringa. ora, come mai tutta questa perifrasi della getType? perché passare per il "tipo" per ottenere il nome, non si può semplicemente mettere già nel droppable un metodo toString?
ps: si fek, abbiamo un volontario.
jappilas
27-01-2008, 14:32
AbstractDroppable.getType. se ancora non mi sono rincoglionito del tutto e ricordo qualcosa di quanto mi ha insegnato fek, quella "cosa" va eliminata.
osservazione numero 1: cercando in Eclipse tutti i riferimenti a quella getType ne trova molti... :fagiano:
osservazione numero 2: ...molti dei quali sono delle cose disgustose :Puke: vi faccio un esempio: la getType viene utilizzata da un metodo toString per ottenere un oggetto "tipo" che deve a sua volta restituire il nome del tipo (tramite un metodo getName tra l'altro; toString ci faceva proprio schifo eh?) utilizzato poi per costruire la stringa. ora, come mai tutta questa perifrasi della getType? perché passare per il "tipo" per ottenere il nome, non si può semplicemente mettere già nel droppable un metodo toString?prendiamo come esempio questo, trovato poco fa in TestNextGemsPanel.java:
DroppableType firstGemType = firstGem.getGridObject().getType();
<...>
assertEquals("gfx/droppables/" + firstGemType.getName() + "/"
+ firstGemColor.getName(),
((MockEngine)environment.getEngine()).getImage(0).getName());in questo caso con getType().getName() si ottiene la parte variabile in funzione del tipo di droppable, del path completo corrispondente alla texture in uso per quella droppable
avendo le subdirectory nomi ("gems", "boxes", "stones", "flashing") interamente lowercase e soprattutto diversi dagli identificatori dei rispetti DroppableType, desumo che non si potesse passare per toString()...
comunque sì, la struttura attuale è troppo convoluta...
ps: si fek, abbiamo un volontario.ottimo :D
major achievements dei refactoring che ho fatto oggi: ho eliminato una catena di if, ridisposto alcuni test case in modo tale che i packages rispecchino la struttura del codice di produzione, e ho eliminato un parametro in un metodo virtuale :p
più altri minor achievements.
PS: del metodo virtuale in questione va eliminato anche l'altro parametro, ci penso domani.
PPS: preciso che di tutto ciò solo la catena di if è in-topic :D
i prossimi passi per eliminare quella getType sono di trasportare in AbstractDroppable (se non addirittura eliminare del tutto) tutti i metodi che stanno in AbstractDroppableType: non ha assolutamente senso tenersi oggetti aggiuntivi per descrivere il tipo dei droppables quando in realtà la stessa AbstractDroppable si suddivide in BigGem, Gem, Chest, ecc.
per dirla in altri termini: non dobbiamo sfruttare la tabella delle funzioni virtuali degli oggetti AbstractDroppableType, ma quella degli oggetti AbstractDroppable.
(se non addirittura eliminare del tutto) questa parentesi meritava un approfondimento.
molti di quei metodi ci tengo ad eliminarli proprio, e se ne guardate i nomi capirete il perché: isChest, isGem, isStone... sono tutte "simil-getType", cioè sono tutti metodi che testano a runtime il tipo di oggetto, e noterete anche che dove questi metodi vengono chiamati gli if non mancano di certo.
il metodo createInstance invece di per se' potrebbe anche restare, ma refattorizzando in modo tale da trasportarlo in AbstractDroppable esso si trasforma naturalmente in una semplice chiamata a un costruttore (almeno spero :mbe: ).
cdimauro
27-01-2008, 20:43
L'idea che cullo da parecchio tempo è quella di avere un'interfaccia (o classe astratta) base da cui far derivare tutti gli oggetti che stanno in una griglia, e di variare quindi soltanto l'implementazione dei metodi virtuali dei discendenti.
In sostanza il tutto si dovrebbe ridurre a un'interazione (più ad alto livello) fra oggetti che rispondono agli stessi "stimoli", e si "adeguano" di conseguenza.
Bisogna però definire per benino quest'interfaccia, e non è cosa semplice.
AbstractDroppable non corrisponde alla descrizione?
Probabilmente la struttura convoluta viene fuori dalla stratificazione diversi task di diverse storie. Per fortuna esiste il refactoring!!! :D ( Chge forse non siamo stati sufficentemente bravi ad applicare ai tempi :( )
In ogni caso, se non ricordo male, il probelma del getType era in relazione all "colore" della gemma o del chest, non tanto relativo al tipo di droppable.
In ogni caso sono d'accordo con te, più "getType" si eliminano, meglio è!
jappilas
27-01-2008, 21:16
L'idea che cullo da parecchio tempo è quella di avere un'interfaccia (o classe astratta) base da cui far derivare tutti gli oggetti che stanno in una griglia, e di variare quindi soltanto l'implementazione dei metodi virtuali dei discendenti.e, come sai, è un' idea che appoggio ;)
Bisogna però definire per benino quest'interfaccia, e non è cosa semplice.con un po' di impegno, si dovrebbe riuscire in un paio di giorni ( fek torna mercoledì ) :O
Probabilmente la struttura convoluta viene fuori dalla stratificazione diversi task di diverse storie. Per fortuna esiste il refactoring!!! :D ( Chge forse non siamo stati sufficentemente bravi ad applicare ai tempi :( )ora però si è molto più "scafati" che non ai tempi... non ci sono più scuse :D
In ogni caso, se non ricordo male, il probelma del getType era in relazione all "colore" della gemma o del chest, non tanto relativo al tipo di droppable.esatto
in quel caso si testava il colore della gemma passando dal nome della texture usata... bisogna vedere se ci sono altri casi di questo tipo ed eventualmente quali sono sono riferiti alla "logica" del gioco e quali effettivamente all' aspetto grafico (nel qual caso non servono assolutamente)
AbstractDroppable non corrisponde alla descrizione?più o meno sì, se le si accorpa AbstractSingleDroppable e si rimuove la pletora di interfaccie attuale
cdimauro
28-01-2008, 08:21
Esattamente. Vorrei vedere eliminati tutti i metodi "isChest" ecc. sostituendoli con metodi che vengano interrogati, invece, su quello che questi oggetti sono in grado di fare (allo stato attuale o in risposta a determinate sollecitazioni), come ad esempio la possibilità di essere "inglobate" (in una big gem) oppure di essere distrutte, ecc.
Per quanto riguarda il colore, se serve conoscerlo si può sempre realizzare un metodo apposito che lo restituisce (non è come restituire un tipo).
Il fronte anti interfacce continua acrescere :O
Perché non aprite un branch e fate qualche commit sperimentale? Fran potrebbe non essere d'accordo ora ma magari se vede che il codice è più pulito potreste riuscire a convincero a fare un merge.
io non sono mica anti interfacce: io sono solo contrario a qualsiasi tipo di testing a runtime del tipo di un oggetto. isChest per me se ne deve andare, assieme ovviamente a isStone, isGem, ecc.
io non sono mica anti interfacce: io sono solo contrario a qualsiasi tipo di testing a runtime del tipo di un oggetto. isChest per me se ne deve andare, assieme ovviamente a isStone, isGem, ecc.
come testi se in griglia una droppable è una stone, gem, chest, etc?
Comunque non sono convintissimo che si riescano a togliere tutti i vari isType senza complicare codice e test(a volte si può aver bisogno di comportamenti diversi per vari componenti, ma non è possibile/è complicato integrarli nel componente stesso)
Si potrebbe comunque provare come branch è vedere se ne vale la pena o no.
come testi se in griglia una droppable è una stone, gem, chest, etc? analizzando altre differenze dei droppables che ti ritrovi, ma non quella; oppure, se a fek va bene, utilizzando instanceof, cosa che io, nota bene, riterrei un obbrobbio solo nel codice di produzione, non nei test.
Comunque non sono convintissimo che si riescano a togliere tutti i vari isType senza complicare codice e test(a volte si può aver bisogno di comportamenti diversi per vari componenti, ma non è possibile/è complicato integrarli nel componente stesso) io nei miei progetti personali/universitari ci sono sempre riuscito...
analizzando altre differenze dei droppables che ti ritrovi, ma non quella; oppure, se a fek va bene, utilizzando instanceof.
Proviamo a vedere se è fattibile senza complicare di più le cose.
Se vedo una GetType nella code base, prendo l'aereo da Varna, mi paracaduto in Italia, vi spezzo a tutti quanti le ginocchia e poi riparto per l'Inghilterra.
Certo che dopo la FP vi siete dati alla pazza gioia... le pagherete tutte :|
Il fronte anti interfacce continua acrescere :O
Perché non aprite un branch e fate qualche commit sperimentale? Fran potrebbe non essere d'accordo ora ma magari se vede che il codice è più pulito potreste riuscire a convincero a fare un merge.
A me piace lavorare per interfacce, ma non e' un dogma. Se il codice che esce dal refactoring e' piu' semplice e pulito, non c'e' alcun problema a fare il merge.
io nei miei progetti personali/universitari ci sono sempre riuscito...
Ho scritto centinaia di migliaia di righe di codice di produzione senza usare nulla di simile al GetType, quindi non vedo il problema.
jappilas
28-01-2008, 12:29
analizzando altre differenze dei droppables che ti ritrovi, ma non quella; oppure, se a fek va bene, utilizzando instanceof, cosa che io, nota bene, riterrei un obbrobbio solo nel codice di produzione, non nei test.
io nei miei progetti personali/universitari ci sono sempre riuscito...vedi, c'è un problema: in diamonds quelle interfaccie hanno dei client anche nel codice di produzione, nella fattispecie l' insieme delle grid action - le quali iterano sulle Droppables presenti in griglia esaminandone a runtime il tipo per fare le loro cose...
quindi, si può rimuovere le varie getType(), isChest() e affini - a patto però di ripensare anche il resto del codice che circonda la Grid;
oppure, si può come dici tu usare instanceof, ma l' "obbrobrio" finirebbe anche nel codice di produzione e non solo nei test;
oppure, si può:
lasciare un set di "capability query", relativo a ciò che le droppable specializzate possono eventualmente fare, come metodi virtuali nella classe base astratta, lasciare il resto del codice dei test più o meno com'è ora ( a riprogettarlo c'è sempre tempo) applicando un refactoring meno gravoso
EDIT: non avevo letto i post di fek :O
fek, tanto che ci sei devo chiederti due cose fondamentali:
1) che dobbiamo fare quando vediamo in una classe metodi pubblici utilizzati solamente dai test, cerchiamo di eliminarli e di testare in altri modi?
2) possiamo usare instanceof solo nei test, e non nel codice di produzione?
cdimauro
28-01-2008, 13:31
Il fronte anti interfacce continua acrescere :O
Se fossi anti-interfacce non avrei masochisticamente :D proposto di crearne una per Environment (per far sparire le factory)... :p
Io vedo ogni costrutto sintattico come uno strumento, coi suoi pregi e difetti, e cerco di usare quello che mi sembra "migliore" per il particolare problema che mi si presenta. :)
Se vedo una GetType nella code base, prendo l'aereo da Varna, mi paracaduto in Italia, vi spezzo a tutti quanti le ginocchia e poi riparto per l'Inghilterra.
Certo che dopo la FP vi siete dati alla pazza gioia... le pagherete tutte :|
Ehm... c'erano anche prima della FP :flower:
In ogni caso pernso che l'eliminazione di isChest e altri non è con un canCrush ( che alla fine siamo lì), ma con il polimorfismo.
Ovvero tutti quanti hanno il metodo crush() che fa qualcosa e in base a che istanza sono mi comporto di conseguenza...
Il getType si elimina veramente solo con il polimorfismo!
cdimauro
28-01-2008, 20:02
E' esattamente ciò che auspico. :)
jappilas
28-01-2008, 20:26
<...>
Il getType si elimina veramente solo con il polimorfismo!su questo non ci piove
ma, come dicevo nel post precedente, non ci sono solo le Droppables con le loro interfaccie, ma anche un sistema di GridAction, che si appoggiano a quelle interfaccie - ma anche, che incapsulano la policy nonchè la conoscenza ( ad es liste di Droppable da testare, o cancellare dalla Grid) con cui gestire le Droppables le quali da oggetti passivi si limitano a essere testate e manipolate
se venisse introdotto un design polimorfico questa relazione tra Droppables e Grid Action verrebbe invertita, o sovvertita (le GridAction in effetti avrebbero molto meno senso di esistere)
se si introducesse un metodo crush() in ogni Droppable, bisognerebbe trovare un design appropriato per gestire a livello della singola Droppable (tenendo conto della "simmetria" tra Droppable) la policy e la "conoscenza" che prima era "globale" nella GridAction, e probabilmente ricontestualizzare il comportamento della Droppable stessa ( riadattare gli stati della Grid inficianti il behaviour) ...
non dico che sia intrinsecamente difficile (anzi magari viene fuori che può essere implementato con pochissimo codice semplice ;) ) ciononostante ho: il sospetto che si tratti non di un lavoro di ripulitura del codice ma proprio di un problema nuovo di design di cui dovrete tenere conto a livello sia di codice che dei test, l' impressione che la riconversione sia meno semplice e indolore dell' (eventuale) interfaccia a "queries" di cui sopra ( che comunque era intesa come un primo passo per intanto introdurre nell' attuale code base una gerarchia di classi ad interfaccia unificata, e non come "soluzione finale"...) e il timore che un nuovo "caso Dynamite" sia sempre in agguato
fek, tanto che ci sei devo chiederti due cose fondamentali:
1) che dobbiamo fare quando vediamo in una classe metodi pubblici utilizzati solamente dai test, cerchiamo di eliminarli e di testare in altri modi?
2) possiamo usare instanceof solo nei test, e non nel codice di produzione?
Dico la mia sulla (1): per me van tolti...
fek, tanto che ci sei devo chiederti due cose fondamentali:
1) che dobbiamo fare quando vediamo in una classe metodi pubblici utilizzati solamente dai test, cerchiamo di eliminarli e di testare in altri modi?
2) possiamo usare instanceof solo nei test, e non nel codice di produzione?
1) A me quei metodi pubblici non creano grossissimi problemi, ma, come sempre, se e' possibile eliminarli e semplificare l'interfaccia di una classe non ho nulla in contrario. Ad esempio isNull() nel caso di un NullAudio si puo' facilmente eliminare rifattorizzando i test relativi usando dei cast.
2) instanceof non mi piace, mi sembra sempre una scorciatoia poco elegante per non pensare al design e buttare giu' qualcosa alla spera in fek (delirio di onnipotenza :D). Se semplifica il codice e non c'e' altra soluzione posso anche soprassedere, ma mi devi giustificare la scelta molto bene e nove su dice c'e' sempre una soluzione piu' semplice.
Ehm... c'erano anche prima della FP :flower:
Non li ho visti, maledetti.
In ogni caso pernso che l'eliminazione di isChest e altri non è con un canCrush ( che alla fine siamo lì), ma con il polimorfismo.
Ovvero tutti quanti hanno il metodo crush() che fa qualcosa e in base a che istanza sono mi comporto di conseguenza...
Il getType si elimina veramente solo con il polimorfismo!
L'idea del metodo crush() mi piace, ma come dice Marco questo e' un refactoring un po' complesso e va pensato bene. Vedo che il design a query e' stato abusato e non mi piace l'implementazione corrente.
quindi, si può rimuovere le varie getType(), isChest() e affini - a patto però di ripensare anche il resto del codice che circonda la Grid;
oppure, si può come dici tu usare instanceof, ma l' "obbrobrio" finirebbe anche nel codice di produzione e non solo nei test;
La soluzione tipica qui e' creare un oggetto polimorfico. Ma non voglio creare una enorme gerarchia che poi va mantenuta. Quindi non piu' di due livelli, massimo tre in casi eccezionali.
La soluzione tipica qui e' creare un oggetto polimorfico. Ma non voglio creare una enorme gerarchia che poi va mantenuta. Quindi non piu' di due livelli, massimo tre in casi eccezionali. se da AbstractDroppable derivano tutti i tipi di gemme di livelli ne vengono solo due; se però ci teniamo anche la AbstractSingleDroppable (che deriva da AbstractDroppable, e dalla quale derivano tutte le gemme tranne BigGem) allora diventano tre. comunque questa gerarchia ce l'abbiamo già, quindi il polimorfismo non può fare che bene :flower:
il problema qui è che questa getType che abbiamo restituisce un oggetto AbstractDroppableType che rispecchia esattamente il tipo (la classe) di gemma, inteso come foglia dell'albero della gerarchia sopra descritta. if e catene di if a non finire insomma, cioè se usassimo instanceof nel codice di produzione sarebbe esattamente la stessa cosa :D :D :D (di' la verità, ti sono venuti i brividi, non credevi che la codebase fosse tanto sporca :asd: )
non c'è da ristrutturare la gerarchia, ci troviamo semplicemente nella situazione in cui:
1) i metodi polimorfici di AbstractDroppableType e derivati possono essere spostati dall'oggetto tipo all'oggetto vero e proprio, cioè la gemma (e questo refactoring è non dico semplice ma non-troppo-complesso)
2) tra questi metodi, tutti quelli che cominciamo con "is" effettuano un test a runtime del tipo di gemma (e qui viene il discorso "instanceof sarebbe la stessa cosa" :)), e quindi devono essere eliminati ed il tutto refattorizzato di conseguenza, e non devono essere sostituiti da metodi canXxx perché sarebbe qualcosa di molto simile (e questo è il refactoring difficilerrimo).
perché il refactoring del punto 2 è difficile? non perché vada ristrutturata la gerarchia delle gemme radicata in AbstractDroppable (che resta così com'è tale e quale), ma perché va ristrutturato ben altro; indovina cosa? :D
il codice per il logging delle actions :asd: :asd: :asd:
il problema è il seguente: quei metodi isXxx vengono usati ad esempio per testare se una gemma che ha appena finito di cadere è un chest o una flashing gem, insomma se può "crushare", perché in quel caso il crush va effettuato tramite una action, che è l'unico modo per modificare Grid (o almeno in teoria dovrebbe esserlo ai fini della costruzione del log, ma in realtà il log che attualmente esce fuori da una partita è fatto male perché mancano alcuni eventi a causa del fatto che non tutto il codice usa le actions, ancora ci sono dei metodi "diretti" in Grid): una volta istanziata la action questa chiama un metodo isXxx e controlla come si deve comportare a seconda del tipo di gemma.
il tutto mi porta (OT) al discorso "codice di logging": quando è stato deciso di introdurre il paradigma delle actions avevamo del codice ancora piuttosto sporco per poterlo fare, e sostituire tutti i metodi diretti non era facile (e quindi lo si è fatto solo in parte). il codice per il logging insomma è un lavoro mai terminato che per ora (IMHO) ci crea solo problemi. possiamo eliminarlo? (cosa che avevamo già fatto nella versione di SourceForge :Prrr: )
l'idea comunque non era male, solo che la reintrodurrei (se proprio necessaria) casomai molto più avanti, a codice completamente ripulito.
edit - sia chiaro, un conto è il logging delle actions e un conto sono le actions di per se'; a creare problemi sono le actions (sono le maggiori utenti dei metodi isXxx di cui sopra), ma ovviamente senza le actions scompare anche il logging.
2) instanceof non mi piace, mi sembra sempre una scorciatoia poco elegante per non pensare al design e buttare giu' qualcosa alla spera in fek (delirio di onnipotenza :D). Se semplifica il codice e non c'e' altra soluzione posso anche soprassedere, ma mi devi giustificare la scelta molto bene e nove su dice c'e' sempre una soluzione piu' semplice. il problema è questo: quando abbiamo a che fare coi famosi oggetti polimorfici (come ad esempio i numerosi tipi di gemme) e dobbiamo ad esempio testare che un certo metodo (il cui valore di ritorno nominalmente è un generico AbstractSingleDroppable) restituisca un'istanza di una gemma e non di un chest (per dire), nel test dobbiamo essere in grado di distinguere la gemma dal chest, entrambi derivati da AbstractSingleDroppable. per farlo finora avevamo il metodo ovvio di usare getType().isChest o getType().isGem; quando questi metodi scompariranno (perché si spera sia così :D) dovremo passare a delle alternative, ovvero testare sull'AbstractSingleDroppable ritornato delle caratteristiche uniche per ciascun tipo di oggetto: il punteggio per esempio, tramite AbstractSingleDroppable.getScore(), oppure il nome tramite toString(). le caratteristiche di questi oggetti però potrebbero essere tante e ciascuna di esse potrebbe non identificare univocamente l'oggetto (il punteggio, bah... e se due classi derivate hanno lo stesso punteggio?), quindi, dal momento che stiamo parlando dei test e non del codice di produzione, mi piacerebbe se fosse avallato l'uso di instanceof :D
AnonimoVeneziano
30-01-2008, 01:29
Beh, visto che hai tirato in ballo la toString() facciamo in modo che la toString() di ogni droppable ritorni il nome dell'istanza dell'oggetto, così siamo apposto :asd: :D (tanto non mi sembra sia usata per altri scopi , credo sia ancora all'implementazione di Object, e non credo possa avere scopi più nobili in futuro)
Ciao
infatti ci avevo pensato, però io a toString preferisco addirittura instanceof: mi sembra completamente univoco, "hardcoded", senza possibilità di errore.
Ad esempio isNull() nel caso di un NullAudio si puo' facilmente eliminare rifattorizzando i test relativi usando dei cast. inizialmente non capivo bene quest'ultima cosa, poi andando a zonzo per il codice ne ho incontrato un esempio e ho capito. praticamente si tratta di lasciare che sia un'eccezione a far fallire il test piuttosto che una assertTrue su instanceof.
ottimo, allora farò così :)
Sicuramente il meccanismo ad actions e gli states ( ovvero tutta la logica di gioco :asd:) sono la parte piu' "critica" dell'applicazione.
Non per nulla le action hanno la complessita' piu' alta di tuttoil resto,come testimonia cobertura.
Per i test, siccome te le crei tu a manina le istanze o di gem o di chest, basta stare attenti mentre si fa il test a non perdersi le refrenze e tutto dovrebbe andre a posto da solo ;)
Ultima cosa: asfaltiamo il logging e rifacciamolo. Sicuramente se avremo le action fatte bene, basta che le facciamo implementare il pattern command e dovremmo avre logging e replay della partita praticamente gratis.
1) A me quei metodi pubblici non creano grossissimi problemi, ma, come sempre, se e' possibile eliminarli e semplificare l'interfaccia di una classe non ho nulla in contrario. Ad esempio isNull() nel caso di un NullAudio si puo' facilmente eliminare rifattorizzando i test relativi usando dei cast.
2) instanceof non mi piace, mi sembra sempre una scorciatoia poco elegante per non pensare al design e buttare giu' qualcosa alla spera in fek (delirio di onnipotenza :D). Se semplifica il codice e non c'e' altra soluzione posso anche soprassedere, ma mi devi giustificare la scelta molto bene e nove su dice c'e' sempre una soluzione piu' semplice.
Potrei sbagliarmi visto che e` passato un sacco di tempo ma mi pare che per testare gli stati del gridcontroller (sono sempre in GC?) instanceof sarebbe stato utile perche` non esisterebbe altro modo per identificare lo stato (forse il dynamic cast).
Sicuramente il meccanismo ad actions e gli states ( ovvero tutta la logica di gioco :asd:) sono la parte piu' "critica" dell'applicazione.
Non per nulla le action hanno la complessita' piu' alta di tuttoil resto,come testimonia cobertura.
Per i test, siccome te le crei tu a manina le istanze o di gem o di chest, basta stare attenti mentre si fa il test a non perdersi le refrenze e tutto dovrebbe andre a posto da solo ;)
Ultima cosa: asfaltiamo il logging e rifacciamolo. Sicuramente se avremo le action fatte bene, basta che le facciamo implementare il pattern command e dovremmo avre logging e replay della partita praticamente gratis.
Il logging l'ho personalmente asfaltato qualche giorno fa. Se vedi altro codice relativo (dubito) cancella pure :P
(di' la verità, ti sono venuti i brividi, non credevi che la codebase fosse tanto sporca :asd: )
Mi e' venuto un sospetto quando ho letto qualcuno scrivere che il problema di Diamonds fosse la metodologia test-driven ed ho pensato "Vediamo che succede quando smettono di scrivere test-driven"... Abbiamo visto ;)
non c'è da ristrutturare la gerarchia, ci troviamo semplicemente nella situazione in cui:
1) i metodi polimorfici di AbstractDroppableType e derivati possono essere spostati dall'oggetto tipo all'oggetto vero e proprio, cioè la gemma (e questo refactoring è non dico semplice ma non-troppo-complesso)
2) tra questi metodi, tutti quelli che cominciamo con "is" effettuano un test a runtime del tipo di gemma (e qui viene il discorso "instanceof sarebbe la stessa cosa" :)), e quindi devono essere eliminati ed il tutto refattorizzato di conseguenza, e non devono essere sostituiti da metodi canXxx perché sarebbe qualcosa di molto simile (e questo è il refactoring difficilerrimo).
Il piano mi piace, abbiamo anche un volontario.
Mi apri un topic e mi scrivi una serie di task dettagliati che descrivono i passi di questi due refactoring?
Ogni passo dev'essere sufficientemente breve e conciso e relativamente semplice da implementare e testare. Diciamo che ti sto chiedendo un "task break down" del lavoro.
perché il refactoring del punto 2 è difficile? non perché vada ristrutturata la gerarchia delle gemme radicata in AbstractDroppable (che resta così com'è tale e quale), ma perché va ristrutturato ben altro; indovina cosa? :D
il codice per il logging delle actions :asd: :asd: :asd:
Il logging e' eliminato, quindi non porti il problema. Lo introduciamo di nuovo quando la code base e' piu' semplice.
il codice per il logging insomma è un lavoro mai terminato che per ora (IMHO) ci crea solo problemi. possiamo eliminarlo? (cosa che avevamo già fatto nella versione di SourceForge :Prrr: )
Togli tutto.
Beh, visto che hai tirato in ballo la toString() facciamo in modo che la toString() di ogni droppable ritorni il nome dell'istanza dell'oggetto, così siamo apposto :asd: :D (tanto non mi sembra sia usata per altri scopi , credo sia ancora all'implementazione di Object, e non credo possa avere scopi più nobili in futuro)
Ciao
Che fai bari? :)
Un problema per volta: una volta che siamo alle prese con questo codice vediamo se c'e' un modo semplice per testare queste classi senza usare instanceof.
Potrei sbagliarmi visto che e` passato un sacco di tempo ma mi pare che per testare gli stati del gridcontroller (sono sempre in GC?) instanceof sarebbe stato utile perche` non esisterebbe altro modo per identificare lo stato (forse il dynamic cast). post #32
Il logging l'ho personalmente asfaltato qualche giorno fa. Se vedi altro codice relativo (dubito) cancella pure :P le actions sono state introdotte a suo tempo per favorire il logging: dovendo essere l'unico modo per modificare Grid costituivano una "canalizzazione intercettabile" di tutti gli eventi da registrare nel log. le actions le considero strettamente legate al sistema di logging, quindi vanno eliminate assieme ad esso e sostituite con metodi diretti in Grid (che tra l'altro ancora stanno là :D) o altrimenti in GridController.
Mi e' venuto un sospetto quando ho letto qualcuno scrivere che il problema di Diamonds fosse la metodologia test-driven ed ho pensato "Vediamo che succede quando smettono di scrivere test-driven"... Abbiamo visto ;) a me sembrava di averlo detto dopo il fallimento... :mbe:
il codice su cui stiamo lavorando è antecedente, no? :|
Il piano mi piace, abbiamo anche un volontario.
Mi apri un topic e mi scrivi una serie di task dettagliati che descrivono i passi di questi due refactoring?
Ogni passo dev'essere sufficientemente breve e conciso e relativamente semplice da implementare e testare. Diciamo che ti sto chiedendo un "task break down" del lavoro. ci provo, ma i punti potrebbero essere soggetti a modifiche future: man mano che si va avanti coi refactoring bisogna vedere come si evolve la situazione per decidere cosa fare.
Il logging e' eliminato, quindi non porti il problema. Lo introduciamo di nuovo quando la code base e' piu' semplice. sempre se servirà ancora visto che il suo scopo era il debugging, e una codebase semplice funziona semplicemente meglio :)
Che fai bari? :)
Un problema per volta: una volta che siamo alle prese con questo codice vediamo se c'e' un modo semplice per testare queste classi senza usare instanceof. ormai io ho iniziato già da ieri sera come hai detto di fare tu nel post #25 al punto 1) :)
(ho già tolto i due isNull)
http://www.hwupgrade.it/forum/showthread.php?t=1666152
jappilas
30-01-2008, 18:58
post #32
le actions sono state introdotte a suo tempo per favorire il logging: ah ecco... ero convinto che servissero principalmente a gestire le Dynamites, le quali dovendo essere attivate sequenzialmente rispetto al loro ordine d' inserimento avevano portato alle action che iteravano sulle Droppable esaminandole una a una, meccanismo poi esteso a tutte gli altri eventi possibili sulla Grid ... :D
quindi vanno eliminate assieme ad esso e sostituite con metodi diretti in Grid (che tra l'altro ancora stanno là :D) o altrimenti in GridController.IMHO, una cosa del genere non ha senso o comunque non è necessaria :)
poteva averne ai tempi dell' advanced mode, quando si richiedeva alla Grid di contenere Droppable dal comportamento eterogeneo e complesso come potevano essere le bigGem rispetto alle Dynamite...
ma avendo solo gemme colorate, bauli (che cancellano le gemme attigue alla loro cella, in sezioni contigue dello stesso colore) e gemme flash (che cancellano tutte le gemme e i bauli di un colore in griglia)
e analizzando le operazioni necessarie per una singola Crush (determinare la "region" coinvolta -> rimuovere le Droppable coinvolte -> far cadere tutte le Droppable libere -> verificare se vi sia una nuova Crush -> eventualmente ricominciare ) ,
l' impressione che si ricava è che alla Grid non serva implementare un metodo per ogni action ma poche funzioni primitive (ad esempio una che, data una posizione <riga, colonna> e un "colore", ritorni una DroppableList di gemme attigue dello stesso tipo)
l' impressione che si ricava è che alla Grid non serva implementare un metodo per ogni action ma poche funzioni primitive (ad esempio una che, data una posizione <riga, colonna> e un "colore", ritorni una DroppableList di gemme attigue dello stesso tipo)
Un momento: Grid non deve implementare un metodo per ogni action ma semplicemente fornire gli strumenti necessari per implementare le action negli oggetti polimorfici.
jappilas
30-01-2008, 19:23
Un momento: Grid non deve implementare un metodo per ogni action ma semplicemente fornire gli strumenti necessari per implementare le action negli oggetti polimorfici.che è quello che volevo dire, metodi primitivi (e meglio testabili) a supporto dell' intelligenza decentralizzata nelle Droppable :)
"un metodo di Grid per ogni action attuale" è quello che mi è parso (con terrore) di vedere nella task list di Alberto nell' altro thread, e rispondevo "bilateralmente" :D
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.