Ist das Werfen einer Ausnahme hier ein Anti-Pattern?

Ich hatte gerade eine Diskussion über eine Designauswahl nach einer Codeüberprüfung. Ich frage mich, was deine Meinung ist.

Es gibt diese PreferencesKlasse, die ein Bucket für Schlüssel-Wert-Paare ist. Nullwerte sind legal (das ist wichtig). Wir gehen davon aus, dass bestimmte Werte möglicherweise noch nicht gespeichert wurden, und möchten diese Fälle automatisch behandeln, indem wir sie auf Anforderung mit einem vordefinierten Standardwert initialisieren.

Die besprochene Lösung verwendete folgendes Muster (HINWEIS: Dies ist offensichtlich nicht der eigentliche Code - es wird zur Veranschaulichung vereinfacht):

public class Preferences {
    // null values are legal
    private Map<String, String> valuesFromDatabase;

    private static Map<String, String> defaultValues;

    class KeyNotFoundException extends Exception {
    }

    public String getByKey(String key) {
        try {
            return getValueByKey(key);
        } catch (KeyNotFoundException e) {
            String defaultValue = defaultValues.get(key);
            valuesFromDatabase.put(key, defaultvalue);
            return defaultValue;
        }
    }

    private String getValueByKey(String key) throws KeyNotFoundException {
        if (valuesFromDatabase.containsKey(key)) {
            return valuesFromDatabase.get(key);
        } else {
            throw new KeyNotFoundException();
        }
    }
}

Es wurde als Anti-Pattern kritisiert - Missbrauch von Ausnahmen, um den Fluss zu kontrollieren . KeyNotFoundException- nur für diesen einen Anwendungsfall zum Leben erweckt - wird niemals aus dem Rahmen dieser Klasse herausgesehen.

Es sind im Wesentlichen zwei Methoden, die damit spielen, nur um etwas miteinander zu kommunizieren.

Der Schlüssel, der nicht in der Datenbank vorhanden ist, ist nicht alarmierend oder außergewöhnlich. Dies wird erwartet, wenn eine neue Voreinstellung hinzugefügt wird, daher der Mechanismus, der ihn bei Bedarf ordnungsgemäß mit einem Standardwert initialisiert.

Das Gegenargument war, dass getValueByKeydie private Methode, wie sie derzeit definiert ist, keine natürliche Möglichkeit bietet, die öffentliche Methode sowohl über den Wert als auch darüber zu informieren , ob der Schlüssel vorhanden ist. (Wenn dies nicht der Fall ist, muss es hinzugefügt werden, damit der Wert aktualisiert werden kann.)

Die Rückgabe nullwäre zweideutig, da nulles sich um einen vollkommen legalen Wert handelt. Es lässt sich also nicht sagen, ob der Schlüssel nicht vorhanden war oder ob ein Schlüssel vorhanden war null.

getValueByKeyTuple<Boolean, String>Müsste eine Art von a zurückgeben , das Bool auf true setzen, wenn der Schlüssel bereits vorhanden ist, damit wir zwischen (true, null)und unterscheiden können (false, null). (Ein outParameter könnte in C # verwendet werden, aber das ist Java).

Ist das eine schönere Alternative? Ja, Sie müssten eine Klasse für den einmaligen Gebrauch definieren Tuple<Boolean, String>, aber dann werden wir sie los KeyNotFoundException, damit diese Art von Ausgleich entsteht. Wir vermeiden auch den Aufwand für die Behandlung von Ausnahmen, obwohl dies praktisch nicht von Bedeutung ist. Es gibt keine nennenswerten Leistungsaspekte, es handelt sich um eine Client-App und es ist nicht so, als würden Benutzereinstellungen millionenfach pro Sekunde abgerufen.

Eine Variation dieses Ansatzes könnte Guavas verwenden Optional<String>(Guava wird bereits im gesamten Projekt verwendet), anstatt irgendeiner Gewohnheit Tuple<Boolean, String>, und dann können wir zwischen Optional.<String>absent()und "richtig" unterscheiden null. Es fühlt sich jedoch aus Gründen, die leicht zu erkennen sind, immer noch hackisch an - die Einführung von zwei Ebenen der "Nullheit" scheint das Konzept zu missbrauchen, das hinter der Schaffung von Optionals an erster Stelle stand.

Eine andere Möglichkeit wäre, explizit zu prüfen, ob der Schlüssel existiert (fügen Sie eine boolean containsKey(String key)Methode hinzu und rufen Sie sie getValueByKeynur auf, wenn wir bereits bestätigt haben, dass sie existiert).

Schließlich könnte man auch die private Methode einbinden, aber die eigentliche getByKeyist etwas komplexer als mein Codebeispiel, so dass das Einbetten ziemlich unangenehm aussehen würde.

Ich mag hier Haare spalten, aber ich bin neugierig, worauf Sie wetten würden, um in diesem Fall der besten Praxis am nächsten zu sein. Ich habe in den Styleguides von Oracle oder Google keine Antwort gefunden.

Ist die Verwendung von Ausnahmen wie im Codebeispiel ein Anti-Pattern, oder ist es akzeptabel, da Alternativen auch nicht sehr sauber sind? Wenn ja, unter welchen Umständen wäre das in Ordnung? Und umgekehrt?

Ja, Ihr Kollege hat Recht: Das ist ein schlechter Code. Wenn ein Fehler lokal behoben werden kann, sollte er sofort behoben werden. Eine Ausnahme sollte nicht ausgelöst und dann sofort behandelt werden.

Dies ist viel sauberer als Ihre Version (die getValueByKey()Methode wurde entfernt):

public String getByKey(String key) {
    if (valuesFromDatabase.containsKey(key)) {
        return valuesFromDatabase.get(key);
    } else {
        String defaultValue = defaultValues.get(key);
        valuesFromDatabase.put(key, defaultvalue);
        return defaultValue;
    }
}

Eine Ausnahme sollte nur ausgelöst werden, wenn Sie nicht wissen, wie Sie den Fehler lokal beheben können.

Ich würde diese Verwendung von Ausnahmen nicht als Antimuster bezeichnen, nur nicht die beste Lösung für das Problem, ein komplexes Ergebnis zu kommunizieren.

Die beste Lösung (vorausgesetzt, Sie arbeiten noch mit Java 7) ist die Verwendung von Guava's Optional. Ich bin nicht einverstanden, dass es in diesem Fall hackisch wäre. Ausgehend von Guavas ausführlicher Erklärung von Optional scheint mir dies ein perfektes Beispiel für die Verwendung zu sein. Sie unterscheiden zwischen "no value found" und "value of null found".

Da es keine Überlegungen zur Leistung gibt und es sich um ein Implementierungsdetail handelt, spielt es letztendlich keine Rolle, für welche Lösung Sie sich entscheiden. Aber ich muss zustimmen, dass es ein schlechter Stil ist. wobei der Schlüssel fehlt etwas , dass Sie wissen , wird passieren, und Sie es sogar mehr als einen Anruf auf dem Stapel nicht umgehen, das ist , wo Ausnahmen am nützlichsten sind.

Der Tupel-Ansatz ist ein bisschen hacky, weil das zweite Feld bedeutungslos ist, wenn der Boolesche Wert falsch ist. Zuvor zu prüfen, ob der Schlüssel vorhanden ist, ist albern, da die Karte den Schlüssel zweimal nachschlägt. (Nun, das machst du schon, also in diesem Fall dreimal.) Die OptionalLösung ist perfekt für das Problem. Es mag ein bisschen ironisch erscheinen, ein nullin einem zu speichern Optional, aber wenn der Benutzer das möchte, kann man nicht nein sagen.

Wie von Mike in den Kommentaren bemerkt, gibt es ein Problem damit; Weder Guava noch Java 8 Optionalerlauben das Speichern von nulls. Daher müssten Sie Ihre eigenen Rollen erstellen, die zwar einfach sind, aber eine Menge Boilerplate enthalten. Dies kann zu einem Overkill für etwas führen, das nur einmal intern verwendet wird. Sie können auch den Kartentyp auf ändern, die Map<String, Optional<String>>Handhabung von Optional<Optional<String>>s ist jedoch umständlich.

Ein vernünftiger Kompromiss könnte darin bestehen, die Ausnahme beizubehalten, aber ihre Rolle als "alternative Rückkehr" anzuerkennen. Erstellen Sie eine neue geprüfte Ausnahme mit deaktiviertem Stack-Trace und werfen Sie eine vorab erstellte Instanz davon, die Sie in einer statischen Variablen behalten können. Dies ist billig - möglicherweise so günstig wie eine lokale Niederlassung - und Sie können nicht vergessen, damit umzugehen, sodass das Fehlen eines Stack-Trace kein Problem darstellt.

Es gibt diese Preferences-Klasse, die ein Bucket für Schlüssel-Wert-Paare ist. Nullwerte sind legal (das ist wichtig). Wir gehen davon aus, dass bestimmte Werte möglicherweise noch nicht gespeichert wurden, und möchten diese Fälle automatisch behandeln, indem wir sie auf Anforderung mit einem vordefinierten Standardwert initialisieren.

Das Problem ist genau das. Aber Sie haben die Lösung bereits selbst gepostet:

Eine Variation dieses Ansatzes könnte Guavas Optional (Guava wird bereits im gesamten Projekt verwendet) anstelle eines benutzerdefinierten Tupels verwenden, und dann können wir zwischen Optional.absent () und "richtigen" Null unterscheiden . Es fühlt sich jedoch aus Gründen, die leicht zu erkennen sind, immer noch hackisch an - die Einführung von zwei Ebenen der "Nullheit" scheint das Konzept zu missbrauchen, das ursprünglich für die Schaffung von Optionals verantwortlich war.

Verwenden Sie jedoch nicht null oder Optional . Sie können und sollten Optionalnur verwenden. Für Ihr "hackisches" Gefühl verwenden Sie es einfach verschachtelt, so dass Sie am Ende Optional<Optional<String>>explizit erkennen, dass sich möglicherweise ein Schlüssel in der Datenbank befindet (erste Optionsebene) und dass er einen vordefinierten Wert enthält (zweite Optionsebene).

Dieser Ansatz ist besser als die Verwendung von Ausnahmen und leicht verständlich, solange er Optionalfür Sie nicht neu ist.

Beachten Sie auch, dass Optionaleinige Komfortfunktionen vorhanden sind, sodass Sie nicht die ganze Arbeit selbst erledigen müssen. Diese schließen ein:

• static static <T> Optional<T> fromNullable(T nullableReference)um Ihre Datenbankeingaben in die OptionalTypen umzuwandeln
• abstract T or(T defaultValue) um den Schlüsselwert der inneren Optionsebene abzurufen oder (falls nicht vorhanden) Ihren Standardschlüsselwert abzurufen

Ich weiß, dass ich zu spät zur Party komme, aber auf jeden Fall ähnelt Ihr Anwendungsfall dem, in dem Java auchProperties eine Reihe von Standardeigenschaften definiert, die überprüft werden, wenn von der Instanz kein entsprechender Schlüssel geladen wird.

Sehen Sie sich an, wie die Implementierung durchgeführt wird Properties.getProperty(String)(ab Java 7):

Object oval = super.get(key);
String sval = (oval instanceof String) ? (String)oval : null;
return ((sval == null) && (defaults != null)) ? defaults.getProperty(key) : sval;

Wie Sie zitiert haben, ist es nicht unbedingt erforderlich, "Ausnahmen zu missbrauchen, um den Datenfluss zu steuern".

Ein ähnlicher, aber etwas knapperer Ansatz für die Antwort von @ BЈовић kann auch lauten:

public String getByKey(String key) {
    if (!valuesFromDatabase.containsKey(key)) {
        valuesFromDatabase.put(key, defaultValues.get(key));
    }
    return valuesFromDatabase.get(key);
}

Obwohl ich denke, dass die Antwort von @ Bћовић in Ordnung ist, falls getValueByKeyes nirgendwo anders gebraucht wird, denke ich nicht, dass Ihre Lösung schlecht ist, falls Ihr Programm beide Anwendungsfälle enthält:

• Abruf per Schlüssel mit automatischer Erstellung, falls der Schlüssel vorher nicht existiert, und

• Abrufen ohne diesen Automatismus, ohne irgendetwas in der Datenbank, dem Repository oder der Schlüsselkarte zu getValueByKeyändern (denken Sie daran, öffentlich zu sein, nicht privat)

Wenn dies Ihre Situation ist und die Leistung akzeptabel ist, denke ich, dass Ihre vorgeschlagene Lösung völlig in Ordnung ist. Es hat den Vorteil, dass Duplikationen des Abrufcodes vermieden werden, es ist nicht auf Frameworks von Drittanbietern angewiesen und es ist ziemlich einfach (zumindest in meinen Augen).

Tatsächlich hängt es in einer solchen Situation vom Kontext ab, ob ein fehlender Schlüssel eine "außergewöhnliche" Situation ist oder nicht. Für einen Kontext, in dem es ist, wird so etwas getValueByKeybenötigt. In einem Kontext, in dem eine automatische Schlüsselerstellung erwartet wird, ist es durchaus sinnvoll, eine Methode bereitzustellen, die die bereits verfügbare Funktion wiederverwendet, die Ausnahme verschluckt und ein anderes Fehlerverhalten bietet. Dies kann als Erweiterung oder "Dekorator" interpretiert werden getValueByKey, weniger als eine Funktion, bei der die "Ausnahme für den Kontrollfluss missbraucht wird".

Natürlich gibt es eine dritte Alternative: Erstellen Sie eine dritte, private Methode, die die Tuple<Boolean, String>von Ihnen vorgeschlagene zurückgibt, und verwenden Sie diese Methode sowohl in getValueByKeyals auch wieder getByKey. Für einen komplizierteren Fall ist das in der Tat die bessere Alternative, aber für einen so einfachen Fall, wie er hier gezeigt wird, riecht es nach Überentwicklung, und ich bezweifle, dass der Code auf diese Weise wirklich besser gewartet werden kann. Ich bin hier mit der obersten Antwort von Karl Bielefeldt :

msgstr "Sie sollten sich nicht schlecht fühlen, wenn Sie Ausnahmen verwenden, um Ihren Code zu vereinfachen".

Optionalist die richtige Lösung. Wenn Sie es jedoch vorziehen, gibt es eine Alternative, die sich weniger als "zwei Nullen" anfühlt, betrachten Sie einen Sentinel.

Definieren Sie eine private statische Zeichenfolge keyNotFoundSentinelmit einem Wert new String(""). *

Jetzt kann die private Methode return keyNotFoundSentineleher als throw new KeyNotFoundException(). Die öffentliche Methode kann nach diesem Wert suchen

String rval = getValueByKey(key);
if (rval == keyNotFoundSentinel) { // reference equality, not string.equals
    ... // generate default value
} else {
    return rval;
}

In Wirklichkeit nullhandelt es sich um einen Sonderfall eines Sentinels. Dies ist ein von der Sprache definierter Sentinel-Wert mit einigen besonderen Verhaltensweisen (dh einem genau definierten Verhalten, wenn Sie eine Methode aufrufen null). Es ist einfach so nützlich, einen solchen Sentinel zu haben, wie ihn fast jede Sprache verwendet.

* Wir verwenden Java absichtlich new String("")und nicht nur "", um zu verhindern, dass es den String "interniert". Dies würde ihm den gleichen Verweis geben wie jedem anderen leeren String. Da wir diesen zusätzlichen Schritt ausgeführt haben, ist garantiert, dass es sich bei dem von angegebenen String um keyNotFoundSentineleine eindeutige Instanz handelt, die auf keinen Fall in der Map selbst angezeigt werden darf.

Lernen Sie aus dem Framework, das aus allen Schwachpunkten von Java gelernt hat:

.NET bietet zwei weitaus elegantere Lösungen für dieses Problem:

• Dictionary<TKey, TValue>.TryGetValue(TKey, out TValue)
• Nullable<T>.GetValueOrDefault(T default)

Letzteres ist in Java sehr einfach zu schreiben, ersteres erfordert lediglich eine "starke Referenz" -Hilfeklasse.