Buone pratiche di IllegalStateException

Question

Buone pratiche di IllegalStateException

#1 da (9 voti)
#2 da (4 voti)
#3 da (2 voti)
#4 da (2 voti)

5

So cosa è IllegalStateException , ma mi chiedo se in alcuni casi dovrei effettivamente usarlo o è solo questione di design.

Diciamo che ho una classe Tournament che modella un torneo sportivo e un torneo ha un Schedule , che deve essere calcolato. Quindi, a meno che tu non avvii effettivamente il processo di risoluzione che genera la pianificazione, non hai una pianificazione e cercare di ottenerla prima che questo sia effettivamente accaduto dovrebbe essere trattata in qualche modo. Ho pensato che questo sarebbe stato uno scenario appropriato in cui sarebbe stato lanciato un IllegalStateException : se la pianificazione non è stata ancora calcolata, buttala. Vediamo:

public Tournament {
    private Schedule schedule;

    public boolean solve() {
        boolean solved = solver.solve();
        if (solved)
            schedule = solver.getSolution();
        return solved;
    }

    public Schedule getSchedule() {
        if (schedule == null)
            throw new IllegalStateException("Schedule has not been calculated yet");
        return schedule;
    }

    // This udpates the current schedule with the "next" solution. 
    // Same logic regarding the ISE is applied
    public boolean nextSchedule() {
        if (schedule == null)
            throw new IllegalStateException("Schedule has not been calculated yet");
        boolean hasNextSolution = solver.update();
        if (hasNextSolution)
            schedule = solver.getSolution();
       return hasNextSolution;
    }
}

È ISE usato in modo appropriato qui? Dovrebbero essere apportate modifiche nel design della classe?

Quali sono le migliori pratiche?

design java exceptions

posta dabadaba 06.08.2016 - 22:09

fonte

4 risposte

4

Raccomando di renderlo così è impossibile usare in modo errato un oggetto.

Il modo in cui descrivi il problema è simile a questo:

Widget w = ...;
w.method1();
w.method2();
// Happy happy joy joy!

Tuttavia, è possibile farlo:

Widget w = ...;
w.method2();
// Stimpy you eeediot! You broke the widget!

La soluzione è dividere gli oggetti in più pezzi e creare dipendenze transitorie. L'oggetto A ti dà l'oggetto B, che ti dà l'oggetto C. Non puoi ottenere sia B che C da A, forse nell'ordine sbagliato.

Si tratta essenzialmente del controllo statico (applicato dal compilatore) per applicare il disaccoppiamento temporale.

Questa era la soluzione generale - nel tuo esempio specifico, vorrei semplicemente rendere Schedule un argomento costruttore richiesto a Tournament . Se esiste una dipendenza circolare (non chiara dal codice), creo del generatore o classe di build che raccoglie i dati necessari dati e assicura che gli oggetti siano completamente costruiti e inizializzati prima di renderli disponibili per l'uso.

Per ulteriori informazioni, vedi questa mia risposta a una domanda simile.

risposta data 06.08.2016 - 22:26

fonte

2

La migliore pratica è quella di non consentire all'oggetto di avere uno stato illegale.

Certo, questo significa che non puoi mai lanciare questa eccezione, ma solo perché esiste non significa che usarla sia una buona cosa.

Ora certo che una persona intelligente può probabilmente inventare un caso che richiede il suo uso ma la mia politica qui è "evita quando è possibile".

Quindi come dovresti riprogettare questo?

Prova il caching. getSchedual () non tenta nemmeno di creare una pianificazione. Ti punisce solo per averlo chiesto prima che esista. Se esiste bene, restituiscilo. Se non lo fa, perché non provare almeno a crearlo?

Se solver.solve () è il figlio di qualcun altro, io capisco ma se è il tuo perché stai mescolando i booleani degli errori di stile c con le eccezioni di stile OOP? Scegli uno stile e seguilo.

Un ultimo nitpick. È quasi sempre meglio dire a un oggetto comportamentale di fare qualcosa piuttosto che chiedergli cosa sta succedendo. Quando uso il torneo, perché devo sapere del programma? Perché non posso semplicemente dirlo di stampare chi ha vinto?

risposta data 06.08.2016 - 23:05

fonte

2

Penso che l'utilizzo che stai facendo di IllegalStateException sia OK.
A volte non puoi evitare di avere un accoppiamento temporale. Prendiamo ad esempio la classe Matcher nel JDK di Java. Genera questa eccezione se cerchi di trovare una corrispondenza su un modello inesistente.
Alcuni potrebbero obiettare che dovresti restituire una pianificazione nullo e lasciare che il codice del client gestisca NullPointerException o controllarlo per null e agire di conseguenza, ma penso che sarebbe contro la progettazione che per me è molto simile alla% JavaMatcher / Pattern classi che usano espressioni regolari per eseguire operazioni di corrispondenza su una sequenza di caratteri.

Raccomando però di aggiungere un metodo boolean hasSchedule() per aiutare il codice client a evitare l'eccezione. Ma penso che sarebbe una cosa piacevole e non qualcosa di veramente necessario.

Qui troverai alcuni usi legittimi di IllegalStateException :

link

risposta data 06.08.2016 - 23:32

fonte

Leggi altre domande sui tag design java exceptions

Il chiamare un metodo protetto virtuale da una classe base viola l'LSP? Test delle unità più forti Scopri molti bug, cosa fare? Validazioni?

score 9 · Accepted Answer

Allo stato attuale, la tua classe è fondamentalmente un iterable.

getSchedule() è più o meno next()
nextSchedule() è più o meno hasNext()
solve è una combinazione hasNext() e iterator()

Un modello di utilizzo dell'iteratore standard è:

Iterator iter = item.iterator();
while(iter.hasNext()) {
  doSomething(iter.next())
}

Il tuo modello di utilizzo è:

if (item.solve()) {
   doSomething(item.getSchedule());
   while (item.nextSchedule()) {
       doSomething(item.getSchedule());
   }
}

Quindi, in sostanza, la tua classe è una versione povera di un iterabile. La soluzione: usa un iterable.

class Tournament {
    Iterable<Schedule> calculateSchedules();
}

Ora utilizziamo un'interfaccia standard che tutti conoscono e che possono essere utilizzati con codice generico.