Orrore, abbiamo una GetType!!!

[71104]

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...
• osservazione numero 2: ...molti dei quali sono delle cose disgustose 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]

Originariamente inviato da 71104

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...
• osservazione numero 2: ...molti dei quali sono delle cose disgustose 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

[71104]

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
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
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.

[71104]

Originariamente inviato da 71104

(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 ).

[cdimauro]

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.

[71104]

AbstractDroppable non corrisponde alla descrizione?

[Bonfo]

Probabilmente la struttura convoluta viene fuori dalla stratificazione diversi task di diverse storie. Per fortuna esiste il refactoring!!! ( 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]

Originariamente inviato da cdimauro

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ì )

Originariamente inviato da Bonfo

Probabilmente la struttura convoluta viene fuori dalla stratificazione diversi task di diverse storie. Per fortuna esiste il refactoring!!! ( 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

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)

Originariamente inviato da 71104

AbstractDroppable non corrisponde alla descrizione?

più o meno sì, se le si accorpa AbstractSingleDroppable e si rimuove la pletora di interfaccie attuale

[cdimauro]

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).

[VICIUS]

Il fronte anti interfacce continua acrescere

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.

[71104]

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.

[thebol]

Originariamente inviato da 71104

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.

[71104]

Originariamente inviato da thebol

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...

[thebol]

Originariamente inviato da 71104

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.

[fek]

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 :|

[fek]

Originariamente inviato da VICIUS

Il fronte anti interfacce continua acrescere

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.

[fek]

Originariamente inviato da 71104

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]

Originariamente inviato da 71104

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

[71104]

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]

Originariamente inviato da VICIUS

Il fronte anti interfacce continua acrescere

Se fossi anti-interfacce non avrei masochisticamente proposto di crearne una per Environment (per far sparire le factory)...

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.