Sto refactoring sa da fà o non sa da fà??

[Bonfo]

Vado subito al nocciolo.
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

[cdimauro]

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.

[VICIUS]

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]

Perfettamente d'accordo.

Penso si riferisca a questo:

"Ovvero alcune volte si restituisce dei null, altre volte delle eccezzioni, altre volte altro ancora."

[VICIUS]

Originariamente inviato da cdimauro

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

[Bonfo]

Originariamente inviato da VICIUS

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

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

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

P.S.:mazza quanto ho scritto

[cionci]

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

[Bonfo]

Quindi cionci diresti di aggiungere i test relativi ai parametri nel momento della promozione...

...MI PIACE

[cdimauro]

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.

[VICIUS]

Originariamente inviato da Bonfo

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

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

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.

Originariamente inviato da Bonfo

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

P.S.:mazza quanto ho scritto

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]

Originariamente inviato da VICIUS

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]

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

[VICIUS]

Originariamente inviato da cdimauro

[fek MODE ON]
I test sono la migliore documentazione del codice.
[fek MODE OFF]

Dai che qualche disegnino e qualcosa di generale può sempre servire. Anche a fek non dispiacerebbe

Originariamente inviato da cdimauro

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

[fek]

Originariamente inviato da Bonfo

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

[fek]

Originariamente inviato da VICIUS

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

[Bonfo]

Manco a cercarlo
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

Giusto??

[Ufo13]

secondo me spariscono

[fek]

Originariamente inviato da Bonfo

Quindi, secondo la filosofia YAGNI, le 3 linee spariscono dalla faccia della terra

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.

[Bonfo]

Ok...come pensavo io

[cionci]

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

[Ufo13]

Originariamente inviato da cionci

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