So che le persone fanno un grosso problema usando le eccezioni per il controllo del flusso ma questo non è il problema più grande qui. Vedo una grande violazione della separazione di comandi . Ma anche questo non è il peggiore.
No, il peggio è proprio qui:
/**
* Closes the door if open
*/
Perché diavolo fa tutto il resto se la tua ipotesi sullo stato delle porte è sbagliata, ma lock()
lo aggiusta per te? Dimentica che ciò rende impossibile bloccare la porta aperta, il che è assolutamente possibile e occasionalmente utile. No, il problema qui è che hai mischiato due filosofie diverse per affrontare ipotesi errate. Questo è confuso. Non farlo. Non allo stesso livello di astrazione con lo stesso stile di denominazione. Ahi! Porta fuori una di quelle idee. I metodi di servizio delle porte dovrebbero funzionare tutti allo stesso modo.
Per quanto riguarda la violazione del comando Query Separation, non dovrei provare a chiudere una porta per scoprire se è chiusa o aperta. Dovrei riuscire a chiedere. Il servizio porta non fornisce un modo per farlo senza modificare lo stato della porta. Ciò rende molto più grave dei comandi che capita anche di restituire valori (un comune fraintendimento su cosa sia CQS). Qui, i comandi che cambiano lo stato sono l'unico modo per fare query! Ahi!
Poiché le eccezioni sono più costose dei codici di stato, si parla di ottimizzazione. Abbastanza veloce è abbastanza veloce. No, il vero problema è che gli umani non si aspettano eccezioni per casi tipici. Puoi discutere su ciò che è tipico tutto ciò che ti piace. Per me la grande domanda è quanto leggibile stai usando il codice.
ensureClosed(DoorService service, Door door){
// Need door closed and unlocked. No idea of its state. What to do?
try {
service.open(door)
service.close(door)
}
catch( DoorLockedException e ){
//Have no way to unlock the door so give up and die
log(e);
throw new NoOneGaveMeAKeyException(e);
}
catch( DoorAlreadyOpenedException e ){
try {
service.close(door);
}
catch( DoorAlreadyClosedException e ){
//Some multithreaded goof has been messing with our door.
//Oh well, this is what we wanted anyway.
//Hope they didn't lock it.
}
}
}
Per favore non farmi scrivere codice come questo. Per favore forniscici metodi isLocked()
e isClosed()
. Con quelli posso scrivere i miei metodi ensureClosed()
e ensureUnlocked()
che sono facili da leggere. Quelli che si lanciano solo se le loro condizioni postali sono violate. Preferirei solo che tu abbia già scritto e testato, naturalmente. Basta non mescolarli con quelli che si lanciano quando non possono cambiare lo stato. Perlomeno dai loro nomi distintivi.
Qualunque cosa tu faccia, per favore non chiamare nulla tryClose()
. È un nome terribile.
Per quanto riguarda DoorLockedException
solo vs anche avendo DoorAlreadyLockedException
ammesso questo: si tratta di utilizzare il codice. Si prega di non progettare servizi come questo senza scrivere il codice usando e guardando il casino che stai creando. Refactoring e riprogettazione fino a quando il codice using è almeno leggibile. In effetti, considera prima di scrivere il codice usando.
ensureClosed(DoorService service, Door door){
if( !service.isClosed(door) ){
try{
service.close(door);
}
catch( DoorAlreadyClosedException e ){
//Some multithreaded goof has been messing with our door.
//Oh well, this is what we wanted anyway.
//Hope they didn't lock it.
}
} else {
//This is what you wanted, so quietly do nothing.
//Why are you even here? Who bothers to write empty else conditions?
}
}
ensureUnlocked(DoorService service, Door door){
if( service.islocked(door) ){
throw new NoOneGaveMeAKeyException();
}
}