View Full Version : Sto refactoring sa da fà o non sa da fà??
Vado subito al nocciolo. :D
Lungo il nostro lavoro abbiamo sviluppato TDD e abbiamo aggiunto tutte le funzionalità in questo modo.
Così facendo però non abbiamo mai realizzato una buona gestione degli errori e delle eccezzioni.
Cosa voglio dire: ci sono alcune classi che se vendute come componente separato, cosa che sarebbe giusto poter fare, avrebbero un sacco di comportamenti non prestabiliti perchè, ad esempio, non si fa nessun controllo sui parametri in ingresso di alcuni metodi o costruttori.
Inoltre, nel caso il controllo sia stato fatto, non c'è una trattazione omogenea. Ovvero alcune volte si restituisce dei null, altre volte delle eccezzioni, altre volte altro ancora.
Ecco allora la mia domanda: dobbiamo riprendere in mano tutto ed aggiungere tutti i controlli sui parametri, naturalmente con test annessi, oppure va bene così?!?
Domanda secondaria: ma siamo noi che non abbiamo applicato perfettamente il TDD oppure è il TDD che ha una mancanza???
Grazie per aver sopportato il mio sproloquio :flower:
cdimauro
24-03-2006, 07:27
Sono d'accordo sulla trattazione omogenea, se è possibile e ha senso ovviamente.
Per quanto riguarda i test, invece, penso che se l'obiettivo di esplicitare i requisiti viene raggiunto, non vedo perché dovremmo occuparci di aggiungere altri controlli sui valori ricevuti in ingresso e l'output risultante: per Diamonds va bene così.
Sarà la natura stessa dell'approccio TDD che ci porterà, quando se ne presenterà la necessità, ad aggiungere altri test per "coprimere meglio" l'utilizzo dei componenti.
Il tutto IMHO, chiaramente. :p
Per i controlli sui parametri sono sfavorevole. Abbiamo i test che indicano quali parametri accetta la nostra api in ingresso. Se un autore manda in input ad una nostra classe valori non validi anche sapendo dei test allora c'è qualcosa che non va nella mente del programmatore. :)
Se però lo fa perché i test sono poco chiari o non specificano bene queste cose è il caso di aggiungere qualche test qua e la per chiarificare queste cose.
Per quanto riguardi una gestione omogenea degli errori cosa intendi?
ciao ;)
cdimauro
24-03-2006, 10:28
Perfettamente d'accordo.
Penso si riferisca a questo:
"Ovvero alcune volte si restituisce dei null, altre volte delle eccezzioni, altre volte altro ancora."
Perfettamente d'accordo.
Penso si riferisca a questo:
"Ovvero alcune volte si restituisce dei null, altre volte delle eccezzioni, altre volte altro ancora."
Si ma non capisco cosa vuole fare. Dovremmo per caso usare solo eccezioni?
ciao ;)
Per i controlli sui parametri sono sfavorevole. Abbiamo i test che indicano quali parametri accetta la nostra api in ingresso. Se un autore manda in input ad una nostra classe valori non validi anche sapendo dei test allora c'è qualcosa che non va nella mente del programmatore. :)
1) Cambio la domanda per farmi capire. Supponiamo di vendere il package it.diamonds.engine (esempio a caso), ovviamente senza codice. In questo caso dobbiamo garantire il perfetto funzionamento a chi lo vendiamo.
Cosa facciamo: gli diamo l'intero insieme dei test perchè possa controllare i parametri?? E se passa dei parametri sbagliati..come facciamoa dirglielo.
Nonero lo stesso fek che aveva detto: "Per la gestione delle condizioni di errore applichiamo la filosofia "Die very painfully". "
Questa domanda è molto più filosofica che un problema reale in diamonds :rolleyes:
Il fatto è che ho pensato alla discussione con fek del riutilizzo con l'agile programming. E lui mi ha pienamente convinto con l'idea che da un progetto si possono "promuovere" ad un nodo superiore ad entrambi i progetti le classi che possono diventare librerie.
Il fatto è che però le classi promosse si dovrebbero comportare da liberire...quindi anche con tutti i controlli sui parametri
Spero di essere stato chiaro ... :mc: :mc:
Per quanto riguardi una gestione omogenea degli errori cosa intendi?
2) Intendo semplicemente decidere in quali casi usare le eccezioni e in quali casi usare i null o altro.
Per esempio: molte creazioni di oggetti non hanno controllo sui parametri.
Nel momento in cui si farà il controllo cosa si fa?
-si lancia un eccezzione dicendo "cretino hai sbagliato i parametri"
-si ritorna null e dici: cacchi suoi, ha sbagliato e mo se la gestisce lui
Sono approcci diversi che comportano codice diverso da parte dell'utilizzatore..è un gioco di responsabilità fra le classi.
In diamonds sono mischiate le tre cose:
1) non si fa nulla
2) si ritorna null
3) si lanciano eccezzioni
Io dico, nel caso si dica che i controlli vanno fatti, mettere giù una linea generale...che poi sarà valutata caso per caso.
Spero di essere stato chiaro ... anche qua :mc: :mc:
P.S.:mazza quanto ho scritto :D :D
Secondo me bisogna mettere questi pnti fermi:
- nel repository non deve entrare codice non testato
- se le condizioni di errore non sono testate non devono entrare nel repository
IMHO la penso così, magari mi sbaglio, ma se una classe deve essere promossa a libreria allora dovrà essere testata come tale (il TDD dice che in teoria si potrebbero anche fare test di sistema sulle librerie usate, comprese le API Java)...
Quindi cionci diresti di aggiungere i test relativi ai parametri nel momento della promozione...
...MI PIACE :D
cdimauro
24-03-2006, 13:09
1) SE devono diventare API di libreria, magari sì, ma in mancanza è meglio seguire la filosofia YAGNI.
2) D'accordo. Amo l'uniformità e nel caso di errori preferisco, in genere, che sia sollevata un'eccezione. Non amo però la gestione delle eccezioni di Java con la specifica dei throws. :D
1) Cambio la domanda per farmi capire. Supponiamo di vendere il package it.diamonds.engine (esempio a caso), ovviamente senza codice. In questo caso dobbiamo garantire il perfetto funzionamento a chi lo vendiamo.
Cosa facciamo: gli diamo l'intero insieme dei test perchè possa controllare i parametri?? E se passa dei parametri sbagliati..come facciamoa dirglielo.
Nonero lo stesso fek che aveva detto: "Per la gestione delle condizioni di errore applichiamo la filosofia "Die very painfully". "
Questa domanda è molto più filosofica che un problema reale in diamonds :rolleyes:
Il fatto è che ho pensato alla discussione con fek del riutilizzo con l'agile programming. E lui mi ha pienamente convinto con l'idea che da un progetto si possono "promuovere" ad un nodo superiore ad entrambi i progetti le classi che possono diventare librerie.
Il fatto è che però le classi promosse si dovrebbero comportare da liberire...quindi anche con tutti i controlli sui parametri
Spero di essere stato chiaro ... :mc: :mc:
Oltre a fornire la libreira e i test che la rigardano dovremmo preoccuparci di distribuire anche un po' di documentazione su carta, qualche info sul architettura ed uno o due grafici uml. Nel caso di una libreria andrebbe fatta si una validazione più stretta dei parametri di ingresso ma solo nelle funzioni a contatto con l'esterno. Se dovessimo controlalre i parametri in ogni funzione il codice si complicherebbe di molto.
In questo momento engine rimane all'interno di dimaond quindi direi di evitare di aggiungere controlli vari.
2) Intendo semplicemente decidere in quali casi usare le eccezioni e in quali casi usare i null o altro.
Per esempio: molte creazioni di oggetti non hanno controllo sui parametri.
Nel momento in cui si farà il controllo cosa si fa?
-si lancia un eccezzione dicendo "cretino hai sbagliato i parametri"
-si ritorna null e dici: cacchi suoi, ha sbagliato e mo se la gestisce lui
Sono approcci diversi che comportano codice diverso da parte dell'utilizzatore..è un gioco di responsabilità fra le classi.
In diamonds sono mischiate le tre cose:
1) non si fa nulla
2) si ritorna null
3) si lanciano eccezzioni
Io dico, nel caso si dica che i controlli vanno fatti, mettere giù una linea generale...che poi sarà valutata caso per caso.
Spero di essere stato chiaro ... anche qua :mc: :mc:
P.S.:mazza quanto ho scritto :D :D
In effetti non abbiamo delle linee guida. Provo a scrivere qualcosa ora da quello che mi ricordo di cc2 che diceva secondo me cose molto sensate:
- niente try/catch vuote se possibile.
- Niente scarica barili. Se si può gestire una eccezione la si deve gestire nel momento in cui la si intercetta.
- Usare le eccezioni solo per casi veramente speciali od errori che non possono essere ignorati. Ad esempio quando un parametro passato è valido ma non è possibile portare a termine una funzione per qualche motivo. config.getInteger("proprieta"); Qui "proprieta" è un valore valido ma se non è presente nel file di configurazione va lanciata una eccezione.
Esempio in cui non fare check e non usare eccezioni. grid.getGemAt(-1009,2044); I parametri sono completamente sballati. Non serve controllare ogni volta. Se l'accesso avviene ad indici assurdi ci pensa java stesso a lanciare una runtime exception.
- Niente eccezioni lanciate dai costruttori. Un null dopo una new è già di per se un segno che qualcosa è andato storto. Se chi ha chiamato il costruttore non controlla allora ci penserà java a lanciare una eccezione nel momento in cui cercherà di accedervi.
- Usare messaggi per le eccezioni con più informazioni possibili.
ciao ;)
cdimauro
24-03-2006, 13:42
Oltre a fornire la libreira e i test che la rigardano dovremmo preoccuparci di distribuire anche un po' di documentazione su carta, qualche info sul architettura ed uno o due grafici uml.
[fek MODE ON]
I test sono la migliore documentazione del codice. :)
[fek MODE OFF]
:D
- Niente eccezioni lanciate dai costruttori. Un null dopo una new è già di per se un segno che qualcosa è andato storto. Se chi ha chiamato il costruttore non controlla allora ci penserà java a lanciare una eccezione nel momento in cui cercherà di accedervi.
Però in questo modo si perderà l'informazione sulla causa che ha scatenato l'errore. La JVM, insomma, solleverà una "anonima" NullObjectException o giù di lì.
Se invece il codice solleva un'opportuna eccezione, magari avremo più informazioni su cos'è successo e sarà più facile provvedere al debug e al fix.
E' chiaro che, comunque, dipende dai casi. ;)
[fek MODE ON]
I test sono la migliore documentazione del codice. :)
[fek MODE OFF]
:D
Dai che qualche disegnino e qualcosa di generale può sempre servire. Anche a fek non dispiacerebbe :p
Però in questo modo si perderà l'informazione sulla causa che ha scatenato l'errore. La JVM, insomma, solleverà una "anonima" NullObjectException o giù di lì.
Se invece il codice solleva un'opportuna eccezione, magari avremo più informazioni su cos'è successo e sarà più facile provvedere al debug e al fix.
E' chiaro che, comunque, dipende dai casi. ;)
Hai ragione. In oggetti in cui il costruttore ha molti parametri in effetti potrebbe essere molto utile ma a patto di usare eccezioni giuste e fornire messaggi chiari. Finché un oggetto ha uno o due parametri ci si riesce anche senza eccezioni.
ciao ;)
Cosa voglio dire: ci sono alcune classi che se vendute come componente separato, cosa che sarebbe giusto poter fare, avrebbero un sacco di comportamenti non prestabiliti perchè, ad esempio, non si fa nessun controllo sui parametri in ingresso di alcuni metodi o costruttori
YAGNI!!!
Hai ragione. In oggetti in cui il costruttore ha molti parametri in effetti potrebbe essere molto utile ma a patto di usare eccezioni giuste e fornire messaggi chiari. Finché un oggetto ha uno o due parametri ci si riesce anche senza eccezioni.
ciao ;)
Soluzione: se un costruttore ha molti parametri lo si rifattorizza per averne meno :)
Manco a cercarlo :D
Allora mi stavo accingendo ad aggiungere un test per migliorare la copertura.
Lancio cobertura e dal report risulta che CreateNewBigGemsAction ha un 3 linee di codice non coperte.
Le linee non coperte riguarda un controllo sul parametro gem in ingersso diverso da null nel metodo isGemNotValidForBigGem(),
Guardo e scopro che è un metodo privato, quindi non direttamente testabile.
Vado a vedere come viene usato dalla classe. Il metodo viene invocato da un metodo pubblico senza alcun controllo sul parametro.
Insomma...quelle 3 linee di codice per adesso non sono mai usate in quanto nessuno invoca con il parametro uguale a null.
Quindi, secondo la filosofia YAGNI, le 3 linee spariscono dalla faccia della terra :ciapet:
Giusto??
Quindi, secondo la filosofia YAGNI, le 3 linee spariscono dalla faccia della terra :ciapet:
Giusto??
Dipende. Se l'applicazione puo' logicamente passare un oggetto null e noi dobbiamo controllare la condizione per contratto allora vanno solo testate. Altrimenti saltano.
Secondo me sarebbe meglio procedere al refactoring di Grid dopo aver effettuato un refactoring delle varie gemme, chest, BigGem e stone... Si era parlato di fare una gerarchia di classi se non sbaglio... Io in tal caso sarei per trasferire parte di quello che viene fatto in Grid nelle gemme (come ad esempio la creazione delle BigGem e i crush)...
Secondo me sarebbe meglio procedere al refactoring di Grid dopo aver effettuato un refactoring delle varie gemme, chest, BigGem e stone... Si era parlato di fare una gerarchia di classi se non sbaglio... Io in tal caso sarei per trasferire parte di quello che viene fatto in Grid nelle gemme (come ad esempio la creazione delle BigGem e i crush)...
Non so bene cosa intendi però secondo me le modifiche al "gameplay" devono coinvolgere il meno possibile le entità presenti nel gioco (quindi Gem, Grid etc) mentre dovrebbero esserci delle classi che gestiscono appunto le azioni che riguardano le meccaniche del gioco (come ad esempio fa GridController).
Ok...cerco di spiegarmi meglio... Diverso tempo fa si parlò di creare una gerarchia che implementasse Gem, Chest, BigGem ed a questo punto Stone... Che fine ha fatto questa cosa ?
Io intendevo che avendo a disposizione questa gerarchia ogni classe che appartiene alla gerarchia potrebbe andare ad implementare il meccanismo di crush (e non solo) secondo il significato della classe specifica...
Questo unito alla creazione di eventi specifici...come ad esempio l'arresto della caduta di uno deglli oggetti sopra...oppure l'arresto di tutte le cadute...
Ad esempio: all'evento "arresto della caduta di un oggetto", si può andare a richiamare un metodo dell'oggetto caduto: nelle gemme potrebbe portare alla creazione di BigGem con un algoritmo che tiene conto della posizione corrente...
Scrivo un esempietto:
In corrispondenza dell'evento "arresto della caduta di un oggetto" richiamo il metodo onStop(grid) dell'oggetto caduto...
Nel metodo onStop della classe Gem si andrà a fare queste operazioni:
controllo del colore delle Gem adiacenti (ottenute dal riferimento a Grid), se è possibile formiamo una BigGem. Altrimenti se sono accanto ad una BigGem del mio colore provo ad estendere la BigGem.
All'evento "tutte gli oggetti si sono arrestati" andremo ad applicare il metodo doCrush(grid) a tutti gli oggetti a partire dal più in alto (ad occhio dovrà farlo GridController)...
doCrush provocherà un effetto per le chest (se possibile), un altro effetto per le FlashingGem (se possibile) e nessun effetto per le gem e per le stone..
Per le FlashinGem e per le Chest partirà una catena di crush che farà uso di chiamate a Grid solo per ottenere la posizione ed il colore di una gemma, mentre farà uso di un algoritmo che scorrerà tutte le gemme che faranno parte del crush per cancellarle dalla griglia e calcolare il punteggio.
Quindi con questo metodo riportiamo Grid alla sua funzione originale e inseriamo gli eventi, al costo di spostare un po' intelliggenza in FlashingGem, BigGem, Gem, Stone e Chest, che andranno organizzate secondo una gerarchia di classi (che potrebbe anche essere Droppable)...
E' un'operazione moooolto complessa e che richiederà del tempo, ma che potrebbe snellire di molto TUTTO...
Sono d'accordissimo sulla gestione tipo onDrop però il passaggio di grid all'interno di gem crea un coupling.
Oltre allo snellimento io terrei anche un coupling il più basso possibile :)
Approvo tutto al 100%
...sia quello che ha detto Ufo che Cionci !!! :mano:
Sono d'accordissimo sulla gestione tipo onDrop però il passaggio di grid all'interno di gem crea un coupling.
Oltre allo snellimento io terrei anche un coupling il più basso possibile :)
Ok, ma in tal caso la gestione dovrebbe passare ad altre classi (GridController ??? O qualcosa come DroppableController ????)...
Comunque sulla gerarchia siamo d'accordo ? Io direi di fare quella appaena possibile e poi passare a tirare fuori il codice da Grid...
Suppongo un altro tipo di gestione... La gestione potrebbe essere di questo tipo:
DroppableController è l'interfaccia per tutti i droppable controller...
Dall'interno della gerarchia di classi è possibile chiamare la funzione getController che alloca (la classe non avendo membri interni può semplicemente essere allocata ogni volta) la classe controller relativa all'oggetto della gerarchia su cui abbiamo richiamato il metodo...
Ad esempio:
Gem.getController allocherà e ritornerà un GemController...
Chest.getController allocherà e ritornerà un ChestController...
Stone.getController allocherà e ritornerà un StoneController...
Queste classi avranno tutte la stessa interfaccia che gli imporrà di implementare onDrop e così via...
Non so che design pattern sia...
E' un'operazione moooolto complessa e che richiederà del tempo, ma che potrebbe snellire di molto TUTTO...
E che quindi faremo nel prossimo Ciclo come Storia. Vicius?
Ad esempio:
Gem.getController allocherà e ritornerà un GemController...
Chest.getController allocherà e ritornerà un ChestController...
Stone.getController allocherà e ritornerà un StoneController...
Queste classi avranno tutte la stessa interfaccia che gli imporrà di implementare onDrop e così via...
Non so che design pattern sia...
Non ti preoccupare del DP, implementalo che l'idea mi piace ed e' molto elegante :)
A occhio sembra una specie di Command.
Anche a me ha fatto pensare al Command...
Non ti preoccupare del DP, implementalo che l'idea mi piace ed e' molto elegante :)
Ho pensato ad una cosa del genere per evitare cose del tipo if(gem.isChest()) e così via...
Ottimo cionci. ;)
Un avvertimento prima del refactoring.
Siccome sembra essere un refactoring molto ampio, prima di farlo consiglio una copertura di test di almeno il 100%
:D :D :D
Quando stanoote finisco la mia parte di porgetto di reti aggiungo almeno un test ;)
Cionci io ho un'idea di questo tipo.
action = gem.getDropAction();
action.do(grid);
action è un oggetto che implementa una interfaccia "GemAction". Poi la action esegue un certo numero di operazioni sulla griglia se è il caso :)
E che quindi faremo nel prossimo Ciclo come Storia. Vicius?
Piace molto anche a me come idea. Pero il prossimo ciclo dovrete farlo senza di me. Ora sono senza adsl.
ciao ;)
Cionci io ho un'idea di questo tipo.
action = gem.getDropAction();
action.do(grid);
action è un oggetto che implementa una interfaccia "GemAction". Poi la action esegue un certo numero di operazioni sulla griglia se è il caso :)
Anche questo sarebbe possibile, ma necessiterebbe la creazione di molte classi, una per ogni azione di ogni oggetto (per quelli che nn devono fare operazioni possiamo usare un Null Object), e molti metodi per ritornare le diverse action, invece avrei preferito realizzare una classe che raggruppasse tutte le azioni che si possono svolgere su un droppable... Comunque è da valutare... Dipende tutto da come vengono gestiti gli eventi...
Ad esempio se la gestione fosse centralizzata la tua soluzione potrebbe essere utile, perchè potremmo realizzarla in questo modo:
action = gem.getEventHandler(event);
action.do(grid);
Ma questo implicherebbe forse complicare un po' di più i Droppable (devono fare l'associazione degli eventi con l'handler) e cosa ancora più scomoda che ad ogni evento generato questo venisse applicato su ogni droppable (altrimenti la soluzione non è centralizzata e sarebbe comunque più comodo ritornare alla tua soluzione, ma con le controindicazioni che avevo sopra esposto)...
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.