Trattamento di eccezione con / senza ricorsione

3

Sono entrato in una discussione con un collega collega su un pezzo di codice. Mentre lui pensa che sia ok, io no. Ma non ho più argomenti di quelli che chiamare lo stesso metodo (ricorsione) in un blocco catch non è una buona pratica.

Quindi ecco uno pseudocodice di ciò di cui stiamo discutendo:

public String someMethod( SomeType param1, SomeType param2 ..., int attempts ){
    int max_attempts = 5;
    try{
        //doSomething here;
        return result;
    }catch (Exception e){
        if ( attempts == max_attempts ){
            throw new RuntimeException(e);
        }

        //log exception
        //sleep for some time

        //here is the part I do not agree.
        return someMethod( param1, param2 ..., attempts+1 );
    }
}

Il mio suggerimento è di avere una struttura come questa:

public String someMethod( SomeType param1, SomeType param2... ) 
                                           throws Exception /*or whatever*/{
     //do something
     return result;
}

public String executeProcess( SomeType param1, SomeType param2 ... ){
    int attempts = 0;
    boolean ok = false;
    String result;
    do{
        try{
            result = someMethod( param1, param2... );
            ok=true;
        }catch( /*specific exceptions here*/ e ){
            attempts++;
            //log exception
            //sleep for some time
        }
    }while( !ok || attempts<5 ); //just to be clear 5 here would be a constant

    if ( attempts >= 5 ){ 
        throw new RuntimeException(e);
    }
    return result;
}

Nella mia soluzione potrebbe persino evolversi in qualcosa di ancora più elegante come una classe esecutore con una lista di processi e priorità, ecc. (ovviamente solo se necessario).

Quindi quale di questi è il miglior modo di agire e quali sarebbero gli argomenti per difenderlo (pro e contro).

Ciò che someMethod fa è creare un HttpURLConnection e inviare una richiesta POST a un webservice che restituirà una stringa JSON che verrà quindi gestita e convertita in un oggetto al di fuori del contesto di questo metodo (la parte di conversione).

Tutta la gestione delle eccezioni sarà basata sulla connessione con il webservice.

Quello che ho dimenticato di menzionare qui è che la sua versione del codice è già in produzione da molto tempo, quindi non ci sono "bug di programmazione". Le uniche eccezioni che sono state lanciate sono quelle relative alla connessione / timeout (qualsiasi cosa) relative al servizio web.

Ho persino parlato con lui per andare al file di log (cronologia) e ottenere tutte le eccezioni specifiche e inserire il blocco catch.

Considerando che le eccezioni che sono state gestite saranno quelle specifiche, quale sarebbe la soluzione migliore?

    
posta Jorge Campos 18.02.2016 - 13:52
fonte

4 risposte

5

Sembra che l'intento sia di provare 5 volte, registrando il problema ogni volta. Se nessuno dei tentativi riesce, rinunciare dopo 5 tentativi. Etichetterò questo scaffold e chiamerò la cosa per provare 5 volte la funzionalità di base .

Diamo un'occhiata alle due versioni con riferimento ad alcune linee guida di codifica da tutto il web. Per semplicità, chiamerò le versioni R per la versione ricorsiva e S per la versione non ricorsiva perché ha il codice separato per lo scaffold e la funzionalità di base.

S è conforme ai principi SOLID di R, in particolare al singolo responsabilità , ma anche in parte al principio Sostituzione di Liskov .

  • singola responsabilità - separare lo scaffold dalla funzionalità principale consente a ciascuno di concentrarsi sulle proprie complessità .; e

  • Sostituzione di Liskov - la separazione delle funzionalità di base consente l'utilizzo di altre funzionalità di base con lo stesso scaffold, come osservi.

La tua versione non ricorsiva incorpora l'intento di codifica più chiaramente e ha meno confusione tra funzionalità di base e codice di eccezione. Questo aiuta con la revisione del codice e la manutenzione. Per migliorarlo, puoi considerare l'utilizzo di un altro dei principi SOLID: inversione di dipendenza . Invece di avere il generico executeProcess dipende dal concreto someMethod , aggiungi un call-back come parametro a executeProcess e chiama executeProcess con (una forma incapsulata di) someMethod . Ciò ti consente di inserire executeProcess in una libreria generica anziché copiare e incollare, modificarlo per ogni nuovo someOtherMethod che potrebbe essere introdotto in seguito.

La confusione della business logic con la gestione delle eccezioni è il primo avviso in questo elenco delle migliori pratiche per la gestione delle eccezioni . Anche il sonno può essere considerato parte della pulizia, forse relegato ad un blocco finale o collocato al di fuori del meccanismo di eccezione del tutto.

Un altro problema con R è che la ricorsione (nel codice che ha generato l'eccezione) si verifica prima che l'eccezione sia completamente gestita.

La tua versione refactored è l'opzione migliore dei due .

    
risposta data 18.02.2016 - 15:35
fonte
4

Entrambi gli esempi di codice sono errati, poiché si attende e si riprova indipendentemente dal tipo di lancio di eccezione. Se la sezione "fai qualcosa" genera un'eccezione a causa di un bug di programmazione o di qualche altra condizione di errore permanente, allora non ha senso riprovare. Dovrebbe solo riprovare in caso di condizioni specifiche in cui ha senso, ad es. timeout da una chiamata di servizio.

Detto questo, la prima versione contiene meno righe di codice e meno complessità. Lo trovo più facile da leggere.

    
risposta data 18.02.2016 - 14:51
fonte
2

C'è un'altra buona ragione per non usare la ricorsione qui, oltre ai punti sopra riportati, che è che se viene raggiunto il limite di tentativi, l'eccezione che viene lanciata avrà una traccia di pila riempita inutilmente, il che potrebbe rendere la comprensione della causa dell'eccezione più difficile.

    
risposta data 19.02.2016 - 22:52
fonte
1

Ho affrontato questo problema una volta, dopo aver scavato nel mio vecchio codice, questo è il "pattern" che ho usato per gestire questo caso:

public String someMethod(SomeType param1, SomeType param2, ...)
{
    for(int attempts = 1; attempts <= maxAttempts; attempts++)
    {
        try
        {
            return doSomeWebServiceCall();
        }
        catch(/*specific exception here*/ e)
        {
            if(attempts == maxAttempts)
            {
                throw new RuntimeException(e);
            }

            //log'n'sleep
        }
    }

    throw new RuntimeException("Unreachable");
}

Non sono particolarmente soddisfatto del RuntimeException "non raggiungibile", ma per il resto il codice sembra abbastanza chiaro (anche dopo anni) per me. L'idea più importante qui è di mantenere la logica relativa alla chiamata del servizio web indipendente da tutte le eccezioni di gestione / retry / sleep.

    
risposta data 19.02.2016 - 07:54
fonte

Leggi altre domande sui tag