alternative a try-catch nidificati per fallbacks

13

Ho una situazione in cui sto cercando di recuperare un oggetto. Se la ricerca fallisce, sono presenti diversi fallback, ognuno dei quali potrebbe non riuscire. Quindi il codice assomiglia a:

try {
    return repository.getElement(x);
} catch (NotFoundException e) {
    try {
        return repository.getSimilarElement(x);
    } catch (NotFoundException e1) {
        try {
            return repository.getParentElement(x);
        } catch (NotFoundException e2) {
            //can't recover
            throw new IllegalArgumentException(e);
        }
    }
}

Questo sembra terribilmente brutto. Odio restituire null, ma è meglio in questa situazione?

Element e = return repository.getElement(x);
if (e == null) {
    e = repository.getSimilarElement(x);
}
if (e == null) {
    e = repository.getParentElement(x);
}
if (e == null) {
    throw new IllegalArgumentException();
}
return e;

Ci sono altre alternative?

L'uso di blocchi try-catch nidificati è un anti-pattern? è correlato, ma le risposte ci sono le linee di "a volte, ma di solito è evitabile", senza dire quando o come evitarlo.

    
posta Alex Wittig 29.04.2014 - 23:16
fonte

6 risposte

16

Il modo usuale per eliminare il nesting è usare le funzioni:

Element getElement(x) {
    try {
        return repository.getElement(x);
    } catch (NotFoundException e) {
        return fallbackToSimilar(x);
    }  
}

Element fallbackToSimilar(x) {
    try {
        return repository.getSimilarElement(x);
     } catch (NotFoundException e1) {
        return fallbackToParent(x);
     }
}

Element fallbackToParent(x) {
    try {
        return repository.getParentElement(x);
    } catch (NotFoundException e2) {
        throw new IllegalArgumentException(e);
    }
}

Se queste regole di fallback sono universali, potresti prendere in considerazione l'implementazione direttamente nell'oggetto repository , in cui potresti essere in grado di utilizzare solo semplici istruzioni if anziché un'eccezione.

    
risposta data 30.04.2014 - 00:18
fonte
12

Questo sarebbe davvero facile con qualcosa come una monade Option. Sfortunatamente, Java non ne ha. In Scala, utilizzerei il Try type per trovare il prima soluzione di successo.

Nella mia mentalità di programmazione funzionale, avrei creato un elenco di callback che rappresentano le varie fonti possibili e li ho fatti scorrere fino a trovare il primo riuscito:

interface ElementSource {
    public Element get();
}

...

final repository = ...;

// this could be simplified a lot using Java 8's lambdas
List<ElementSource> sources = Arrays.asList(
    new ElementSource() {
        @Override
        public Element get() { return repository.getElement(); }
    },
    new ElementSource() {
        @Override
        public Element get() { return repository.getSimilarElement(); }
    },
    new ElementSource() {
        @Override
        public Element get() { return repository.getParentElement(); }
    }
);

Throwable exception = new NoSuchElementException("no sources set up");
for (ElementSource source : sources) {
    try {
        return source.get();
    } catch (NotFoundException e) {
        exception = e;
    }
}
// we end up here if we didn't already return
// so throw the last exception
throw exception;

Questo può essere raccomandato solo se si dispone di un numero elevato di fonti o se è necessario configurare i sorgenti in fase di runtime. Altrimenti, questa è un'astrazione superflua e trarrai maggior profitto dal mantenere il tuo codice semplice e stupido, e semplicemente usare quei brutti tentativi di cattura annidati.

    
risposta data 29.04.2014 - 23:36
fonte
3

Se anticipi che molte di quelle chiamate di repository genereranno NotFoundException , potresti utilizzare un wrapper attorno al repository per semplificare il codice. Non lo consiglierei per le normali operazioni, attenzione:

public class TolerantRepository implements SomeKindOfRepositoryInterfaceHopefully {

    private Repository repo;

    public TolerantRepository( Repository r ) {
        this.repo = r;
    }

    public SomeType getElement( SomeType x ) {
        try {
            return this.repo.getElement(x);
        }
        catch (NotFoundException e) {
            /* For example */
            return null;
        }
    }

    // and the same for other methods...

}
    
risposta data 30.04.2014 - 12:19
fonte
3

Al suggerimento di @ amon, ecco una risposta più monadica. È una versione molto ridotta, in cui devi accettare alcune ipotesi:

  • la funzione "unit" o "return" è il costruttore della classe

  • l'operazione "bind" avviene in fase di compilazione, quindi è nascosta dall'invocazione

  • anche le funzioni "azione" sono legate alla classe in fase di compilazione

  • sebbene la classe sia generica e avvolga qualsiasi classe arbitraria E, penso che in questo caso sia davvero eccessivo. Ma l'ho lasciato in questo modo come esempio di cosa potresti fare.

Con queste considerazioni, la monade si traduce in una fluente classe wrapper (sebbene tu stia rinunciando alla grande flessibilità che otterresti in un linguaggio puramente funzionale):

public class RepositoryLookup<E> {
    private String source;
    private E answer;
    private Exception exception;

    public RepositoryLookup<E>(String source) {
        this.source = source;
    }

    public RepositoryLookup<E> fetchElement() {
        if (answer != null) return this;
        if (! exception instanceOf NotFoundException) return this;

        try {
            answer = lookup(source);
        }
        catch (Exception e) {
            exception = e;
        }

        return this;
    }

    public RepositoryLookup<E> orFetchSimilarElement() {
        if (answer != null) return this; 
        if (! exception instanceOf NotFoundException) return this;

        try {
            answer = lookupVariation(source);
        }
        catch (Exception e) {
            exception = e;
        }

        return this;
    }

    public RepositoryLookup<E> orFetchParentElement() {
        if (answer != null) return this; 
        if (! exception instanceOf NotFoundException) return this;

        try {
            answer = lookupParent(source);
        }
        catch (Exception e) {
            exception = e;
        }

        return this;
    }

    public boolean failed() {
        return exception != null;
    }

    public Exception getException() {
        return exception;
    }

    public E getAnswer() {
        // better to check failed() explicitly ;)
        if (this.exception != null) {
            throw new IllegalArgumentException(exception);
        }
        // TODO: add a null check here?
        return answer;
    }
}

(questo non verrà compilato ... alcuni dettagli sono lasciati incompiuti per mantenere il campione piccolo)

E l'invocazione sarebbe simile a questa:

Repository<String> repository = new Repository<String>(x);
repository.fetchElement().orFetchParentElement().orFetchSimilarElement();

if (repository.failed()) {
    throw new IllegalArgumentException(repository.getException());
}

System.err.println("Got " + repository.getAnswer());

Nota che hai la flessibilità di comporre le operazioni "fetch" come preferisci. Si fermerà quando otterrà una risposta o un'eccezione diversa da quella non trovata.

L'ho fatto molto velocemente; non è giusto, ma si spera che trasmetta l'idea

    
risposta data 30.04.2014 - 00:52
fonte
2

Un altro modo per strutturare una serie di condizioni come questa è portare un flag, o testare null (meglio ancora, usare Guava's Optional per determinare quando è presente una buona risposta) al fine di concatenare le condizioni.

Element e = null;

try {
    e = repository.getElement(x);
} catch (NotFoundException e) {
    // nope -- try again!
}

if (e == null) {  // or ! optionalElement.isPresent()
    try {
        return repository.getSimilarElement(x);
    } catch (NotFoundException e1) {
        // nope -- try again!
    }
}

if (e == null) {  // or ! optionalElement.isPresent()
    try {
        return repository.getParentElement(x);
    } catch (NotFoundException e2) {
        // nope -- try again!
    }
}

if (e == null) {  // or ! optionalElement.isPresent()
    //can't recover
    throw new IllegalArgumentException(e);
}

return e;

In questo modo, stai osservando lo stato dell'elemento e facendo le chiamate giuste in base al suo stato, cioè fino a quando non hai ancora una risposta.

(Sono d'accordo con @amon, però, ti consiglio di guardare un pattern Monad, con un oggetto wrapper come class Repository<E> che ha membri E answer; e Exception error; . Ad ogni fase controlla se c'è un eccezione, e in tal caso, salta ogni passaggio rimanente. Alla fine, ti rimane una risposta, l'assenza di una risposta o un'eccezione e puoi decidere cosa fare con quello.)

    
risposta data 30.04.2014 - 00:32
fonte
-2

In primo luogo, mi sembra che ci dovrebbe essere una funzione come repository.getMostSimilar(x) (dovresti scegliere un nome più appropriato) in quanto sembra esserci una logica che viene utilizzata per trovare l'elemento più vicino o più simile per un dato elemento.

Il repository può quindi implementare la logica come mostrato negli amons post. Ciò significa che l'unico caso in cui deve essere generata un'eccezione è quando non è possibile trovare un singolo elemento.

Tuttavia questo è ovviamente possibile solo se le logiche per trovare l'elemento più vicino possono essere incapsulate nel repository. Se questo non è possibile, fornisci maggiori informazioni su come (con quali criteri) può essere scelto l'elemento più vicino.

    
risposta data 30.04.2014 - 00:15
fonte

Leggi altre domande sui tag