Alternativen zu verschachtelten Try-Catches für Fallbacks

14

Ich habe eine Situation, in der ich versuche, ein Objekt abzurufen. Wenn die Suche fehlschlägt, sind mehrere Fallbacks vorhanden, von denen jedes fehlschlagen kann. Der Code sieht also so aus:

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);
        }
    }
}

Das sieht furchtbar hässlich aus. Ich hasse es, null zurückzugeben, aber ist das in dieser Situation besser?

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;

Gibt es noch andere Alternativen?

Ist die Verwendung verschachtelter Try-Catch-Blöcke ein Anti-Pattern? ist verwandt, aber die Antworten lauten "manchmal, aber normalerweise vermeidbar", ohne zu sagen, wann oder wie man es vermeidet.

Alex Wittig
quelle
1
Ist das NotFoundExceptioneigentlich etwas Außergewöhnliches?
Ich weiß es nicht, und deshalb habe ich wahrscheinlich Probleme. Dies geschieht in einem E-Commerce-Kontext, in dem Produkte täglich eingestellt werden. Wenn jemand ein Produkt mit einem Lesezeichen markiert, das anschließend eingestellt wird, und dann versucht, das Lesezeichen zu öffnen, ist das eine Ausnahme?
Alex Wittig
@FiveNine meiner Meinung nach definitiv nein - das ist zu erwarten. Siehe stackoverflow.com/questions/729379/…
Konrad Morawski

Antworten:

17

Der übliche Weg, das Verschachteln zu eliminieren, besteht in der Verwendung von Funktionen:

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);
    }
}

Wenn diese Fallback-Regeln universell sind, können Sie sie direkt im repositoryObjekt implementieren , wobei Sie möglicherweise ifanstelle einer Ausnahme nur einfache Anweisungen verwenden können.

Karl Bielefeldt
quelle
1
In diesem Zusammenhang methodwäre ein besseres Wort als function.
Sulthan
12

Dies wäre mit so etwas wie einer Option Monade wirklich einfach. Leider hat Java diese nicht. In Scala würde ich den TryTyp verwenden , um die erste erfolgreiche Lösung zu finden.

In meiner Einstellung zur funktionalen Programmierung habe ich eine Liste von Rückrufen erstellt, die die verschiedenen möglichen Quellen darstellen, und diese durchlaufen, bis wir die erste erfolgreiche finden:

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;

Dies kann nur empfohlen werden, wenn Sie wirklich eine große Anzahl von Quellen haben oder wenn Sie die Quellen zur Laufzeit konfigurieren müssen. Andernfalls ist dies eine unnötige Abstraktion, und Sie würden mehr davon profitieren, wenn Sie Ihren Code einfach und dumm halten und nur diese hässlichen, verschachtelten Versuchsfänge verwenden würden.

amon
quelle
+1 für die Erwähnung des TryTyps in Scala, für die Erwähnung von Monaden und für die Lösung mit einer Schleife.
Giorgio
Wenn ich schon auf Java 8 wäre, würde ich das machen, aber wie Sie sagen, ist es ein bisschen viel für ein paar Fallbacks.
Alex Wittig
1
Zum Zeitpunkt der Veröffentlichung dieser Antwort war Java 8 mit Unterstützung für die OptionalMonade ( Proof ) bereits freigegeben.
mkalkov
3

Wenn Sie damit rechnen, dass viele dieser Repository-Aufrufe ausgelöst werden NotFoundException, können Sie eine Umhüllung des Repositorys verwenden, um den Code zu optimieren. Ich würde dies nicht für den normalen Betrieb empfehlen, wohlgemerkt:

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...

}
Rory Hunter
quelle
3

Auf @ amons Vorschlag folgt eine monadischere Antwort. Es ist eine sehr heruntergekommene Version, in der Sie einige Annahmen akzeptieren müssen:

  • Die Funktion "unit" oder "return" ist der Klassenkonstruktor

  • Die "Bind" -Operation findet zur Kompilierungszeit statt und ist daher vor dem Aufruf verborgen

  • Die "Aktions" -Funktionen sind auch zur Kompilierungszeit an die Klasse gebunden

  • Obwohl die Klasse generisch ist und jede beliebige Klasse E einschließt, denke ich, dass das in diesem Fall tatsächlich übertrieben ist. Aber ich habe es so belassen, als Beispiel dafür, was Sie tun könnten.

Mit diesen Überlegungen wird die Monade in eine fließende Wrapper-Klasse übersetzt (obwohl Sie einen Großteil der Flexibilität aufgeben, die Sie in einer rein funktionalen Sprache erhalten würden):

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;
    }
}

(dies wird nicht kompiliert ... bestimmte Details bleiben unvollendet, um das Sample klein zu halten)

Und der Aufruf würde so aussehen:

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());

Beachten Sie, dass Sie die Flexibilität haben, die "Abruf" -Operationen nach Ihren Wünschen zusammenzustellen. Es wird angehalten, wenn eine andere Antwort oder Ausnahme als "Nicht gefunden" angezeigt wird.

Ich habe das sehr schnell gemacht; Es ist nicht ganz richtig, aber hoffentlich vermittelt die Idee

rauben
quelle
1
repository.fetchElement().fetchParentElement().fetchSimilarElement();- Meiner Meinung nach: böser Code (im Sinne von Jon Skeet)
Konrad Morawski
Einige Leute mögen diesen Stil nicht, aber es return thisgibt ihn schon seit langer Zeit, um verkettete Objektaufrufe zu erstellen. Da es sich bei OO um veränderbare Objekte handelt, return thisist dies mehr oder weniger gleichbedeutend mit return nullohne Verkettung. Allerdings return new Thing<E>öffnet die Tür zu einer anderen Fähigkeit , dass dieses Beispiel nicht in sich gehen, so dass es für dieses Muster ist wichtig, wenn Sie diesen Weg gehen wählen.
Rob
1
Aber ich mag diesen Stil und bin nicht dagegen, Anrufe oder fließende Schnittstellen als solche zu verketten. Es gibt jedoch einen Unterschied zwischen CustomerBuilder.withName("Steve").withID(403)und diesem Code, da nur aus der Sicht .fetchElement().fetchParentElement().fetchSimilarElement()nicht klar ist, was passiert, und das ist hier der Schlüssel. Werden sie alle abgeholt? Es ist in diesem Fall nicht akkumulativ und daher auch nicht so intuitiv. Das muss ich erst sehen, if (answer != null) return thisbevor ich es wirklich verstehe. Vielleicht ist es nur eine Frage der richtigen Benennung ( orFetchParent), aber es ist trotzdem "Magie".
Konrad Morawski
1
Übrigens (ich weiß, dass Ihr Code zu stark vereinfacht und nur ein Proof of Concept ist), wäre es gut, einen Klon von answerin zurückzugeben getAnswerund das answerFeld selbst zurückzusetzen (zu löschen), bevor der Wert zurückgegeben wird. Andernfalls wird das Prinzip der Trennung von Befehlen und Abfragen verletzt, da das Abrufen eines Elements (Abfragen) den Status Ihres Repository-Objekts ändert ( answerniemals zurückgesetzt wird) und das Verhalten beeinflusst, fetchElementwenn Sie es das nächste Mal aufrufen. Ja, ich nicke ein bisschen, ich denke, die Antwort ist gültig, ich war nicht derjenige, der sie abgelehnt hat.
Konrad Morawski
1
das ist ein guter Punkt. Ein anderer Weg wäre "attemptToFetch ...". Der wichtige Punkt ist, dass in diesem Fall alle 3 Methoden aufgerufen werden, in einem anderen Fall jedoch ein Client möglicherweise "attemptFetch (). AttemptFetchParent ()" verwendet. Außerdem ist die Bezeichnung "Repository" falsch, da hier wirklich ein einzelner Abruf modelliert wird. Vielleicht werde ich mit den Namen fummeln, um es "RepositoryLookup" zu nennen, und "versuchen", klar zu machen, dass es sich um ein vorübergehendes One-Shot-Artefakt handelt, das eine bestimmte Semantik für eine Suche bietet.
Rob
2

Eine andere Möglichkeit, eine Reihe von Bedingungen wie diese zu strukturieren, besteht darin, ein Flag zu führen oder auf Null zu testen (besser noch: Verwenden Sie Guavas Optional, um festzustellen, wann eine gute Antwort vorliegt), um die Bedingungen miteinander zu verketten.

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;

Auf diese Weise beobachten Sie den Status des Elements und führen die richtigen Aufrufe basierend auf dem Status durch - das heißt, solange Sie noch keine Antwort haben.

(Ich stimme jedoch mit @amon überein. Ich würde empfehlen, ein Monad-Muster mit einem solchen Wrapper-Objekt class Repository<E>mit Mitgliedern E answer;und zu betrachten Exception error;. Überprüfen Sie in jeder Phase, ob es eine Ausnahme gibt, und überspringen Sie in diesem Fall jeden verbleibenden Schritt Am Ende haben Sie entweder eine Antwort, keine Antwort oder eine Ausnahme und Sie können entscheiden, was Sie damit tun möchten.)

rauben
quelle
-2

Zunächst scheint es mir, dass es eine Funktion geben sollte, wie repository.getMostSimilar(x) (Sie sollten einen passenderen Namen wählen) geben sollte, da es eine Logik zu geben scheint, die verwendet wird, um das nächste oder ähnlichste Element für ein bestimmtes Element zu finden.

Das Repository kann dann die in amons post gezeigte Logik implementieren. Dies bedeutet, dass nur dann eine Ausnahme ausgelöst werden muss, wenn kein einzelnes Element gefunden werden konnte.

Dies ist jedoch natürlich nur möglich, wenn die Logik zum Auffinden des nächstgelegenen Elements in das Repository eingekapselt werden kann. Ist dies nicht möglich, geben Sie bitte an, wie (nach welchen Kriterien) das nächstgelegene Element ausgewählt werden kann.

valenterry
quelle
Antworten sind auf die Frage zu beantworten, nicht um Klärung zu bitten
Mücke
Nun, meine Antwort ist das Lösen seines Problems, da es einen Weg zeigt, die verschachtelten Versuche / Fänge unter bestimmten Bedingungen zu vermeiden. Nur wenn diese Bedingungen nicht erfüllt sind, benötigen wir weitere Informationen. Warum ist das keine gültige Antwort?
Valentinstag