[Importante] Refactoring di BigGem / Droppable & co.

[Ufo13]

Portiamo alla luce alcune questioni che riguardano questo importantissimo refactoring:

Il nostro obiettivo è quello di far sparire la differenziazione dei comportamenti di Grid/GridController nei confronti di BigGem ed i vari Droppable.

Il problema nasce dal fatto che è un refactoring più grosso del previsto in quanto richiede il cambiamento di molte parti del codice (Grid, GridController, BigGem, le varie DroppableAction e gli State) che non è semplice da eseguire a piccoli passi e la scorsa settimana mi sono ritrovato a tentare alcune strade per poi revertare quando mi accorgevo che prima bisognava essere fatto qualcos'altro.

La strada che attualmente ho scelto è quella di eliminare ogni riferimento a Gem all'interno di BigGem.

Ho notato che per fare ciò bisogna rivedere i comportamenti di canMoveDown e moveDown di BigGem. Qui si rivela un problema. Il comportamento di canMoveDown e moveDown di Droppable non è per niente chiaro e nonostante qualche refactoring ancora ci da dei problemi:

Innanzitutto Droppable non deve dipendere dalla posizione della Sprite (pixel) nello schermo e dalla posizione della griglia (sempre pixel) per determinare se ci si può muovere verso il basso. Questo comportamento va assolutamente chiarito perchè in BigGem non vorrei riprodurre comportamenti ambigui che darebbero problemi in futuro.

Il cuore del problema risiede nel fatto che gli oggetti si muovono non solo a livello di "cella" ma anche a livello di "pixel" durante la caduta e questa gestione deve essere indipendente da Sprite.

Se avete proposte in merito sarò lieto di valutarle

[Bonfo]

Siccome ho ci ho lavotarato un po' sopra dico le mie idee.

Partiamo da un assunto: qualunque tipo di gravità, che rappresente il valore di spostamente verticale, è sempre MINORE della grandezza verticale della cella.

Ora si possono presentare 3 casi:
1) la gemma non si può muovere
2) la gemma può cadere e può muoversi dell'interà gravità senza collidere
3) la gemma può cadere ma non può muoversi dell'intera gravità perchè colliderebbe prima.

Dunque canMoveDown ritorna false nel caso 1 e true nel caso 2 e 3.

MoveDown opera solo nei casi 2 e 3, quindi controlla prima canMoveDown.
Se 2 aggiorna la posizione muovendo dell'intera gravità, altrimenti se 3 si allinea la gemma alla riga.
A questo punto sdi ricontrolla canMoveDown e se è passato a false si fa drop.

Fin qua direi che è semplice...

Ora ci sono due questioni:
a) come calcolare l'offset rispetto alla cella.
b) aggironare la cella capendo che la gemma ha cambiato riga.

Fino ad ora direi che il cambio di riga è stato determinato nel momento in cui la gemma supera il bordo inferiore della cella corrente o, in altre parole, quando entra nel bordo superiore della cella sottostante. In tal caso vienbe aggiornato il valore della cella.

Io manterrei l'offset coerente con questa informazione. Quindi offset=0 implica gemma allineata alla cella. Quindi obbligattoriamente l'offset si calcolerebbe tra il bordo superiore della cella e della gemma. In tal modo l'offset si comporta in questo modo:
0: allineta
31: entrata in una nuova riga (Cell.SIZE = 32)
30
29
....
....
1
0: allineata nuovamente

Tuttto chiaro??

Ora è indubbio che la determinazione del cambio di riga può essere fatto in modo diverso, basta che sia mantenuto coerentemente nel codice, come ad esempio considerare il cambio di riga solo ad allienamento concluso.

Ora le lamentele.
Devo dire che a sto giro i test stanno aiutando molto poco
Molto probabilmente per un problema che avevo già sollevato relativamente agli update: praticamente ogni test usa un numero di update empirico, calcolato per tentativi, in modo che il test abbia il comportamento corretto...e non fraintendetemi, non sto dicendo che sia stato fatto il codice prima del test.
Questo fa si che molti test non siano chiarissimi o che definiscano esattamente che cosa volevano testare.

Ok...direi che ho detto

[Bonfo]

Altra cosa....propongo un direttore d'orchestra per questo refactoring.
E' molto complesso e avere un supervisore attivo al refactoring mi sembrerebbe un ottima idea.

Io propongo Ufo perchè è oltre una settimana che ci sta sopra e ormai ha un quadro sufficentemente ampio.

[Ufo13]

Originariamente inviato da Bonfo

Altra cosa....propongo un direttore d'orchestra per questo refactoring.
E' molto complesso e avere un supervisore attivo al refactoring mi sembrerebbe un ottima idea.

Io propongo Ufo perchè è oltre una settimana che ci sta sopra e ormai ha un quadro sufficentemente ampio.

E l'orchestra?

[Bonfo]

Originariamente inviato da Ufo13

E l'orchestra?

Se vuoi io faccio il triangolo

[cdimauro]

Adesso vi dico la mia da niubbone che ha appena finito di visionare tutti i sorgenti legati a Gem, BigGem, Grid, ecc. ecc. e che per questo è ancora in stato confusionale : non sarebbe il caso che Grid gestisse tutti gli oggetti in maniera "uniforme"?

Con ciò intendo: avere a che fare con UN UNICO tipo base per gli oggetti, che poi sono specializzati e si comportano in modo diverso a seconda dell'implementazione.

Quindi Gem, BigGem, Chest, ecc. ma anche la stessa GemsPair dal punto di vista di Grid dovrebbero essere oggetti (di dimensione arbitraria) che si muovono e cadono oppure si sono fermati in fondo alla griglia, e che "interagiscono" con la griglia e con gli altri oggetti eventualmente in essa presenti.

Interazione vuol dire che il loro stato e quello degli oggetti con cui interagiscono può cambiare.

E' troppo sempliciotto come discorso, vero?

Ehi, tu, ti ho visto, sai: metti giù quella mazza!!!

[jappilas]

Originariamente inviato da cdimauro

Adesso vi dico la mia da niubbone che ha appena finito di visionare tutti i sorgenti legati a Gem, BigGem, Grid, ecc. ecc. e che per questo è ancora in stato confusionale : non sarebbe il caso che Grid gestisse tutti gli oggetti in maniera "uniforme"?
...
E' troppo sempliciotto come discorso, vero?

veramente, no
dal mio pdv anzi è quello che mi aspettavo diamonds già facesse prima di vedere il codice effettivamente implementato

ps: composite + delegation ?

[cdimauro]

Originariamente inviato da jappilas

veramente, no
dal mio pdv anzi è quello che mi aspettavo diamonds già facesse prima di vedere il codice effettivamente implementato

Idem: adoro la semplicità.

ps: composite + delegation ?

Se serve a semplificare tutto e a renderci la vita più facile, perché no?

[Bonfo]

Originariamente inviato da cdimauro

Adesso vi dico la mia da niubbone che ha appena finito di visionare tutti i sorgenti legati a Gem, BigGem, Grid, ecc. ecc. e che per questo è ancora in stato confusionale : non sarebbe il caso che Grid gestisse tutti gli oggetti in maniera "uniforme"?

Con ciò intendo: avere a che fare con UN UNICO tipo base per gli oggetti, che poi sono specializzati e si comportano in modo diverso a seconda dell'implementazione.

Quindi Gem, BigGem, Chest, ecc. ma anche la stessa GemsPair dal punto di vista di Grid dovrebbero essere oggetti (di dimensione arbitraria) che si muovono e cadono oppure si sono fermati in fondo alla griglia, e che "interagiscono" con la griglia e con gli altri oggetti eventualmente in essa presenti.

Interazione vuol dire che il loro stato e quello degli oggetti con cui interagiscono può cambiare.

E' troppo sempliciotto come discorso, vero?

Ehi, tu, ti ho visto, sai: metti giù quella mazza!!!

L'idea è stata sin dall'inizio quella
Figuratevi che il moveDown delle AbstractdDroppable lo faceva grid...quindi altro che omogeneità di gestione

L'idea che esponi è esattamente quello a cui dobbiamo arrivare, già fek in un precedente post ha sottolineato come le action siano da considerasi azioni che modificano lo stato della griglia e, perchè no, queste chiedano ai singoli oggetti dimofiicare il loro stato

Il problema è arrivarci a quel punto considerando che il codice rema al contrario

[cdimauro]

Infatti: il mal di testa dovuto alla lettura del codice mi si trascina ancora...

Vediamo di arrivarci, anche se so bene che sarà un'impresa titanica.

Continuo a sbatterci la testa per cercare di capire DA DOVE si potrebbe iniziare...

[cisc]

allora, io sono sempre alle prese con il refactoring di AbstractDroppable, in modo che si usi un sistema di coordinate relative alla riga della droppable, in particolare l'idea proposta da Ufo era quella di mantenere un yOffsetByCell che mantenessel'offset tra il bottom della droppable ed il top della cella (può essere un po contorto, ma è il valore che rende più semplice tutto il lavoro di controllo di canMoveDown, e cambio di riga in moveDown)...introducendo questo refactoring abbiamo avuto il problema che in pratica nella gemsPair la gemma sopra, usando il metodo isCellBelowFull di grid, si droppava perchè rilevala che l'altra gemma della pair occupava la cella sotto, allora Bonfo ha proposto di controllare anche se la gemma che occupi la cella sotto sia dropped....ho seguito questa strada, ho dovuto modificare qualche test, ma mi sono trovato di fronte ad un unico test che non passa relativo alle caduta di una bigGem dopo un crush (testBigGemFallingWithGravityToGridBottom) ...ora devo sospendere il lavoro, mi sembrava che le gemme, dopo una crush, cadessero da dropped, e mi sembra strano che sia solo un test a non passare...cosa ne pensate!?

[cdimauro]

Originariamente inviato da cisc

introducendo questo refactoring abbiamo avuto il problema che in pratica nella gemsPair la gemma sopra, usando il metodo isCellBelowFull di grid, si droppava perchè rilevala che l'altra gemma della pair occupava la cella sotto, allora Bonfo ha proposto di controllare anche se la gemma che occupi la cella sotto sia dropped....ho seguito questa strada, ho dovuto modificare qualche test, ma mi sono trovato di fronte ad un unico test che non passa relativo alle caduta di una bigGem dopo un crush (testBigGemFallingWithGravityToGridBottom) ...ora devo sospendere il lavoro, mi sembrava che le gemme, dopo una crush, cadessero da dropped, e mi sembra strano che sia solo un test a non passare...cosa ne pensate!?

Penso che col modello che ho esposto questo problema sparirebbe del tutto, perché la GemsPair diventerebbe un'oggetto a sé stante ed è in quanto tale che "interagirebbe" con la griglia e gli altri oggetti (Gem, BigGem, ecc.).

Le due gemme di cui è composta, insomma, sono un dettaglio puramente interno e che NON dev'essere noto agli altri.
Soltanto gli effetti delle interazioni sono importanti, ed è solo in questo caso che le due "gemme" di cui è composta una GemsPair incideranno nei cambiamenti di stato di oggetti e griglia.

[Bonfo]

Interessante. Quindi anche GemsPair dovrebbe implementare Droppable...

... MI PIACE !!!

[cdimauro]

Certamente. In questo modo si uniformerebbe la gestione di QUALUNQUE oggetto che Grid deve gestire, presente e futuro.

L'unica cosa che cambia è la diversa "reazione" di un oggetto quando viene a contatto con un altro oggetto oppure col fondo della griglia.

[Ufo13]

Originariamente inviato da cdimauro

Adesso vi dico la mia da niubbone che ha appena finito di visionare tutti i sorgenti legati a Gem, BigGem, Grid, ecc. ecc. e che per questo è ancora in stato confusionale : non sarebbe il caso che Grid gestisse tutti gli oggetti in maniera "uniforme"?

Con ciò intendo: avere a che fare con UN UNICO tipo base per gli oggetti, che poi sono specializzati e si comportano in modo diverso a seconda dell'implementazione.

Quindi Gem, BigGem, Chest, ecc. ma anche la stessa GemsPair dal punto di vista di Grid dovrebbero essere oggetti (di dimensione arbitraria) che si muovono e cadono oppure si sono fermati in fondo alla griglia, e che "interagiscono" con la griglia e con gli altri oggetti eventualmente in essa presenti.

Interazione vuol dire che il loro stato e quello degli oggetti con cui interagiscono può cambiare.

E' troppo sempliciotto come discorso, vero?

Ehi, tu, ti ho visto, sai: metti giù quella mazza!!!

Hai azzeccato in pieno l'obbiettivo finale di questo refactoring

[Ufo13]

Originariamente inviato da Bonfo

Interessante. Quindi anche GemsPair dovrebbe implementare Droppable...

... MI PIACE !!!

A mio parere ora abbiamo la priorità di eliminare la BigGemList da Grid. Per fare ciò BigGem deve diventare indipendente da Droppable prima di tutto.

Inoltre non devono più esistere in Grid le Gem che costituiscono una BigGem, nel momento in cui vengono assorbite da una BigGem devono essere eliminate

[Bonfo]

Certo...è quello l'obbiettivo.
Il problema è che se implementiamo il moveDown di AbstractDroppable come se fosse "sola" succede quello che diceva daniele....e quindi test rossi

Bisogna implementare per ogni tipo, cioè BigGem, AbstractDroppable e GemsPair, il moveDown attraverso varie iterazioni su per cercare di disaccoppiarli sempre un po' di più ad ogni iterazione...altrimenti ci scordiamo i test verdi per un bel po'

[cdimauro]

Ragazzi, è un vero casino.

Vi faccio la proposta indecente di cui ho già parlato con Federico ieri sera.

Per arrivare alla soluzione del problema ci sono 3 strade:
1) buttare tutto ciò che riguarda Gem, BigGem, ecc. e ricominciare da capo;
2) rifattorizzare a forza finché non si raggiunge l'obiettivo;
3) sviluppare le nuove classi parallelemente a quelle esistenti, trasferendo man mano le competenze di queste ultime.

La 1), anche se la preferirei (quando il codice è troppo incasinato preferisco buttare tutto, fermarmi a riflettere, e "rebuild from scratch"), è da scartare, perché ci bloccherebbe e noi siamo già troppo indietro con lo sviluppo.

La 2) è quella che stiamo seguendo adesso, e che ci sta facendo perdere parecchio tempo.

La 3) è la mia proposta. Si può benissimo andare avanti con le storie, finché non ci sarà il "passaggio di consegne" fra vecchie e nuove classi. Inoltre ci permette di testare il funzionamento delle vecchie e delle nuove classi, per vedere se ci sono "deviazioni" dal contratto / requisiti stabilito ("Quante deviazioni hai... Dimmi che non ne hai!" - Vasco ).

OK, lo so: mi sono scavato la fossa con le mie mani. E' stata una vita breve, ma intensa.

Comunque oggi continuo a studiarmi i sorgenti, ma da domani e fino a martedì non ci sarò (sono a Roma, e senza PC. ).

Cerchiamo di definire bene il da farsi. Sono convintissimo che, una volta raggiunto l'obiettivo, avremo la strada spianata e potremo andare avanti molto più velocemente.

Investire oggi per guadagnare domani (lo so, è YAGNI, ma abbassate la baionette!!! ).

[VICIUS]

Originariamente inviato da cdimauro

La 1), anche se la preferirei (quando il codice è troppo incasinato preferisco buttare tutto, fermarmi a riflettere, e "rebuild from scratch"), è da scartare, perché ci bloccherebbe e noi siamo già troppo indietro con lo sviluppo.

È anche la mia preferita. Questo è uno dei rari casi una approccio top-down classico è più conveniente. Abbiamo le specifiche chiare ed immutabili nei test. Penso che con un paio di persone full-time e una settimana a lavorarci in un branch separato sarebbe possibile completare la riscrittura. Il problema è che togliendo un paio di persone dai task normali dimezziamo quasi il team di sviluppo quindi non praticabile con le nostre risorse.

Originariamente inviato da cdimauro

La 2) è quella che stiamo seguendo adesso, e che ci sta facendo perdere parecchio tempo.

Abbiamo già appurato che è poco efficace.

Originariamente inviato da cdimauro

La 3) è la mia proposta. Si può benissimo andare avanti con le storie, finché non ci sarà il "passaggio di consegne" fra vecchie e nuove classi. Inoltre ci permette di testare il funzionamento delle vecchie e delle nuove classi, per vedere se ci sono "deviazioni" dal contratto / requisiti stabilito ("Quante deviazioni hai... Dimmi che non ne hai!" - Vasco ).

Possiamo provare.

ciao

[71104]

Originariamente inviato da cdimauro

Investire oggi per guadagnare domani (lo so, è YAGNI, ma abbassate la baionette!!! ).

veramente non è YAGNI...