Wie defensiv sollte ich programmieren? [geschlossen]

Question

i wurde mit einer kleinen Routine arbeiten, die eine Datenbankverbindung erstellen verwendet wird:

Vor

public DbConnection GetConnection(String connectionName)
{
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
   DbConnection conn = factory.CreateConnection();
   conn.ConnectionString = cs.ConnectionString;
   conn.Open();

   return conn;
}

Dann begann ich Blick in die .NET-Framework-Dokumentation, um zu sehen, was die dokumentiert Verhalten verschiedenen Dinge sind, und sehen, ob ich sich verarbeiten kann.

Zum Beispiel:

ConfigurationManager.ConnectionStrings...

Die Dokumentation sagt, dass Aufruf < strong> Connection wirft ein ConfigurationErrorException wenn es könnte die Sammlung nicht abrufen. In diesem Fall gibt es nichts, was ich tun kann, diese Ausnahme zu behandeln, so dass ich lasse es gehen.

---

Der nächste Teil ist die eigentliche Indizierung der Connection finden connection :

...ConnectionStrings[connectionName];

In diesem Fall wird die Connection Dokumentation sagt, dass die Eigenschaft zurückkehren null , wenn der Verbindungsname konnte nicht gefunden werden. Ich kann für dieses Ereignis überprüfen, und eine Ausnahme auslösen jemand hoch nachgelassen, dass sie eine ungültige connection haben:

ConnectionStringSettings cs= 
      ConfigurationManager.ConnectionStrings[connectionName];
if (cs == null)
   throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

---

i wiederholen Sie die gleiche Übung mit:

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);

Die GetFactory Methode hat keine Dokumentation auf was ist, wenn eine Fabrik geschieht für den angegebenen ProviderName konnte nicht gefunden werden. Es ist nicht zurückkehren null dokumentiert, aber ich kann immer noch defensiv, und überprüfen für null:

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);
if (factory == null) 
   throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

---

Als nächstes ist der Bau des DbConnection Objekts:

DbConnection conn = factory.CreateConnection()

Auch die Dokumentation Doesn ‚t sagen, was passiert, wenn es nicht eine Verbindung schaffen könnte, aber auch hier kann ich für ein null-Return-Objekt überprüfen:

DbConnection conn = factory.CreateConnection()
if (conn == null) 
   throw new Exception.Create("Connection factory did not return a connection object");

---

Als nächstes wird eine Eigenschaft des Connection-Objekts festlegen:

conn.ConnectionString = cs.ConnectionString;

Die docs sagen nicht, was passiert, wenn es nicht die Verbindungszeichenfolge einstellen könnte. Gibt es eine Ausnahme auslösen? Hält sie es ignorieren? Wie bei den meisten Ausnahme, wenn ein Fehler war bei dem Versuch, die Connection einer Verbindung zu setzen, gibt es nichts, was ich davon erholen tun können. Also ich werde nichts tun.

---

Und schließlich Öffnen der Datenbankverbindung:

conn.Open();

Die rel="noreferrer"> von DbConnection ist abstrakt, so ist es bis zu, was Anbieter von DbConnection absteigend zu entscheiden, welche Ausnahmen sie werfen. Es gibt auch keine Führung in der Zusammenfassung Offene Methoden Dokumentation über das, was ich geschehen erwarten können, wenn ein Fehler vorliegt. Wenn es verbindet ein Fehler, ich weiß, ich kann nicht damit umgehen - ich werde es sprudeln lassen müssen, wo der Anrufer eine UI für den Benutzer zeigen kann, und lassen Sie sie wieder versuchen

.

---

Nach

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
    if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

Zusammenfassung

Also meine vier Linienfunktion wurde 12 Linien und 5 erforderlich mintues die Dokumentation Lookup. Am Ende hat i einen Fall fangen, wo ein Verfahren zur Rück null erlaubt. Aber in der Praxis alles, was ich tat, war eine Zugriffsverletzung Ausnahme konvertieren (wenn ich versuche, Methoden auf einer Nullreferenz zu nennen) in ein InvalidArgumentException .

i fangen auch zwei mögliche Fälle, wo es sein könnte null Rückkehr Objekte; aber auch hier gehandelt i nur eine Ausnahme für einen anderen.

Auf der positiven Seite, hat es zwei Probleme fangen, und zu erklären, was in der Ausnahmemeldung passiert, anstatt schlechte Dinge HappEning die Straße hinunter (das heißt der Bock stoppt hier)

Aber ist es das wert? Ist das übertrieben? Ist die defensive Programmierung schief gegangen?

Answer 1

manuell für eine Konfiguration überprüft und eine Ausnahme zu werfen ist nicht besser als nur der Rahmen, um die Ausnahme zu lassen werfen, wenn die Konfiguration fehlt. Sie sind nur Voraussetzung prüft duplizieren, die ohnehin in den Rahmen Methoden geschieht, und es macht Sie ohne Nutzen ausführliche codieren. (Eigentlich könnten Sie sein Entfernen Informationen von allem als Basisausnahmeklasse zu werfen. Die Ausnahmen vom Framework geworfen sind in der Regel spezifisch.)

Edit: Diese Antwort scheint etwas umstritten zu sein, also ein bisschen Ausarbeitung: Defensive Programmierung bedeutet „Vorbereitung für die unerwarteten“ (oder „paranoid“) und eine der Möglichkeiten, das zu tun, ist viel Voraussetzung Kontrollen machen . In vielen Fällen ist dies eine gute Praxis, aber wie bei allen Praktiken sollten Kosten gegen Nutzen abgewogen werden.

Zum Beispiel es bietet keinen Vorteil eine „erhält Verbindung kann nicht von der Fabrik“ zu werfen Ausnahme, da es sagt nichts über Warum der Anbieter kann nicht erreicht werden - und die nächste Zeile wird eine Ausnahme ohnehin werfen, wenn der Provider null ist. So sind die Kosten der Voraussetzung Prüfung (in der Entwicklungszeit und die Komplexität des Codes) nicht gerechtfertigt ist.

Auf der anderen Seite die Prüfung, um sicherzustellen, dass die Verbindungszeichenfolge Konfiguration vorhanden ist können gerechtfertigt werden kann, da die Ausnahme helfen, die Entwickler sagen, wie das Problem zu lösen. Die Null-Ausnahme, die Sie in der nächsten Zeile erhalten werden sowieso nicht sagt dem Namen der Verbindungszeichenfolge, die fehlt, so dass Ihre Voraussetzung Prüfung einen Wert liefert. Wenn Ihr Code Teil einer Komponente zum Beispiel ist, ist der Wert ziemlich groß, da der Benutzer der Komponente möglicherweise nicht wissen, welche die Komponentenkonfigurationen erfordert.

Eine andere Interpretation der defensiven Programmierung ist, dass Sie nicht nur Fehlerzustände erkennen sollen, sollten Sie auch versuchen, aus jedem Fehler oder eine Ausnahme zu erholen, die auftreten können. Ich glaube nicht, das ist eine gute Idee im Allgemeinen ist.

Grundsätzlich sollten Sie nur Ausnahmen behandeln, dass man tun etwas über. Ausnahmen, die Sie von ohnehin nicht mehr erholen können, sollten nur nach oben in den Top-Level-Handler übergeben werden. In einer Web-Anwendung zeigt die Top-Level-Handler wahrscheinlich nur eine generische Fehlerseite. Aber es nicht wirklich viel ist in den meisten Fällen zu tun, wenn die Datenbank offline ist oder einige entscheidende Konfiguration fehlt.

Einige Fälle, in denen diese Art von defensiver Programmierung Sinn macht, ist, wenn Sie Benutzereingaben akzeptieren, und dass die Eingabe zu Fehlern führen kann. Wenn zum Beispiel der Benutzer eine URL als Eingabe zur Verfügung stellt und die Anwendung versucht, etwas aus dieser URL zu holen, dann ist es ganz wichtig, dass Sie überprüfen, dass die URL korrekt aussieht, und Sie können jede Ausnahme behandeln, die von der Anforderung führen kann. Auf diese Weise können Sie wertvolles Feedback für den Benutzer zur Verfügung zu stellen.

Answer 2

Nun, es hängt wer Ihre Zielgruppe ist.

Wenn Sie Bibliothek Code schreiben, die Sie erwarten von einer Menge anderer Leute verwendet werden, die nicht mit Ihnen darüber reden, wie es zu benutzen, dann ist es nicht übertrieben. Sie werden zu schätzen wissen Ihre Bemühungen.

(Das heißt, wenn Sie das tun, empfehle ich Ihnen besser Ausnahmen als nur System.Exception definieren, ist es für die Menschen leichter zu machen, die wollen einige Ihrer Ausnahmen fangen, aber andere nicht.)

Aber wenn Sie nur sich selbst gehen, es zu benutzen (oder Sie und Ihr Kumpel), dann offensichtlich ist es übertrieben, und wahrscheinlich weh tut man am Ende von Ihrem Code weniger lesbar zu machen.

Answer 3

Ich wünsche, ich mein Team so codieren, erhalten könnte. Die meisten Leute nicht bekommen sogar den Punkt der Defensive Programmierung. Das Beste, was sie tun, ist das ganze Verfahren in einer try catch-Anweisung zu wickeln und lassen Sie alle Ausnahmen von dem allgemeinen Ausnahmeblock behandelt werden!

Hüte weg zu Ihnen Ian. Ich kann Ihr Dilemma verstehen. Ich habe mich durch das gleiche gewesen. Aber was Sie haben dazu beigetragen, wahrscheinlich einige Entwickler mehrere Stunden Tastatur bashing.

Denken Sie daran, wenn Sie eine .NET Framework-API verwenden, was erwarten Sie davon? Was scheint natürlich? Machen Sie dasselbe mit Ihrem Code.

Ich weiß, dass es Zeit braucht. Aber dann kommt Qualität zu einem Preis.

PS: Sie wirklich nicht alle relevanten Fehler behandeln und eine benutzerdefinierte Ausnahme auslösen. Erinnern, dass Ihre Methode wird nur von anderen Entwicklern verwendet werden. Sie sollten in der Lage sein, sich gemeinsamen Rahmen Ausnahmen herauszufinden. Das ist nicht der Mühe wert.

Answer 4

Ihr „vor“ Beispiel hat die Unterscheidung von klar und prägnant sein.

Wenn etwas nicht in Ordnung ist, wird eine Ausnahme schließlich vom Framework geworfen werden. Wenn Sie nichts über die Ausnahme tun kann, dann kann man auch lassen Sie es den Call-Stack propagieren auf.

Es gibt jedoch Zeiten, wenn eine Ausnahme tief im Inneren des Rahmens geworfen wird, die wirklich Schuppen Licht nicht auf das, was das eigentliche Problem ist. Wenn Ihr Problem ist, dass Sie nicht über eine gültige Verbindungszeichenfolge haben, aber der Rahmen löst eine Ausnahme wie „ungültige Verwendung von null,“ dann ist es manchmal besser, die Ausnahme zu fangen und es mit einer Nachricht erneut auslösen, die mehr sinnvoll ist.

ich für null tun überprüfen Objekte viel, da ich ein tatsächliches Objekt muß auf dem Betrieb, und wenn das Objekt leer ist die Ausnahme, die schräg geworfen wird, um es gelinde auszudrücken. Aber ich nur für null Objekte überprüfen, ob ich weiß, das ist, was geschehen wird. Einige Objekt Fabriken nicht zurück null Objekte; sie werfen eine Ausnahme statt und wird in diesen Fällen für null Überprüfung nutzlos sein.

Answer 5

Ich glaube nicht, dass ich irgendwelche dieser Nullverweis schreiben würde Prüflogik - zumindest nicht so, wie Sie es getan haben

.

Meine Programme, die Konfigurationseinstellungen von der Anwendungskonfigurationsdatei überprüfen all diese Einstellungen beim Start zu bekommen. Ich baue in der Regel eine statische Klasse, um die Einstellungen zu enthalten und auf diese Klasse Eigenschaften Referenz (und nicht die ConfigurationManager) an anderer Stelle in der Anwendung. Es gibt zwei Gründe dafür.

Erstens, wenn die Anwendung nicht richtig konfiguriert ist, es wird nicht funktionieren. Ich würde lieber wissen, dass dies im Moment das Programm der Konfigurationsdatei liest, als irgendwann in der Zukunft, wenn ich versuche, eine Datenbankverbindung zu erstellen.

Zweitens sollte die Gültigkeit der Konfigurationsprüfung nicht wirklich sein, die Sorge der Objekte, die von der Konfiguration verlassen. Es hat keinen Sinn zu machen, sich Kontrollen überall im gesamten Code einfügen, wenn Sie bereits diese Kontrollen vorne durchgeführt haben. (Es gibt Ausnahmen, sicher - zum Beispiel, lang laufende Anwendungen, in denen Sie müssen in der Lage sein, die Konfiguration zu ändern, während das Programm läuft, und haben diese Änderungen spiegelt sich in das Verhalten des Programms, in Programme wie dieses Sie müssen gehen Sie auf die ConfigurationManager, wenn Sie eine Einstellung benötigen.)

Ich würde die Null-Referenzprüfung auf den GetFactory und CreateConnection Anrufe nicht tun, auch nicht. Wie würden Sie einen Testfall schreiben, dass Code zu trainieren? Sie können nicht, weil Sie nicht wissen, wie diese Methoden null zurück zu machen - Sie wissen nicht einmal, dass es möglich machen diese Methoden null zurück. So haben Sie ein Problem ersetzt - Ihr Programm kann eine NullReferenceException unter Bedingungen werfen, die Sie nicht verstehen - mit einem anderen, bedeutenderen ein: unter den gleichen Bedingungen mysteriös, Ihr Programm läuft Code, den Sie haben nicht getestet.

Answer 6

Meine Regel von Daumen ist:

  

nicht fangen, wenn die Nachricht von der   geworfen Ausnahme ist relevant für die   Anrufer.

So Nullreferenceexception keine entsprechende Meldung hat, würde ich prüfen, ob es null und eine Ausnahme mit einer besseren Botschaft werfen. ConfigurationErrorException ist relevant, damit ich es nicht fangen.

Die einzige Ausnahme ist, wenn der „Vertrag“ von GetConnection nicht die Verbindungszeichenfolge unbedingt in einer Konfigurationsdatei nicht abgerufen werden.

Wenn es der Fall ist GetConnection einen Vertrag mit einer benutzerdefinierten Ausnahme haben sollte, die sagen, dass die Verbindung nicht abgerufen werden kann, dann können Sie ConfigurationErrorException in Ihrer benutzerdefinierten Ausnahme wickeln.

Die andere Lösung ist zu präzisieren, dass GetConnection nicht werfen kann (kann aber null zurück), dann eine „Exception“, um Ihre Klasse, die Sie hinzufügen.

Answer 7

Ihre Methode Dokumentation fehlt. ; -)

Jede Methode hat einige definierte Ein- und Ausgangsparameter und ein definierten resultierendes Verhalten. In Ihrem Fall so etwas wie:.. „Gibt gültig offene Verbindung im Erfolgsfall, sonst null zurückgibt (oder eine XXXException wirft, wie Sie möchten), dieses Verhalten im Auge behalten, können Sie nun entscheiden, wie defensiv sollten Sie programmieren

• Wenn Ihre Methode sollte detaillierte Informationen aussetzen, warum und was fehlgeschlagen ist, dann tun Sie es, wie Sie haben und überprüfen und alle und alles und gibt die entsprechenden Informationen zu fangen.

• Aber, wenn Sie nur daran interessiert in einem offenen DBConnection sind oder nur null (oder eine Ausnahme benutzerdefiniert) auf Versagen, dann wickelt einfach alles in einem try / catch und zurück null (oder eine Ausnahme) auf Fehler, und das Objekt sonst.

Also ich würde sagen, es hängt davon ab, das Verhalten der Methode und erwartete Ausgabe.

Answer 8

In der Regel datenbankspezifische Ausnahmen sollten als etwas allgemeinere, wie (hypothetische) DataAccessFailure Ausnahme abgefangen und wieder geworfen werden. Übergeordnetes Code in den meisten Fällen muss nicht wissen, dass Sie die Daten aus der Datenbank lesen.

Ein weiterer Grund zu stoppen schnell diese Fehler ist, dass sie oft einige Datenbank Details in ihren Nachrichten enthalten, wie „No solche Tabelle: ACCOUNTS_BLOCKED“ oder „Benutzerschlüssel ungültig: 234234“. Wenn diese an den Endbenutzer ausbreitet, es ist schlecht in mehrfacher Hinsicht:

1. Verwirrende.
2. Potentielle Sicherheitsverletzung.
3. Peinlich für das Image Ihres Unternehmens (eine Client vorstellen, eine Fehlermeldung mit roher Grammatik zu lesen).

Answer 9

ich hätte es genau wie der erste Versuch codiert.

Allerdings würde der Benutzer dieser Funktion das Verbindungsobjekt mit einem weiter verwenden Block schützen.

Ich mag nicht Ausnahmen wie Ihre andere Versionen übersetzen tun, wie es es sehr schwierig macht, um herauszufinden, warum es brach (Datenbank nach unten? Sie keine Berechtigung zur Konfigurationsdatei gelesen, etc?).

Answer 10

Die geänderte Fassung trägt nicht viel Wert, solange die Anwendung eine AppDomain.UnexpectedException Handler hat, der die exception.InnerException Kette und alle Stack-Traces in eine Protokolldatei von einer Art Dumps (oder noch besser, fängt eine Minidump) und rufen Sie dann Environment.FailFast.

Aus diesen Informationen wird es relativ einfach sein, genau zu bestimmen, was falsch gelaufen ist, ohne dass der Methodencode mit zusätzlicher Fehlerprüfung erschweren.

Beachten Sie, dass es bevorzugt AppDomain.UnexpectedException zu handhaben und Environment.FailFast rufen stattdessen ein Top-Level try/catch (Exception x) dem Haben, weil mit dieser, die ursprünglichen Ursache des Problems wahrscheinlich durch weitere Ausnahmen verdeckt wird.

Dies liegt daran, wenn Sie eine Ausnahme zu fangen, werden alle offenen finally Blöcke auszuführen, und wird wahrscheinlich mehr Ausnahmen werfen, was die ursprüngliche Ausnahme verstecken (oder noch schlimmer, sie werden Dateien in einem Versuch löschen, um etwas Zustand zurückkehren, möglicherweise die falsche Dateien, vielleicht sogar wichtige Dateien). Sie sollen niemals eine Ausnahme fangen, die einen ungültigen Programmzustand anzeigt, dass Sie nicht wissen, wie zu handhaben - auch in einem Top-Level-main Funktion try/catch Block. Handhabung AppDomain.UnexpectedException und rufen Environment.FailFast ist ein anderer (und wünschenswerter) Kessel der Fische, weil es finally Blöcke vom Laufen hält, und wenn Sie versuchen, Ihr Programm zu stoppen und einige hilfreichen Informationen anmelden, ohne weiteren Schaden anrichten kann, sind Sie definitiv nicht möchten Ihre finally Blöcke ausgeführt werden.

Answer 11

Vergessen Sie nicht, für OutOfMemoryExceptions zu überprüfen ... Sie wissen, ist es Macht geschehen.

Answer 12

Iains Änderungen aussehen vernünftig zu mir.

Wenn ich ein System benutzen und ich verwende es nicht richtig, ich mag die maximalen Informationen über den Missbrauch. Z.B. wenn ich vergesse, einige Werte in eine Konfiguration einzufügen, bevor Sie eine Methode aufrufen, möchte ich eine InvalidOperationException mit einer Meldung meine Fehler Detaillierung, kein KeyNotFoundException / Nullreferenceexception.

Es geht um Kontext IMO. Ich habe einige ziemlich undurchdringlich Ausnahmemeldungen in meiner Zeit gesehen, aber auch andere Zeiten die Standard-Exception aus dem Rahmen kommt, ist völlig in Ordnung.

Generell denke ich, es ist besser, auf der Seite der Vorsicht irren, vor allem, wenn Sie etwas zu schreiben, die sich stark von anderen Leuten verwendet wird, oder in der Regel tief in dem Aufrufgraphen, wo der Fehler schwerer zu diagnostizieren ist.

Ich versuche immer, sich daran zu erinnern, dass als Entwickler von einem Stück Code oder System, die ich in einer besseren Position bin Ausfälle als jemand zu diagnostizieren, die gerade es verwendet, ist. Es ist manchmal der Fall, dass ein paar Zeilen Code überprüft + eine benutzerdefinierte Ausnahmemeldung kann kumulativ Stunden Debugging-Zeit sparen (und auch einfacher, Ihr eigenes Leben machen, wie Sie gezogen werden nicht herum Maschine jemand anderes ihr Problem zu debuggen).

Answer 13

In meinen Augen, deine „nach“ Probe nicht wirklich defensiv ist. Da Defensive sein würde, die Teile unter Kontrolle zu überprüfen, die das Argument connectionName (Prüfung auf null oder leer, und wirft ein Argument) sein würden.

Answer 14

Warum die Methode nicht aufteilen Sie, nachdem Sie alle defensive Programmierung hinzugefügt? Sie haben eine Reihe von verschiedenen Logikblöcke, die verschiedene Methoden rechtfertigen. Warum? Denn dann kapseln Sie die Logik, die zusammen, und Ihre resultierende öffentliche Methode nur Drähte diese Blöcke in der richtigen Weise gehört.

So etwas wie dies (im SO-Editor bearbeitet, so dass keine Syntax / Compiler prüft Könnte nicht kompilieren; -.))

private string GetConnectionString(String connectionName)
{

   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
   if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
   return cs;
}

private DbProviderFactory GetFactory(String ProviderName)
{
   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
   return factory;
}

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs = GetConnectionString(connectionName);

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = GetFactory(cs.ProviderName);


   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

PS: Dies ist kein vollständiger refactor, hat nur die ersten beiden Blöcke

.