Ausnahme auslösen oder Code fehlschlagen lassen

Ich frage mich, ob es gegen diesen Stil Vor- und Nachteile gibt:

private void LoadMaterial(string name)
{
    if (_Materials.ContainsKey(name))
    {
        throw new ArgumentException("The material named " + name + " has already been loaded.");
    }

    _Materials.Add(
        name,
        Resources.Load(string.Format("Materials/{0}", name)) as Material
    );
}

Diese Methode sollte jeweils namenur einmal ausgeführt werden. _Materials.Add()wird eine Ausnahme auslösen, wenn es mehrmals für das gleiche aufgerufen wird name. Ist meine Wache daher völlig überflüssig, oder gibt es einige weniger offensichtliche Vorteile?

Das ist C #, Unity, wenn sich jemand dafür interessiert.

Der Vorteil ist, dass Ihre "benutzerdefinierte" Ausnahme eine Fehlermeldung enthält, die für jeden, der diese Funktion aufruft, von Bedeutung ist, ohne zu wissen, wie sie implementiert ist (welche in Zukunft Sie sein könnten!).

Zugegeben, in diesem Fall könnten sie wahrscheinlich erraten, was die "Standard" -Ausnahme bedeutete, aber Sie machen trotzdem deutlich, dass sie gegen Ihren Vertrag verstoßen haben, anstatt auf einen seltsamen Fehler in Ihrem Code gestoßen zu sein.

Ich stimme der Antwort von Ixrec zu . Möglicherweise möchten Sie jedoch eine dritte Alternative in Betracht ziehen: die Funktion idempotent machen. Mit anderen Worten, kehren Sie früh zurück, anstatt eine zu werfen ArgumentException. Dies ist häufig vorzuziehen, wenn Sie andernfalls vor LoadMaterialjedem Anruf überprüfen müssten, ob es bereits geladen wurde. Je weniger Voraussetzungen Sie haben, desto geringer ist die kognitive Belastung des Programmiergeräts.

Das Auslösen einer Ausnahme ist vorzuziehen, wenn es sich wirklich um einen Programmiererfehler handelt, bei dem es zur Kompilierungszeit offensichtlich und bekannt sein sollte, ob das Material bereits geladen wurde, ohne dass dies vor dem Aufruf zur Laufzeit überprüft werden muss.

Die grundlegende Frage, die Sie sich stellen müssen, lautet: Wie soll die Schnittstelle für Ihre Funktion aussehen? Ist der Zustand insbesondere _Materials.ContainsKey(name)eine Voraussetzung für die Funktion?

Wenn dies keine Voraussetzung ist, muss die Funktion ein genau definiertes Ergebnis für alle möglichen Werte von liefern name. In diesem Fall wird die Ausnahme, die ausgelöst wird, wenn sie namenicht Teil von ist, _MaterialsTeil der Schnittstelle der Funktion. Das bedeutet, dass es Teil der Schnittstellendokumentation sein muss. Wenn Sie sich jemals dazu entschließen, diese Ausnahme in Zukunft zu ändern, ist dies eine brechende Schnittstellenänderung .

Die interessantere Frage ist, was passiert, wenn dies eine Voraussetzung ist. In diesem Fall wird die Vorbedingung selbst Teil der Funktionsschnittstelle, aber die Art und Weise, wie sich eine Funktion verhält, wenn diese Bedingung verletzt wird, ist nicht unbedingt Teil der Schnittstelle.

In diesem Fall handelt es sich bei dem von Ihnen veröffentlichten Ansatz, nach Verstößen gegen die Voraussetzungen zu suchen und den Fehler zu melden, um eine sogenannte defensive Programmierung . Defensive Programmierung ist insofern von Vorteil, als sie den Benutzer frühzeitig benachrichtigt, wenn er einen Fehler gemacht und die Funktion mit einem falschen Argument aufgerufen hat. Das ist insofern schlimm, als es den Wartungsaufwand erheblich erhöht, da der Benutzercode davon abhängen kann, dass die Funktion auf bestimmte Weise mit Verstößen gegen die Voraussetzungen umgeht. Insbesondere wenn Sie feststellen, dass die Laufzeitüberprüfung in Zukunft jemals zu einem Leistungsengpass wird (unwahrscheinlich für einen so einfachen Fall, aber für komplexere Voraussetzungen durchaus üblich), können Sie sie möglicherweise nicht mehr entfernen.

Es stellt sich heraus, dass diese Nachteile sehr bedeutend sein können, was der defensiven Programmierung in bestimmten Kreisen einen schlechten Ruf verlieh. Das ursprüngliche Ziel ist jedoch weiterhin gültig: Wir möchten, dass Benutzer unserer Funktion frühzeitig bemerken, wenn sie einen Fehler gemacht haben.

Daher schlagen heutzutage viele Entwickler einen etwas anderen Ansatz für diese Art von Problemen vor. Anstatt eine Ausnahme auszulösen, verwenden sie einen assert-ähnlichen Mechanismus zum Überprüfen der Voraussetzungen. Das heißt, in Debugbuilds werden möglicherweise Vorbedingungen überprüft, um Benutzern das frühzeitige Erkennen von Fehlern zu erleichtern, sie sind jedoch nicht Teil der Funktionsschnittstelle. Der Unterschied mag auf den ersten Blick subtil erscheinen, kann aber in der Praxis einen großen Unterschied ausmachen.

Technisch gesehen ist der Aufruf einer Funktion mit verletzten Voraussetzungen undefiniertes Verhalten. Die Implementierung kann jedoch entscheiden, diese Fälle zu erkennen und den Benutzer in diesem Fall unverzüglich zu benachrichtigen. Ausnahmen sind leider kein gutes Werkzeug, um dies zu implementieren, da der Benutzercode auf sie reagieren und sich daher möglicherweise auf ihre Anwesenheit verlassen kann.

Eine ausführliche Erläuterung der Probleme mit dem klassischen defensiven Ansatz sowie eine mögliche Implementierung für Voraussetzungsprüfungen im Assert -Stil finden Sie in John Lakos Vortrag Defensive Programming Done Right from CppCon 2014 ( Folien , Video ).

Hier gibt es bereits einige Antworten, aber hier ist eine Antwort, die Unity3D berücksichtigt (die Antwort ist sehr spezifisch für Unity3D, ich würde das meiste in den meisten Zusammenhängen ganz anders machen):

Im Allgemeinen verwendet Unity3D traditionell keine Ausnahmen. Wenn Sie in Unity3D eine Ausnahme auslösen, ist dies nicht vergleichbar mit einer durchschnittlichen .NET-Anwendung. Das Programm wird nicht gestoppt. Sie können den Editor jedoch so konfigurieren, dass er pausiert. Es wird nur protokolliert. Dies kann das Spiel leicht in einen ungültigen Zustand versetzen und einen Kaskadeneffekt erzeugen, der es schwierig macht, Fehler aufzuspüren. Daher würde ich im Falle von Unity sagen, dass Addes besonders unerwünscht ist, eine Ausnahme auslösen zu lassen.

Bei der Prüfung der Ausnahmegeschwindigkeit kommt es jedoch in einigen Fällen nicht zu einer vorzeitigen Optimierung, da Mono auf einigen Plattformen in Unity funktioniert. Tatsächlich unterstützt Unity3D unter iOS einige erweiterte Skriptoptimierungen, und deaktivierte Ausnahmen * sind ein Nebeneffekt einer davon. Dies ist wirklich zu berücksichtigen, da sich diese Optimierungen für viele Benutzer als sehr wertvoll erwiesen haben und einen realistischen Fall für die Einschränkung der Verwendung von Ausnahmen in Unity3D darstellen. (* verwaltete Ausnahmen von der Engine, nicht Ihr Code)

Ich würde sagen, dass Sie in Unity einen spezielleren Ansatz wählen möchten. Ironischerweise zeigt eine sehr heruntergestimmte Antwort zum Zeitpunkt des Schreibens dieses Artikels , wie ich so etwas speziell im Kontext von Unity3D implementieren könnte (ansonsten ist so etwas wirklich inakzeptabel und selbst in Unity ist es ziemlich inelegant).

Ein anderer Ansatz, den ich in Betracht ziehen würde, besteht darin, dem Anrufer keinen Fehler anzuzeigen, sondern die Debug.LogXXFunktionen zu verwenden. Auf diese Weise erhalten Sie dasselbe Verhalten wie beim Auslösen einer nicht behandelten Ausnahme (aufgrund der Art und Weise, wie Unity3D sie behandelt), ohne das Risiko einzugehen, dass etwas auf der ganzen Linie in einen seltsamen Zustand versetzt wird. Überlegen Sie auch, ob dies wirklich ein Fehler ist (Ist der Versuch, dasselbe Material zweimal einzulegen, in Ihrem Fall unbedingt ein Fehler? Oder ist dies möglicherweise ein Fall, in dem dies Debug.LogWarningzutreffender ist).

Und in Bezug auf die Verwendung von Debug.LogXXFunktionen anstelle von Ausnahmen müssen Sie immer noch überlegen, was passiert, wenn eine Ausnahme von etwas ausgelöst wird, das einen Wert zurückgibt (wie z. B. GetMaterial). Ich tendiere dazu, dies zu erreichen, indem ich Null zusammen mit der Protokollierung des Fehlers übergebe ( wieder nur in Unity ). Ich verwende dann Nullprüfungen in meinen MonoBehaviors, um sicherzustellen, dass eine Abhängigkeit wie ein Material kein Nullwert ist, und deaktiviere das MonoBehavior, falls dies der Fall ist. Ein Beispiel für ein einfaches Verhalten, für das einige Abhängigkeiten erforderlich sind, ist etwa Folgendes:

    public void Awake()
    {
        _inputParameters = GetComponent<VehicleInputParameters>();
        _rigidbody = GetComponent<Rigidbody>();
        _rigidbodyTransform = _rigidbody.transform;
        _raycastStrategySelector = GetComponent<RaycastStrategySelectionBehavior>();

        _trackParameters =
            SceneManager.InstanceOf.CurrentSceneData.GetValue<TrackParameters>();

        this.DisableIfNull(() => _rigidbody);
        this.DisableIfNull(() => _raycastStrategySelector);
        this.DisableIfNull(() => _inputParameters);
        this.DisableIfNull(() => _trackParameters);
    }

SceneData.GetValue<>ähnelt Ihrem Beispiel darin, dass es eine Funktion in einem Wörterbuch aufruft, die eine Ausnahme auslöst. Anstatt jedoch eine Ausnahme auszulösen Debug.LogError, wird eine Stapelablaufverfolgung wie bei einer normalen Ausnahme erstellt und null zurückgegeben. Die folgenden Überprüfungen * deaktivieren das Verhalten, anstatt zuzulassen, dass es weiterhin in einem ungültigen Zustand existiert.

* Die Schecks sehen so aus, weil ich einen kleinen Helfer verwende, der eine formatierte Nachricht druckt, wenn er das Spielobjekt deaktiviert **. Einfache Null-Checks mit ifArbeit hier (** Die Checks des Helfers werden nur in Debug-Builds kompiliert (wie bei Asserts). Die Verwendung von Lambdas und solchen Ausdrücken in Unity kann die Leistung beeinträchtigen.)

Ich mag die beiden Hauptantworten, möchte aber vorschlagen, dass Ihre Funktionsnamen verbessert werden könnten. Ich bin an Java gewöhnt, also YMMV, aber eine "Add" -Methode sollte, IMO, keine Ausnahme auslösen, wenn das Element bereits vorhanden ist. Es sollte das Element erneut hinzufügen oder nichts tun, wenn das Ziel ein Set ist. Da dies nicht das Verhalten von Materials.Add ist, sollte das in TryPut oder AddOnce oder AddOrThrow oder ähnliches umbenannt werden.

Ebenso sollte Ihr LoadMaterial in LoadIfAbsent oder Put oder TryLoad oder LoadOrThrow umbenannt werden (abhängig davon, ob Sie die Antwort 1 oder 2 verwenden) oder in eine solche.

Befolgen Sie die Namenskonventionen von C # Unity.

Dies ist besonders nützlich, wenn Sie über andere AddFoo- und LoadBar-Funktionen verfügen, bei denen es zulässig ist, dasselbe Objekt zweimal zu laden. Ohne eindeutige Namen werden die Entwickler frustriert sein.

Alle Antworten enthalten wertvolle Ideen, die ich gerne kombinieren möchte:

Legen Sie die beabsichtigte und erwartete Semantik der LoadMaterial()Operation fest. Zumindest die folgenden Optionen existieren:

• Voraussetzung für name∉ GeladeneMaterialien : →

  Wenn die Vorbedingung verletzt wird, ist die Wirkung von LoadMaterial()nicht spezifiziert (wie in der Antwort von ComicSansMS ). Dies ermöglicht größtmögliche Freiheit bei der Implementierung und zukünftigen Änderungen von LoadMaterial(). Oder,

• Effekte des Aufrufs LoadMaterial(name)mit name∈ LoadedMaterials werden angegeben; entweder:

  • Die Spezifikation besagt, dass eine Ausnahme ausgelöst wird. oder

  • die Spezifikation besagt, dass das Ergebnis idempotent ist (wie in der Antwort von Karl Bielefeldt ),

    • möglicherweise Zurückgeben eines „hat die Sammlung geändert“ boolean (wie vorgeschlagen von User949300 und vorgeschlagen (in einigen Interpretationen) von Mrlveck .

Wenn Sie sich für die Semantik entschieden haben, müssen Sie eine Implementierung auswählen. Folgende Optionen und Überlegungen wurden vorgeschlagen:

• Werfen Sie eine benutzerdefinierte Ausnahme (wie vorgeschlagen durch Ixrec ) →

  Der Vorteil ist, dass Ihre "benutzerdefinierte" Ausnahme eine Fehlermeldung enthält, die für jeden, der diese Funktion aufruft , von Bedeutung ist - Ixrec und User16547

  • Um die Kosten einer wiederholten Überprüfung von name∉ LoadedMaterials zu vermeiden, können Sie den Anweisungen von Marc van Leeuwen folgen :

    ... Erwägen Sie stattdessen, sich bedingungslos in _Materials.Add zu stürzen, und fangen Sie dann einen möglichen Fehler ab und werfen Sie im Handler einen anderen.

• Lassen Sie Dictionay.Add die Ausnahme auslösen →

  Der Code zum Auslösen von Ausnahmen ist redundant. - Jon Raynor

  Obwohl die meisten Wähler eher mit Ixrec übereinstimmen

  Ein weiterer Grund, sich für diese Implementierung zu entscheiden, sind:

  • dass Anrufer bereits ArgumentExceptionAusnahmen behandeln können, und
  • um zu vermeiden, dass Stapelinformationen verloren gehen.

  Wenn diese beiden Gründe wichtig sind, können Sie Ihre benutzerdefinierte Ausnahme auch aus ArgumentExceptiondem Original ableiten und als verkettete Ausnahme verwenden.

• Machen LoadMaterial()Idempotent als anwser von Karl Bielefeldt und upvoted am häufigsten (75 Mal).

  • möglicherweise Zurückgeben eines „hat die Sammlung geändert“ boolean (wie vorgeschlagen von User949300 und vorgeschlagen (in einigen Interpretationen) von Mrlveck .

  Die Implementierungsoptionen für dieses Verhalten:

  • Überprüfen Sie mit Dictionary.ContainsKey ()

  • Rufen Sie immer Dictionary.Add () auf, um den ArgumentExceptionAuslöser abzufangen, wenn der einzufügende Schlüssel bereits vorhanden ist, und ignorieren Sie diese Ausnahme. Dokumentieren Sie, dass Sie beabsichtigen, die Ausnahme zu ignorieren und warum.

    • Wenn LoadMaterials()(fast) immer einmal für jedes aufgerufen wird name, vermeidet dies die wiederholte Überprüfung der Kosten name∉ LoadedMaterials vgl. Marc van Leeuwen . Jedoch,
    • Wenn LoadedMaterials()oft mehrere Male für dasselbe aufgerufen wird name, entstehen die (teuren) Kosten für das ArgumentExceptionAbwickeln des Stapels.

  • Ich dachte, dass es eine TryAddzu TryGet () analoge Methode gibt, die es Ihnen ermöglicht hätte, das teure Auslösen von Ausnahmen und das Abwickeln von Stapeln fehlgeschlagener Aufrufe von Dictionary.Add zu vermeiden.

    Aber diese TryAddMethode scheint nicht zu existieren.

Der Code zum Auslösen von Ausnahmen ist redundant.

Wenn Sie anrufen:

_Materials.Add (name, Resources.Load (string.Format ("Materials / {0}", name)) als Material

mit dem gleichen Schlüssel wirft es a

System.ArgumentException

Die Meldung lautet: "Ein Objekt mit demselben Schlüssel wurde bereits hinzugefügt."

Die ContainsKeyPrüfung ist redundant, da ohne sie im Wesentlichen das gleiche Verhalten erreicht wird. Das einzige Element, das sich unterscheidet, ist die tatsächliche Nachricht in der Ausnahme. Wenn eine benutzerdefinierte Fehlermeldung einen echten Vorteil bietet, hat der Guard-Code einige Vorteile.

Andernfalls würde ich in diesem Fall die Wache wahrscheinlich umgestalten.

Ich denke, dies ist eher eine Frage zum API-Design als eine Frage zur Kodierungskonvention.

Was ist das erwartete Ergebnis (der Vertrag) des Anrufs:

LoadMaterial("wood");

Wenn der Aufrufer erwarten kann / kann, dass nach dem Aufrufen dieser Methode das Material "Holz" geladen wird, sehe ich keinen Grund, eine Ausnahme auszulösen, wenn dieses Material bereits geladen ist.

Wenn beim Laden des Materials ein Fehler auftritt, z. B. die Datenbankverbindung nicht geöffnet werden konnte oder sich kein Material "Holz" im Repository befindet, ist es richtig, eine Ausnahme auszulösen, um den Anrufer über dieses Problem zu benachrichtigen.

Warum nicht die Methode so ändern, dass sie "wahr" zurückgibt, wenn das Materielle hinzugefügt wird, und "falsch", wenn es nicht so ist?

private bool LoadMaterial(string name)
{
   if (_Materials.ContainsKey(name))

    {

        return false; //already present
    }

    _Materials.Add(
        name,
        Resources.Load(string.Format("Materials/{0}", name)) as Material
    );

    return true;

}