Müssen wir die gesamte Modulnutzung oder nur Argumente öffentlicher Methoden validieren?

9

Ich habe gehört, dass es empfohlen wird, Argumente öffentlicher Methoden zu validieren:

Motivation ist verständlich. Wenn ein Modul falsch verwendet wird, möchten wir sofort eine Ausnahme auslösen, anstatt ein unvorhersehbares Verhalten.

Was mich stört, ist, dass falsche Argumente nicht der einzige Fehler sind, der bei der Verwendung eines Moduls gemacht werden kann. Hier sind einige Fehlerszenarien, in denen wir eine Überprüfungslogik hinzufügen müssen, wenn wir den Empfehlungen folgen und keine Fehlereskalation wünschen:

  • Eingehender Anruf - unerwartete Argumente
  • Eingehender Anruf - Modul ist in einem falschen Zustand
  • Externer Anruf - unerwartete Ergebnisse zurückgegeben
  • Externer Aufruf - unerwartete Nebenwirkungen (doppelter Eintrag in ein aufrufendes Modul, Aufheben anderer Abhängigkeitszustände)

Ich habe versucht, all diese Bedingungen zu berücksichtigen und ein einfaches Modul mit einer Methode zu schreiben (sorry, nicht-C # Jungs):

public sealed class Room
{
    private readonly IDoorFactory _doorFactory;
    private bool _entered;
    private IDoor _door;
    public Room(IDoorFactory doorFactory)
    {
        if (doorFactory == null)
            throw new ArgumentNullException("doorFactory");
        _doorFactory = doorFactory;
    }
    public void Open()
    {
        if (_door != null)
            throw new InvalidOperationException("Room is already opened");
        if (_entered)
            throw new InvalidOperationException("Double entry is not allowed");
        _entered = true;
        _door = _doorFactory.Create();
        if (_door == null)
            throw new IncompatibleDependencyException("doorFactory");
        _door.Open();
        _entered = false;
    }
}

Jetzt ist es sicher =)

Es ist ziemlich gruselig. Aber stellen Sie sich vor, wie gruselig es in einem realen Modul mit Dutzenden von Methoden, komplexem Zustand und vielen externen Aufrufen sein kann (Hallo, Liebhaber der Abhängigkeitsinjektion!). Beachten Sie, dass Sie, wenn Sie ein Modul aufrufen, dessen Verhalten überschrieben werden kann (nicht versiegelte Klasse in C #), einen externen Anruf tätigen und die Konsequenzen für den Aufrufer nicht vorhersehbar sind.

Zusammenfassend, was ist der richtige Weg und warum? Wenn Sie aus den folgenden Optionen auswählen können, beantworten Sie bitte weitere Fragen.

Überprüfen Sie die gesamte Modulnutzung. Benötigen wir Unit-Tests? Gibt es Beispiele für solchen Code? Sollte die Abhängigkeitsinjektion in der Verwendung eingeschränkt sein (da dies mehr Überprüfungslogik verursacht)? Ist es nicht praktisch, diese Prüfungen auf die Debug-Zeit zu verschieben (nicht in der Version enthalten)?

Überprüfen Sie nur Argumente. Nach meiner Erfahrung ist die Argumentprüfung - insbesondere die Nullprüfung - die am wenigsten wirksame Prüfung, da Argumentfehler selten zu komplexen Fehlern und Fehlereskalationen führen. Meistens erhalten Sie eine NullReferenceExceptionin der nächsten Zeile. Warum sind Argumentprüfungen so besonders?

Überprüfen Sie nicht die Modulnutzung. Es ist eine ziemlich unpopuläre Meinung, können Sie erklären, warum?

Astef
quelle
Während der Feldzuweisung sollten Überprüfungen durchgeführt werden, um sicherzustellen, dass Invarianten erhalten bleiben.
Basilevs
@ Basilevs Interessant ... Ist es von Code Contracts Ideologie oder etwas älter? Können Sie etwas zum Lesen empfehlen (im Zusammenhang mit Ihrem Kommentar)?
Astef
Es ist eine grundlegende Trennung von Bedenken. Alle Ihre Fälle werden abgedeckt, während die Codeduplizierung minimal ist und die Verantwortlichkeiten genau definiert sind.
Basilevs
@Basilevs Überprüfen Sie also überhaupt nicht das Verhalten anderer Module, sondern die eigenen Statusinvarianten. Klingt vernünftig. Aber warum sehe ich diese einfache Quittung nicht in den entsprechenden Fragen zu Argumentprüfungen?
Astef
Nun, einige vorläufige Überprüfungen sind noch erforderlich, aber sie sollten nur für tatsächlich verwendete Werte durchgeführt werden, nicht für diejenigen, die an eine andere Stelle weitergeleitet werden. Beispielsweise verlassen Sie sich auf die List-Implementierung, um OOB-Fehler zu überprüfen, anstatt den Index im Client-Code zu überprüfen. In der Regel handelt es sich um Framework-Fehler auf niedriger Ebene, die nicht manuell ausgegeben werden müssen.
Basilevs

Antworten:

2

TL; DR: Statusänderung validieren, auf [Gültigkeit des] aktuellen Status verlassen.

Im Folgenden betrachte ich nur freigegebene Überprüfungen. Nur aktive Debug-Zusicherungen sind eine Form der Dokumentation, die auf ihre eigene Weise nützlich ist und für diese Frage nicht in Frage kommt.

Beachten Sie folgende Grundsätze:

  • Gesunder Menschenverstand
  • Schnell ausfallen
  • TROCKEN
  • SRP

Definitionen

  • Komponente - eine Einheit, die API bereitstellt
  • Client - Benutzer der API der Komponente

Veränderlicher Zustand

Problem

In imperativen Sprachen können das Fehlersymptom und seine Ursache durch stundenlanges schweres Heben getrennt sein. Staatliche Korruption kann sich verstecken und mutieren, was zu unerklärlichen Fehlern führt, da die Überprüfung des aktuellen Zustands keinen vollständigen Korruptionsprozess und damit die Ursache des Fehlers aufdecken kann.

Lösung

Jede Zustandsänderung sollte sorgfältig ausgearbeitet und überprüft werden. Eine Möglichkeit, mit veränderlichen Zuständen umzugehen, besteht darin, sie auf ein Minimum zu beschränken. Dies wird erreicht durch:

  • Typsystem (const und endgültige Mitgliederdeklarationen)
  • Einführung von Invarianten
  • Überprüfen jeder Änderung des Komponentenstatus über öffentliche APIs

Wenn Sie den Status einer Komponente erweitern, sollten Sie dies in Betracht ziehen, indem Sie den Compiler die Unveränderlichkeit neuer Daten erzwingen lassen. Erzwingen Sie außerdem jede sinnvolle Laufzeitbeschränkung und beschränken Sie mögliche resultierende Zustände auf einen kleinstmöglichen genau definierten Satz.

Beispiel

// Wrong
class Natural {
    private int number;
    public Natural(int number) {
        this.number = number;
    }
    public int getInt() {
      if (number < 1)
          throw new InvalidOperationException();
      return number;
    }
}

// Right
class Natural {
    private readonly int number;
    /**
     * @param number - positive number
     */
    public Natural(int number) {
      // Going to modify state, verification is required
      if (number < 1)
        throw new ArgumentException("Natural number should be  positive: " + number);
      this.number = number;
    }
    public int getInt() {
      // State is guaranteed by construction and compiler
      return number;
    }
}

Wiederholung und Verantwortungszusammenhalt

Problem

Das Überprüfen der Voraussetzungen und Nachbedingungen des Betriebs führt zu einer Verdoppelung des Bestätigungscodes sowohl im Client als auch in der Komponente. Das Überprüfen des Komponentenaufrufs zwingt den Client häufig dazu, einige der Verantwortlichkeiten der Komponente zu übernehmen.

Lösung

Verlassen Sie sich nach Möglichkeit auf die Komponente, um die Statusüberprüfung durchzuführen. Komponenten müssen eine API bereitstellen, für die keine spezielle Verifizierungsüberprüfung erforderlich ist (z. B. Überprüfung der Argumente oder Durchsetzung der Operationssequenz), damit der Komponentenstatus genau definiert bleibt. Sie verpflichten sich, API-Aufrufargumente nach Bedarf zu überprüfen, Fehler mit den erforderlichen Mitteln zu melden und ihre Statusbeschädigung zu verhindern.

Clients sollten sich auf Komponenten verlassen, um die Verwendung ihrer API zu überprüfen. Nicht nur Wiederholungen werden vermieden, der Client hängt nicht mehr von zusätzlichen Implementierungsdetails der Komponente ab. Betrachten Sie Framework als eine Komponente. Schreiben Sie nur dann benutzerdefinierten Bestätigungscode, wenn die Invarianten der Komponente nicht streng genug sind oder um die Komponentenausnahme als Implementierungsdetail zu kapseln.

Wenn eine Operation den Status nicht ändert und nicht durch Statusänderungsüberprüfungen abgedeckt ist, überprüfen Sie jedes Argument auf einer möglichst tiefen Ebene.

Beispiel

class Store {
  private readonly List<int> slots = new List<int>();
  public void putToSlot(int slot, int data) {
    if (slot < 0 || slot >= slots.Count) // Unnecessary, validated by List, only needed for custom error message
      throw new ArgumentException("data");
    slots[slot] = data;
  }
}

class Natural {
   int _number;
   public Natural(int number) {
       if (number < 1)
          number = 1;  //Wrong: client can't rely on argument verification, additional state uncertainity is introduced.  Right: throw new ArgumentException(number);
       _number = number;
   }
}

Antworten

Wenn die beschriebenen Prinzipien auf das betreffende Beispiel angewendet werden, erhalten wir:

public sealed class Room
{
    private bool _entered = false;
    // Do not use lazy instantiation if not absolutely necessary, this introduces additional mutable state
    private readonly IDoor _door;
    public Room(IDoorFactory doorFactory)
    {
        // Rely on system null check
        IDoor door = _doorFactory.Create();
        // Modifying own state, verification is required
        if (door == null)
           throw new ArgumentNullException("Door");
        _door = door;
    }
    public void Enter()
    {
        // Room invariants do not guarantee _entered value. Door state is indirectly a part of our state. Verification is required to prevent second door state change below.
        if (_entered)
           throw new InvalidOperationException("Double entry is not allowed");
        _entered = true;     
        // rely on immutability for _door field to be non-null
        // rely on door implementation to control resulting door state       
        _door.Open();            
    }
}

Zusammenfassung

Der Client-Status besteht aus eigenen Feldwerten und Teilen des Komponentenstatus, die nicht durch eigene Invarianten abgedeckt sind. Die Überprüfung sollte nur vor der tatsächlichen Statusänderung eines Clients erfolgen.

Basilevs
quelle
1

Eine Klasse ist für ihren eigenen Zustand verantwortlich. Validieren Sie also in dem Maße, in dem die Dinge in einem akzeptablen Zustand gehalten oder versetzt werden.

Wenn ein Modul falsch verwendet wird, möchten wir sofort eine Ausnahme auslösen, anstatt ein unvorhersehbares Verhalten.

Nein, werfen Sie keine Ausnahme, sondern liefern Sie vorhersehbares Verhalten. Eine Folge der staatlichen Verantwortung ist es, die Klasse / Anwendung so tolerant wie möglich zu gestalten. Zum Beispiel nullan aCollection.Add()? Nur nicht hinzufügen und weitermachen. Sie erhalten nullEingaben zum Erstellen eines Objekts? Erstellen Sie ein Nullobjekt oder ein Standardobjekt. Oben ist das doorschon open? Also, mach weiter. DoorFactoryArgument ist null? Erstellen Sie eine neue. Wenn ich eine erstelle, habe enumich immer ein UndefinedMitglied. Ich benutze Dictionarys liberal und enumsdefiniere Dinge explizit; und dies trägt wesentlich dazu bei, vorhersehbares Verhalten zu liefern.

(Hallo, Liebhaber von Abhängigkeitsinjektionen!)

Ja, obwohl ich durch den Schatten des Tals der Parameter gehe, werde ich keine Argumente fürchten. Zu den vorhergehenden verwende ich auch so oft wie möglich Standard- und optionale Parameter.

All dies ermöglicht es der internen Verarbeitung, weiterzumachen. In einer bestimmten Anwendung habe ich Dutzende von Methoden in mehreren Klassen mit nur einer Stelle, an der eine Ausnahme ausgelöst wird. Selbst dann liegt es nicht an Null-Argumenten oder daran, dass ich die Verarbeitung nicht fortsetzen konnte, sondern daran, dass der Code ein "nicht funktionierendes" / "Null" -Objekt erstellt hat.

bearbeiten

Ich zitiere meinen Kommentar in seiner Gesamtheit. Ich denke, das Design sollte nicht einfach "aufgeben", wenn man auf "null" stößt. Besonders mit einem zusammengesetzten Objekt.

Wir vergessen hier wichtige Konzepte / Annahmen - encapsulation& single responsibility. Nach der ersten, mit dem Client interagierenden Schicht gibt es praktisch keine Nullprüfung. Der Code ist tolerant robust. Klassen werden mit Standardzuständen entworfen und funktionieren so, ohne geschrieben zu werden, als ob interagierender Code von Fehlern befallen ist. Ein zusammengesetzter Elternteil muss nicht die untergeordneten Ebenen erreichen, um die Gültigkeit zu bewerten (und implizit in allen Ecken und Winkeln auf Null prüfen). Der Elternteil weiß, was der Standardstatus eines Kindes bedeutet

Ende bearbeiten

Radarbob
quelle
1
Das Nicht-Hinzufügen eines ungültigen Sammlungselements ist ein sehr unvorhersehbares Verhalten.
Basilevs
1
Wenn alle Schnittstellen eines Tages so tolerant gestaltet werden, werden Programme aufgrund banaler Fehler versehentlich erwachen und die Menschheit zerstören.
Astef
Wir vergessen hier wichtige Konzepte / Annahmen - encapsulation& single responsibility. nullNach der ersten, mit dem Client interagierenden Schicht erfolgt praktisch keine Überprüfung. Der Code ist <strike> tolerant </ strike> robust. Klassen werden mit Standardzuständen entworfen und funktionieren so, ohne geschrieben zu werden, als ob interagierender Code von Fehlern befallen ist. Ein zusammengesetzter Elternteil muss nicht die untergeordneten Ebenen erreichen, um die Gültigkeit zu bewerten (und implizit nullin allen Ecken und Winkeln nachsehen). Der Elternteil weiß, was der Standardzustand eines Kindes bedeutet
Radarbob