On 03/05/2012 01:36, Enrico Giurin wrote:
>> Anche tralasciando i problemi filosofici e di correttezza formale, la tua
>> richiesta ha notevoli problemi pratici:
>>
>> * modifica sostanzialmente il comportamento attuale, creando disastri per il
>> codice esistente.
> Beh pensavo a definire una specifica annotation, se il dao � annotato
> con questo lancia l'eccezione RecordNotFoundException, altrimenti il
> compartamento � lo stesso di ora.
> Quindi non altererebbe il codice esistente.
Non mi pare molto coerente. In questo modo all'interno della stessa
applicazione, e in modo sostanzialmente fuori controllo da parte del
programmatore (i DAO possono arrivare anche da plugins o librerie su cui
lui non ha nessuna possibilit� di modifica), ci si ritroverebbero
comportamenti diversi, e ci si dovrebbe ricordare se occorre gestire un
potenziale null, o una potenziale RecordNotFoundException.
Fatta cos�, mi pare proprio una bella fonte di confusione ed errori.
Se proprio si deve fare dovrebbe essere un comportamento configurabile
ma coerente in tutti i DAO dell'applicazione, non per singolo DAO.
>> * costringerebbe a chi utilizza Lambico a scrivere un try/catch per una
>> semplice verifica di esistenza di un record, operazione frequentissima e che
>> non pu� essere delegata ad un qualche chiamante, quindi immagina l'impatto
>> sulla semplicit� del codice.
> Not really, pensavo a una RuntimeException, quindi la gestisci solo se
> potenzialmente ti aspetti che non ci sia quel record, se invece lo dai
> per scontato � bene che ad un certo punto ti trovi l'eccezione
> RecordNotFoundException.
La stessa cosa accade con i null: se potenzialmente ti aspetti che non
ci sia quel record, lo gestisci, altrimenti no.
>> * non vedo nessun vantaggio pratico: invece di avere delle
>> NullPointerException (causate dall'imperizia del programmatore, che non sa
>> che bisogna gestire i null), avremmo sempre e comunque, anche in casi
>> perfettamente normali, delle RecordNotFoundException. E' una miglioria?
> Odio gestire i casi if(a==null) etc e come me altri, � boilerplate
Francamente trovo pi� boilerplate i try/catch.
Giusto per farti un esempio, con un pezzo di codice che ho scritto
stamattina:
Category dbCategory = categoryDao.findByName(category.getName());
if (dbCategory == null) {
dbCategory = new Category();
BeanUtils.copyProperties(category, dbCategory, new
String[]{"properties"});
categoryDao.create(dbCategory);
}
Se avessi dovuto far la stessa cosa gestendo un eccezione invece che un
null, sarebbe diventato qualcosa del tipo:
Category dbCategory = null;
try {
dbCategory = categoryDao.findByName(category.getName());
} catch (RecordNotFoundException e) {
dbCategory = new Category();
BeanUtils.copyProperties(category, dbCategory, new
String[]{"properties"});
categoryDao.create(dbCategory);
}
La critica principale che si pu� fare a questo secondo pezzo di codice �
che all'interno del catch non si fa alcun uso dell'eccezione, quindi
tale gestione dell'eccezione serve esclusivamente a controllare il
flusso normale di esecuzione del codice all'interno del metodo. E gi�
questo dovrebbe farci sentire qualche puzza sotto il naso: le eccezioni
servono a gestire i flussi "anomali" (imprevisti) del codice, non quelli
che invece sono perfettamente normali. Poi personalmente trovo pi�
difficile identificare immediatamente cosa fa il secondo frammento di
codice (quello con try/catch), rispetto al primo (quello con l'if).
Altre critiche:
* quello sopra � un caso semplice (in realt� il codice da cui l'ho
estratto era gi� piuttosto pi� complicato): in casi pi� complicati
probabilmente all'interno del try ci sarebbe pi� codice (quindi diventa
poi pi� difficile capire da dove arriva quell'eccezione), o ci sarebbero
pi� blocchi try/catch (bruttissimo codice).
* mi costringe a definire variabili (dbCategory nell'esempio) prima del
primo punto in cui le utilizzo...e questa � una rottura di scatole
infinita (ricordiamoci i vari try/catch per l'input/output).
* banalmente, ha un numero maggiore di righe (ok, non molte, ma �
comunque un indice di maggior verbosit� e/o complessit�)
..la critica maggiore rimane comunque che nel codice in esame l'evento
"non trovare il record" non � un caso eccezionale, ma esattamente quello
che ci si aspetta. Quindi perch� doverlo gestire con un eccezione?
Se, al contrario, si � sempre sicuri di trovare il record, che
differenza fa il non gestire il null, e non gestire l'eccezione
RecordNotFoundException? Quindi, perch� introdurre l'eccezione
RecordNotFoundException?
Potrei immaginarla utile solo per il metodo del DAO get (quello che fa
la ricerca usando la chiave primaria), ma solo se la chiave primaria �
una chiave "surrogata" (fittizia...ad esempio, un intero progressivo,
senza nessun legame con il contenuto effettivo del record). Tale chiave,
essendo surrogata, sar� stata ottenuta interrogando il DB, quindi usarla
per recuperare il record, e non trovarlo, potrebbe effettivamente essere
un'eccezione (che per� potrebbe capitare esclusivamente se hai
recuperato l'id in una transazione diversa, e se qualcuno nel frattempo
ti ha cancellato il record da sotto il naso...vale davvero la pena di
considerarlo?). Per� in qualunque altro caso in cui la chiave di ricerca
viene costruita, o addirittura � ottenuta usando input dall'utente, no,
non � un'eccezione, solo un modo sbagliato di usare le eccezioni. Dato
che non possiamo imporre che la chiave primaria sia sempre una chiave
"surrogata", anche se � probabilmente la scelta di design migliore per
un DB, non mi pare il caso di modificare il comportamento di get.
Passando da Lambico a Parancoe poi, non � che si pu� lasciare al
programmatore questa scelta, che ripeto eventualmente dovrebbe essere
una configurazione globale, non ad-hoc per ogni DAO: come sarebbe
possibile avere un sistema modulare (anche solo con i plugin attuali, o
anche solo con librerie esterne che usino DAO) senza sapere a priori
come si comportano i DAO? Se scrivo un plugin aspettandomi
RecordNotFoundException, sar� sostanzialmente inutilizzabile in
un'applicazione configurata per restituire semplicemente null (e viceversa).
>>
>>>
>>> Si ne abbiamo gi� parlato, anche qui andrebbero definite specifiche
>>> annotation per gestire tutti i casi, tipo con un annotation nel
>>> testcase faccio si di caricare tutte le fixtures presenti, altrimenti
>>> se esplicitamente faccio override del metodo getFixtures() carico solo
>>> quelle.
>>> Insomma non sarebbe male, soprattutto non dovere definire l'ordine di
>>> caricamento / cancellazione.
E' esattamente quest'ultima cosa che vedo piuttosto difficile.
--
Lucio Benfante
http://www.lambico.org
JUG Padova
http://www.parancoe.org
www.jugpadova.it http://www.jugevents.org