Sollte ich jetzt redundanten Code hinzufügen, nur für den Fall, dass er in Zukunft benötigt wird?

Zu Recht oder zu Unrecht bin ich derzeit der Überzeugung, dass ich immer versuchen sollte, meinen Code so robust wie möglich zu machen, auch wenn dies das Hinzufügen von redundantem Code / Überprüfungen bedeutet, von denen ich weiß , dass sie momentan nicht von Nutzen sind, aber sie könnte x Jahre später sein.

Zum Beispiel arbeite ich gerade an einer mobilen Anwendung mit folgendem Code:

public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
    }

    //2. Is rows empty? - This will be the case if this is the first appointment / some other unknown reason.
    if(rows.Count == 0)
    {
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

Wenn ich Abschnitt zwei genauer betrachte, weiß ich, dass Abschnitt zwei auch wahr sein wird, wenn Abschnitt eins wahr ist. Ich kann mir keinen Grund vorstellen, warum Abschnitt eins falsch und Abschnitt zwei wahr ist, was die zweite ifAussage überflüssig macht.

In Zukunft kann es jedoch vorkommen, dass diese zweite ifAussage aus einem bekannten Grund tatsächlich benötigt wird.

Einige Leute mögen dies zunächst betrachten und denken, ich programmiere mit Blick auf die Zukunft, was offensichtlich eine gute Sache ist. Aber ich kenne einige Fälle, in denen diese Art von Code Fehler vor mir "versteckt" hat. Bedeutung es mir noch mehr , um herauszufinden genommen hat , warum Funktion xyztut, abcwenn es tatsächlich tun soll def.

Andererseits gab es auch zahlreiche Fälle, in denen es durch diese Art von Code wesentlich einfacher war, den Code mit neuen Verhaltensweisen zu erweitern, da ich nicht zurückgehen und sicherstellen muss, dass alle relevanten Überprüfungen vorhanden sind.

Gibt es allgemeine Faustregeln für diese Art von Code? (Würde mich auch interessieren, ob dies als gute oder schlechte Praxis angesehen wird?)

NB: Dies könnte als ähnlich zu dieser Frage angesehen werden. Im Gegensatz zu dieser Frage hätte ich gerne eine Antwort, vorausgesetzt, es gibt keine Fristen.

TLDR: Sollte ich so weit gehen, redundanten Code hinzuzufügen, um ihn in Zukunft möglicherweise robuster zu machen?

Lassen Sie uns zunächst Ihre Logik überprüfen. Wie wir sehen werden, haben Sie jedoch größere Probleme als jedes andere logische Problem.

Nennen Sie die erste Bedingung A und die zweite Bedingung B.

Sie sagen zuerst:

Wenn ich Abschnitt zwei genauer betrachte, weiß ich, dass Abschnitt zwei auch wahr sein wird, wenn Abschnitt eins wahr ist.

Das heißt: A impliziert B oder grundlegender ausgedrückt (NOT A) OR B

Und dann:

Ich kann mir keinen Grund vorstellen, warum Abschnitt eins falsch und Abschnitt zwei wahr ist, was die zweite if-Anweisung überflüssig macht.

Das heißt: NOT((NOT A) AND B). Wende das Demorgan-Gesetz an, um zu ermitteln, (NOT B) OR Awelches B A impliziert.

Wenn also beide Aussagen wahr sind, impliziert A B und B impliziert A, was bedeutet, dass sie gleich sein müssen.

Daher sind die Prüfungen ja überflüssig. Sie scheinen vier Codepfade durch das Programm zu haben, aber tatsächlich haben Sie nur zwei.

Die Frage ist nun: Wie schreibe ich den Code? Die eigentliche Frage ist: Was ist der angegebene Vertrag der Methode ? Die Frage, ob die Bedingungen überflüssig sind, ist ein roter Hering. Die eigentliche Frage lautet: "Habe ich einen vernünftigen Vertrag entworfen und setzt meine Methode diesen Vertrag eindeutig um?"

Schauen wir uns die Erklärung an:

public static CalendarRow AssignAppointmentToRow(
    Appointment app,    
    List<CalendarRow> rows)

Es ist öffentlich, daher muss es robust gegenüber schlechten Daten von beliebigen Anrufern sein.

Es gibt einen Wert zurück, daher sollte es für seinen Rückgabewert nützlich sein, nicht für seine Nebenwirkung.

Und doch ist der Name der Methode ein Verb, was darauf hindeutet, dass es für seine Nebenwirkung nützlich ist.

Der Vertrag des Listenparameters lautet:

• Eine Nullliste ist OK
• Eine Liste mit einem oder mehreren Elementen ist in Ordnung
• Eine Liste ohne Elemente ist falsch und sollte nicht möglich sein.

Dieser Vertrag ist verrückt . Stellen Sie sich vor, Sie schreiben die Dokumentation dazu! Stellen Sie sich vor, Sie schreiben Testfälle!

Mein Rat: von vorne anfangen. Diese API ist mit einer Candy Machine-Schnittstelle versehen. (Der Ausdruck stammt aus einer alten Geschichte über die Bonbonautomaten bei Microsoft, in der sowohl die Preise als auch die Auswahl zweistellige Zahlen sind. Es ist sehr einfach, "85" einzugeben, was dem Preis für Artikel 75 entspricht, und Sie erhalten Das habe ich eigentlich versehentlich gemacht, als ich bei Microsoft versuchte, Kaugummi aus einem Automaten zu holen!)

So gestalten Sie einen sinnvollen Vertrag:

Machen Sie Ihre Methode entweder für die Nebenwirkung oder für den Rückgabewert nützlich, nicht für beide.

Akzeptieren Sie keine veränderlichen Typen als Eingaben, wie z. B. Listen. Wenn Sie eine Folge von Informationen benötigen, nehmen Sie eine IEnumerable. Lesen Sie nur die Sequenz; schreiben Sie nicht an einen übergebenen in Sammlung , es sei denn es ist sehr klar , dass dies der Vertrag des Verfahrens ist. Indem Sie IEnumerable nehmen, senden Sie dem Anrufer eine Nachricht, dass Sie seine Sammlung nicht mutieren werden.

Akzeptiere niemals Nullen; Eine Null-Sequenz ist ein Gräuel. Fordern Sie den Aufrufer auf, eine leere Sequenz zu übergeben, wenn dies sinnvoll ist, niemals null.

Absturz sofort, wenn der Anrufer gegen Ihren Vertrag verstößt, um ihn zu informieren, dass Sie es ernst meinen, und damit er seine Fehler beim Testen und nicht bei der Produktion entdeckt.

Entwerfen Sie den Vertrag zunächst so sinnvoll wie möglich und setzen Sie ihn dann klar um. So ist ein Design zukunftssicher.

Jetzt habe ich nur über Ihren speziellen Fall gesprochen und Sie haben eine allgemeine Frage gestellt. Hier sind einige zusätzliche allgemeine Ratschläge:

• Wenn es eine Tatsache gibt, die Sie als Entwickler ableiten können, der Compiler jedoch nicht, verwenden Sie eine Behauptung, um diese Tatsache zu dokumentieren. Verstößt ein anderer Entwickler, wie Sie oder einer Ihrer Kollegen, gegen diese Annahme, wird Ihnen dies anhand der Behauptung mitgeteilt.

• Holen Sie sich ein Test-Coverage-Tool. Stellen Sie sicher, dass Ihre Tests jede Codezeile abdecken. Wenn es nicht abgedeckten Code gibt, haben Sie entweder einen fehlenden Test oder Sie haben toten Code. Toter Code ist überraschend gefährlich, da er normalerweise nicht für tot gehalten wird! Die unglaublich schreckliche Sicherheitslücke von Apple vor ein paar Jahren fällt einem sofort ein.

• Holen Sie sich ein statisches Analyse-Tool. Verdammt, holen Sie sich mehrere; Jedes Werkzeug hat seine eigene Spezialität und keines ist eine Obermenge der anderen. Achten Sie darauf, wenn Sie darauf hingewiesen werden, dass nicht erreichbarer oder redundanter Code vorhanden ist. Auch dies sind wahrscheinlich Fehler.

Wenn es so klingt, als würde ich sagen: Erstens, entwerfen Sie den Code gut, und zweitens, testen Sie ihn auf Herz und Nieren, um sicherzustellen, dass er heute korrekt ist. Wenn Sie diese Dinge tun, wird der Umgang mit der Zukunft viel einfacher. Das Schwierigste an der Zukunft ist, sich mit all dem fehlerhaften Code zu beschäftigen, den die Leute in der Vergangenheit geschrieben haben. Machen Sie es heute richtig und die Kosten werden in Zukunft niedriger sein.

Was Sie in dem oben gezeigten Code tun, ist weniger Zukunftssicherheit als vielmehr defensive Codierung .

Beide ifAussagen testen auf verschiedene Dinge. Beides sind geeignete Tests, die von Ihren Anforderungen abhängen.

Abschnitt 1 prüft und korrigiert ein nullObjekt. Randnotiz: Beim Erstellen der Liste werden keine untergeordneten Elemente erstellt (z CalendarRow. B. ).

Abschnitt 2 prüft und korrigiert Benutzer- und / oder Implementierungsfehler. Nur weil Sie ein haben, List<CalendarRow>heißt das nicht, dass Sie Elemente in der Liste haben. Benutzer und Implementierer werden Dinge tun, die Sie sich nicht vorstellen können, nur weil sie es dürfen, ob es für Sie sinnvoll ist oder nicht.

Ich denke, diese Frage hängt im Grunde vom Geschmack ab. Ja, es ist eine gute Idee, robusten Code zu schreiben, aber der Code in Ihrem Beispiel verstößt geringfügig gegen das KISS-Prinzip (so viel "zukunftssicherer" Code wird es auch sein).

Ich persönlich würde mich wahrscheinlich nicht die Mühe machen, Code für die Zukunft kugelsicher zu machen. Ich kenne die Zukunft nicht und als solche ist ein solcher "zukunftssicherer" Code zum Scheitern verurteilt, wenn die Zukunft sowieso ankommt.

Stattdessen würde ich einen anderen Ansatz bevorzugen: Machen Sie die Annahmen, die Sie explizit machen, mithilfe des assert()Makros oder ähnlicher Funktionen. Auf diese Weise erfahren Sie, wenn die Zukunft vor der Tür steht, genau, wo Ihre Annahmen nicht mehr zutreffen.

Ein weiteres Prinzip, über das Sie nachdenken sollten, ist die Idee, schnell zu versagen . Die Idee ist, dass Sie, wenn in Ihrem Programm etwas schief geht, das Programm sofort beenden möchten, zumindest während Sie es entwickeln, bevor Sie es freigeben. Unter diesem Prinzip möchten Sie viele Überprüfungen durchführen, um sicherzustellen, dass Ihre Annahmen stimmen. Sie sollten jedoch ernsthaft in Betracht ziehen, dass Ihr Programm bei Verstößen gegen die Annahmen nicht mehr funktioniert.

Um es kühn auszudrücken: Wenn in Ihrem Programm sogar ein kleiner Fehler auftritt, möchten Sie, dass es beim Ansehen vollständig abstürzt!

Dies hört sich vielleicht nicht intuitiv an, lässt Sie jedoch Fehler während der Routineentwicklung so schnell wie möglich erkennen. Wenn Sie einen Code schreiben und glauben, dass er fertig ist, aber beim Testen abstürzt, besteht kein Zweifel, dass Sie noch nicht fertig sind. Darüber hinaus bieten die meisten Programmiersprachen hervorragende Debugging-Tools, die am einfachsten zu verwenden sind, wenn Ihr Programm vollständig abstürzt, anstatt nach einem Fehler zu versuchen, sein Bestes zu geben. Das häufigste Beispiel ist, dass die Ausnahmemeldung beim Absturz Ihres Programms durch Auslösen einer nicht behandelten Ausnahme eine unglaubliche Menge an Informationen über den Fehler enthält, einschließlich der fehlerhaften Codezeile und des Pfades durch Ihren Code, den das Programm angenommen hat seinen Weg zu dieser Codezeile (dem Stack-Trace).

Lesen Sie für weitere Gedanken diesen kurzen Aufsatz: Nageln Sie Ihr Programm nicht in aufrechter Position

Dies ist für Sie relevant, da die von Ihnen geschriebenen Prüfungen möglicherweise manchmal vorhanden sind, weil Sie möchten, dass Ihr Programm versucht, weiterzulaufen, selbst wenn ein Fehler aufgetreten ist. Betrachten Sie zum Beispiel diese kurze Implementierung der Fibonacci-Sequenz:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Das funktioniert, aber was ist, wenn jemand Ihrer Funktion eine negative Zahl übergibt? Es wird dann nicht funktionieren! Fügen Sie daher ein Häkchen hinzu, um sicherzustellen, dass die Funktion mit einer nichtnegativen Eingabe aufgerufen wird.

Es könnte verlockend sein, die Funktion wie folgt zu schreiben:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    // Make sure the input is nonnegative
    if(n < 0) {
        n = 1; // Replace the negative input with an input that will work
    }

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Wenn Sie dies jedoch tun und später versehentlich Ihre Fibonacci-Funktion mit einem negativen Eingang aufrufen, werden Sie es nie bemerken! Schlimmer noch, Ihr Programm wird wahrscheinlich weiter ausgeführt, aber es liefert unsinnige Ergebnisse, ohne dass Sie Hinweise darauf erhalten, wo etwas schief gelaufen ist. Dies sind die am schwersten zu behebenden Fehler.

Stattdessen ist es besser, einen Scheck wie folgt zu schreiben:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    // Make sure the input is nonnegative
    if(n < 0) {
        throw new ArgumentException("Can't have negative inputs to Fibonacci");
    }

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Wenn Sie jetzt versehentlich die Fibonacci-Funktion mit einer negativen Eingabe aufrufen, wird Ihr Programm sofort gestoppt und Sie werden informiert, dass etwas nicht stimmt. Durch die Angabe eines Stack-Trace informiert Sie das Programm darüber, welcher Teil Ihres Programms versucht hat, die Fibonacci-Funktion falsch auszuführen, und bietet Ihnen einen hervorragenden Ausgangspunkt für die Fehlersuche.

Sollten Sie redundanten Code hinzufügen? Nein.

Was Sie beschreiben, ist jedoch kein redundanter Code .

Was Sie beschreiben, ist eine defensive Programmierung gegen den Aufruf von Code, der die Voraussetzungen Ihrer Funktion verletzt. Ob Sie dies tun oder es einfach den Benutzern überlassen, die Dokumentation zu lesen und diese Verstöße selbst zu vermeiden, ist völlig subjektiv.

Persönlich bin ich ein großer Anhänger dieser Methodik, aber wie bei allem muss man vorsichtig sein. Nehmen Sie zum Beispiel C ++ std::vector::operator[]. Abgesehen von der Debug-Implementierung von VS führt diese Funktion keine Begrenzungsprüfung durch. Wenn Sie ein Element anfordern, das nicht existiert, ist das Ergebnis undefiniert. Es liegt an dem Benutzer, einen gültigen Vektorindex bereitzustellen. Dies ist sehr bewusst: Sie können sich für die Überprüfung der Grenzen "anmelden", indem Sie sie auf der Aufrufseite hinzufügen. Wenn die operator[]Implementierung dies jedoch durchführen würde, könnten Sie sich nicht "abmelden". Als relativ niedrige Funktion ist dies sinnvoll.

Wenn Sie jedoch eine AddEmployee(string name)Funktion für eine übergeordnete Schnittstelle schreiben , würde ich davon ausgehen, dass diese Funktion zumindest eine Ausnahme auslöst name, wenn Sie eine leere bereitstellen. Diese Voraussetzung wird unmittelbar über der Funktionsdeklaration dokumentiert. Möglicherweise geben Sie heute noch keine unbefleckten Benutzereingaben für diese Funktion ein. Wenn Sie sie jedoch auf diese Weise "sicher" machen, können künftig auftretende Verstöße gegen die Voraussetzungen einfach diagnostiziert werden, anstatt möglicherweise eine Kette von schwer zu bestimmenden Dominosteinen auszulösen. Fehler erkennen. Das ist keine Redundanz, sondern Fleiß.

Wenn ich eine allgemeine Regel ausarbeiten müsste (obwohl ich in der Regel versuche, diese zu vermeiden), würde ich sagen, dass eine Funktion eine der folgenden Bedingungen erfüllt:

• lebt in einer Hochsprache (zB JavaScript statt C)
• sitzt an einer Schnittstellengrenze
• ist nicht leistungskritisch
• Akzeptiert Benutzereingaben direkt

… Kann von defensiver Programmierung profitieren. In anderen Fällen können Sie weiterhin assertIonen schreiben , die während des Testens ausgelöst werden, in Release-Builds jedoch deaktiviert sind, um Ihre Fähigkeit, Fehler zu finden, weiter zu verbessern.

Dieses Thema wird auf Wikipedia weiter untersucht ( https://en.wikipedia.org/wiki/Defensive_programming ).

Zwei der zehn Programmiergebote sind hier relevant:

• Du sollst nicht davon ausgehen, dass die Eingabe korrekt ist

• Du sollst keinen Code für die zukünftige Verwendung machen

Hier bedeutet das Überprüfen auf Null nicht, dass Code für die zukünftige Verwendung erstellt wird. Das Erstellen von Code für die zukünftige Verwendung umfasst das Hinzufügen von Schnittstellen, da Sie glauben, dass sie "eines Tages" nützlich sein könnten. Mit anderen Worten, das Gebot lautet, keine Abstraktionsebenen hinzuzufügen, es sei denn, sie werden gerade benötigt.

Die Überprüfung auf null hat nichts mit der zukünftigen Verwendung zu tun. Es hat mit dem Gebot Nr. 1 zu tun: Gehen Sie nicht davon aus, dass die Eingabe korrekt ist. Gehen Sie niemals davon aus, dass Ihre Funktion eine Teilmenge der Eingaben empfängt. Eine Funktion sollte logisch reagieren, egal wie falsch und durcheinander die Eingänge sind.

Die Definition von "redundantem Code" und "YAGNI" hängt oft davon ab, wie weit Sie in die Zukunft schauen.

Wenn Sie auf ein Problem stoßen, neigen Sie dazu, zukünftigen Code so zu schreiben, dass dieses Problem vermieden wird. Ein anderer Programmierer, der dieses spezielle Problem nicht erlebt hat, hält Ihren Code möglicherweise für überflüssig.

Mein Vorschlag ist, nachzuverfolgen, wie viel Zeit Sie für "noch nicht fehlerbehaftete Inhalte" aufwenden, wenn die Datenmengen und Ihre Kollegen die Funktionen schneller ausstoßen als Sie, und diese dann zu verringern.

Wenn Sie jedoch so sind wie ich, dann haben Sie wahrscheinlich alles "standardmäßig" eingegeben und es hat Sie nicht mehr wirklich gekostet.

Es ist eine gute Idee, alle Annahmen zu Parametern zu dokumentieren. Und es ist eine gute Idee, zu überprüfen, ob Ihr Client-Code diese Annahmen nicht verletzt. Ich würde das machen:

/** ...
*   Precondition: rows is null or nonempty
*/
public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    Assert( rows==null || rows.Count > 0 )
    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

[Unter der Annahme, dass dies C # ist, ist Assert möglicherweise nicht das beste Tool für den Job, da es nicht in freigegebenem Code kompiliert ist. Aber das ist eine Debatte für einen anderen Tag.]

Warum ist das besser als das, was du geschrieben hast? Ihr Code ist sinnvoll, wenn in einer Zukunft, in der sich Ihr Kunde geändert hat, beim Übergeben einer leeren Liste eine erste Zeile hinzugefügt und den Terminen eine App hinzugefügt werden soll. Aber woher wissen Sie, dass dies der Fall sein wird? Es ist besser, jetzt weniger Annahmen über die Zukunft zu treffen.

Schätzen Sie jetzt die Kosten für das Hinzufügen dieses Codes . Es wird relativ billig sein, weil es in Ihrem Kopf alles frisch ist, so dass Sie dies schnell tun können. Das Hinzufügen von Komponententests ist notwendig - es gibt nichts Schlimmeres, als ein Jahr später eine Methode zu verwenden, die nicht funktioniert, und Sie stellen fest, dass sie von Anfang an fehlerhaft war und nie wirklich funktioniert hat.

Schätzen Sie die Kosten für das Hinzufügen des Codes bei Bedarf. Es wird teurer, weil Sie zum Code zurückkehren müssen, sich alles merken müssen, und es ist viel schwieriger.

Schätzen Sie die Wahrscheinlichkeit, dass der zusätzliche Code tatsächlich benötigt wird. Dann mach die Mathe.

Andererseits ist Code, der mit den Annahmen "X wird niemals passieren" voll ist, für das Debuggen schrecklich. Wenn etwas nicht wie beabsichtigt funktioniert, bedeutet dies entweder einen dummen Fehler oder eine falsche Annahme. Dein "X wird niemals passieren" ist eine Vermutung, und bei Vorhandensein eines Fehlers ist es verdächtig. Was den nächsten Entwickler dazu zwingt, Zeit darauf zu verschwenden. Es ist normalerweise besser, sich nicht auf solche Annahmen zu verlassen.

Die Hauptfrage lautet hier: "Was passiert, wenn Sie es tun / nicht tun?"

Wie andere betont haben, ist diese Art der defensiven Programmierung gut, aber gelegentlich auch gefährlich.

Wenn Sie beispielsweise einen Standardwert angeben, wird das Programm fortgesetzt. Aber das Programm macht jetzt möglicherweise nicht das, was Sie wollen. Wenn dieses leere Array beispielsweise in eine Datei geschrieben wird, haben Sie Ihren Fehler möglicherweise von "Abstürze, weil ich versehentlich null eingegeben habe" in "Löschen der Kalenderzeilen, weil ich versehentlich null eingegeben habe" geändert. (Zum Beispiel, wenn Sie anfangen , Dinge zu entfernen , die nicht in der Liste in dem Teil enthalten sind, in dem "// blah" steht.)

Der Schlüssel für mich ist Nie korrupte Daten . Lassen Sie mich das wiederholen; NOCH NIE. KORRUPT. DATEN. Wenn Ihr Programm ausfällt, erhalten Sie einen Fehlerbericht, den Sie patchen können. wenn es schlechte Daten in eine Datei schreibt, die es später wird, müssen Sie den Boden mit Salz säen.

Alle Ihre "unnötigen" Entscheidungen sollten unter Berücksichtigung dieser Prämisse getroffen werden.

Was Sie hier zu tun haben, ist im Grunde eine Schnittstelle. Durch das Hinzufügen des Verhaltens "Wenn Eingabe ist null, Eingabe initialisieren" haben Sie die Methodenschnittstelle effektiv erweitert. Anstatt immer eine gültige Liste zu bearbeiten, haben Sie die Eingabe "repariert". Unabhängig davon, ob dies der offizielle oder inoffizielle Teil der Benutzeroberfläche ist, können Sie wetten, dass jemand (höchstwahrscheinlich auch Sie) dieses Verhalten verwenden wird.

Schnittstellen sollten einfach gehalten und relativ stabil sein - insbesondere in einer Art public staticMethode. Sie haben ein wenig Spielraum bei privaten Methoden, insbesondere bei privaten Instanzmethoden. Indem Sie die Schnittstelle implizit erweitern, haben Sie Ihren Code in der Praxis komplexer gemacht. Stellen Sie sich vor, Sie möchten diesen Codepfad nicht wirklich verwenden - vermeiden Sie ihn also. Jetzt haben Sie ein wenig ungeprüften Code, der so tut , als sei er Teil des Verhaltens der Methode. Und ich kann Ihnen jetzt sagen, dass es wahrscheinlich einen Fehler gibt: Wenn Sie eine Liste übergeben, wird diese Liste durch die Methode mutiert. Wenn Sie dies jedoch nicht tun, erstellen Sie eine lokaleListe, und werfen Sie es später weg. Dies ist die Art von inkonsistentem Verhalten, das Sie in einem halben Jahr zum Weinen bringt, wenn Sie versuchen, einen obskuren Fehler aufzuspüren.

Im Allgemeinen ist defensives Programmieren eine ziemlich nützliche Sache. Die Codepfade für die Abwehrprüfungen müssen jedoch wie jeder andere Code getestet werden. In einem solchen Fall verkomplizieren sie Ihren Code ohne Grund, und ich würde stattdessen eine Alternative wie die folgende wählen:

if (rows == null) throw new ArgumentNullException(nameof(rows));

Sie möchten keine Eingabe mit dem rowsWert null, und Sie möchten den Fehler so schnell wie möglich für alle Anrufer sichtbar machen .

Es gibt viele Werte, mit denen Sie bei der Entwicklung von Software herumspielen müssen. Selbst die Robustheit selbst ist eine sehr komplexe Eigenschaft - ich würde beispielsweise Ihren Defensiv-Check nicht für robuster halten, als eine Ausnahme auszulösen. Ausnahmen sind recht praktisch, um Ihnen einen sicheren Ort zu bieten, an dem Sie es erneut versuchen können - Datenkorruptionsprobleme sind normalerweise viel schwieriger zu verfolgen, als ein Problem frühzeitig zu erkennen und sicher zu behandeln. Letztendlich geben sie Ihnen nur eine Illusion von Robustheit, und dann, einen Monat später, bemerken Sie, dass ein Zehntel Ihrer Termine weg ist, weil Sie nie bemerkt haben, dass eine andere Liste aktualisiert wurde. Autsch.

Achten Sie darauf, zwischen den beiden zu unterscheiden. Defensives Programmieren ist eine nützliche Technik, um Fehler an einer Stelle zu finden, an der sie am relevantesten sind, was Ihre Debug-Bemühungen erheblich unterstützt und mit einer guten Ausnahmebehandlung "hinterhältige Korruption" verhindert. Früh scheitern, schnell scheitern. Auf der anderen Seite ist das, was Sie tun, eher wie "Verstecken von Fehlern" - Sie jonglieren mit Eingaben und machen Annahmen darüber, was der Anrufer gemeint hat. Dies ist sehr wichtig für benutzerbezogenen Code (z. B. Rechtschreibprüfung), aber Sie sollten vorsichtig sein, wenn Sie diesen Code mit Entwicklerbezug sehen.

Das Hauptproblem ist, dass die Abstraktion, die Sie machen, auslaufen wird ("Ich wollte eine Rechtschreibprüfung durchführen, nicht nur!"), Und der Code, mit dem Sie alle Sonderfälle und Korrekturen behandeln können, ist immer noch Ihr Code müssen warten und verstehen, und Code, den Sie testen müssen. Vergleichen Sie den Aufwand, um sicherzustellen, dass eine Nicht-Null-Liste übergeben wird, mit der Behebung des Fehlers, den Sie ein Jahr später in der Produktion haben - es ist kein guter Kompromiss. In einer idealen Welt möchten Sie, dass jede Methode ausschließlich mit ihrer eigenen Eingabe arbeitet, ein Ergebnis zurückgibt und keinen globalen Status ändert. In der realen Welt gibt es natürlich viele Fälle, in denen dies nicht der Fall istDie einfachste und übersichtlichste Lösung (z. B. beim Speichern einer Datei), aber ich finde, dass die Beibehaltung von Methoden "rein", wenn es keinen Grund gibt, den globalen Status zu lesen oder zu manipulieren, den Code viel einfacher zum Nachdenken macht. Es gibt Ihnen auch mehr natürliche Punkte für die Aufteilung Ihrer Methoden :)

Dies bedeutet nicht, dass alles Unerwartete zum Absturz Ihrer Anwendung führen sollte, im Gegenteil. Wenn Sie Ausnahmen gut verwenden, bilden sie natürlich sichere Fehlerbehandlungspunkte, an denen Sie einen stabilen Anwendungsstatus wiederherstellen und dem Benutzer ermöglichen können, ihre Arbeit fortzusetzen (idealerweise unter Vermeidung von Datenverlusten für den Benutzer). An diesen Bearbeitungspunkten sehen Sie Möglichkeiten, die Probleme zu beheben ("Keine Bestellnummer 2212 gefunden. Meinten Sie 2212b?") Oder dem Benutzer die Kontrolle zu geben ("Fehler beim Herstellen einer Verbindung zur Datenbank. Erneut versuchen?"). Selbst wenn keine solche Option verfügbar ist, haben Sie zumindest die Möglichkeit, dass nichts beschädigt wurde. Ich habe angefangen, Code zu schätzen, der verwendet usingund try... finallyviel mehr als try...catchEs bietet Ihnen viele Möglichkeiten, Invarianten auch unter außergewöhnlichen Bedingungen zu erhalten.

Benutzer sollten ihre Daten und Arbeit nicht verlieren. Dies muss noch mit den Entwicklungskosten usw. abgeglichen werden, aber es ist eine ziemlich gute allgemeine Richtlinie (wenn die Benutzer entscheiden, ob sie Ihre Software kaufen oder nicht - interne Software hat normalerweise keinen solchen Luxus). Sogar das Abstürzen der gesamten Anwendung wird weniger problematisch, wenn der Benutzer einfach neu starten und zu dem zurückkehren kann, was er gerade getan hat. Dies ist eine echte Robustheit - Word spart Ihre Arbeit die ganze Zeit, ohne das Dokument auf der Festplatte zu beschädigen, und bietet Ihnen eine Optionum diese Änderungen nach einem Neustart von Word nach einem Absturz wiederherzustellen. Ist es überhaupt besser als ein No Bug? Wahrscheinlich nicht - aber vergessen Sie nicht, dass die geleistete Arbeit, um einen seltenen Fehler zu finden, überall besser ausgegeben werden könnte. Aber es ist viel besser als die Alternativen - z. B. ein beschädigtes Dokument auf der Festplatte, die gesamte Arbeit seit dem letzten Speichern ist verloren gegangen, das Dokument wird automatisch durch Änderungen vor dem Absturz ersetzt, die zufällig Strg + A und Löschen lauteten.

Ich werde dies beantworten, basierend auf Ihrer Annahme, dass robuster Code Ihnen in "Jahren" von jetzt an zugute kommen wird. Wenn langfristige Vorteile Ihr Ziel sind, würde ich Design und Wartbarkeit vor Robustheit priorisieren.

Der Kompromiss zwischen Design und Robustheit ist Zeit und Fokus. Die meisten Entwickler würden lieber einen Satz gut gestalteten Codes haben, selbst wenn dies bedeutet, dass sie einige Fehlerquellen durchlaufen und zusätzliche Bedingungen oder Fehlerbehandlungen durchführen müssen. Nach einigen Jahren des Gebrauchs wurden die Stellen, an denen Sie es wirklich benötigen, wahrscheinlich von Benutzern identifiziert.

Vorausgesetzt, das Design hat ungefähr die gleiche Qualität, ist weniger Code einfacher zu warten. Das bedeutet nicht, dass wir besser dran sind, wenn Sie bekannte Probleme für ein paar Jahre loslassen. Das Hinzufügen von Dingen, von denen Sie wissen, dass Sie sie nicht benötigen, macht es jedoch schwierig. Wir haben uns alle den alten Code angesehen und unnötige Teile gefunden. Sie müssen über ein hohes Maß an Vertrauen verfügen, um den Code zu ändern, der seit Jahren funktioniert.

Wenn Sie also der Meinung sind, dass Ihre App so gut wie möglich gestaltet ist, einfach zu warten ist und keine Fehler aufweist, sollten Sie etwas Besseres finden, als nicht benötigten Code hinzuzufügen. Es ist das Mindeste, was Sie aus Respekt vor allen anderen Entwicklern tun können, die stundenlang an sinnlosen Funktionen arbeiten.

Nein solltest du nicht. Und Sie beantworten tatsächlich Ihre eigene Frage, wenn Sie feststellen, dass diese Art der Codierung Fehler verbergen kann . Dies macht den Code nicht robuster, sondern anfälliger für Fehler und erschwert das Debuggen.

Sie geben Ihre aktuellen Erwartungen an das rowsArgument an: Entweder ist es null oder es hat mindestens ein Element. Die Frage ist also: Ist es eine gute Idee, den Code zu schreiben, um zusätzlich den unerwarteten dritten Fall zu behandeln, bei dem rowskeine Elemente vorhanden sind?

Die Antwort ist nein. Bei unerwarteten Eingaben sollten Sie immer eine Ausnahme auslösen. Beachten Sie Folgendes: Wenn andere Teile des Codes die Erwartungen (dh den Vertrag) Ihrer Methode verletzen, liegt ein Fehler vor . Wenn es einen Fehler gibt, möchten Sie ihn so früh wie möglich erkennen, damit Sie ihn beheben können. Eine Ausnahme hilft Ihnen dabei.

Was der Code derzeit tut, ist zu erraten, wie man einen Fehler behebt, der im Code vorhanden sein kann oder nicht. Aber selbst wenn es einen Fehler gibt, kann man nicht wissen, wie man sich vollständig davon erholt. Bugs haben per Definition unbekannte Konsequenzen. Möglicherweise wurde ein Teil des Initialisierungscodes nicht wie erwartet ausgeführt. Dies kann jedoch viele andere Konsequenzen haben als nur die fehlende Zeile.

Ihr Code sollte also so aussehen:

public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    if (rows != null && rows.Count == 0) throw new ArgumentException("Rows is empty."); 

    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

Hinweis: In bestimmten Fällen ist es sinnvoll, zu "raten", wie ungültige Eingaben behandelt werden sollen, anstatt nur eine Ausnahme auszulösen. Wenn Sie beispielsweise externe Eingaben verarbeiten, haben Sie keinen Einfluss darauf. Webbrowser sind ein berüchtigtes Beispiel, weil sie versuchen, jede Art von fehlerhafter und ungültiger Eingabe angemessen zu handhaben. Dies ist jedoch nur bei externen Eingaben sinnvoll, nicht bei Aufrufen aus anderen Programmteilen.

Bearbeiten: In einigen anderen Antworten wird angegeben, dass Sie defensives Programmieren ausführen . Ich stimme dir nicht zu. Defensive Programmierung bedeutet, dass Sie nicht automatisch darauf vertrauen, dass Eingaben gültig sind. Die Validierung von Parametern (wie oben) ist also eine defensive Programmiertechnik. Dies bedeutet jedoch nicht, dass Sie unerwartete oder ungültige Eingaben durch Erraten ändern sollten. Der robuste defensive Ansatz besteht darin, die Eingabe zu validieren und im Falle einer unerwarteten oder ungültigen Eingabe eine Ausnahme auszulösen.

Sollte ich jetzt redundanten Code hinzufügen, nur für den Fall, dass er in Zukunft benötigt wird?

Sie sollten zu keinem Zeitpunkt redundanten Code hinzufügen.

Sie sollten keinen Code hinzufügen, der nur in Zukunft benötigt wird.

Sie sollten sicherstellen, dass sich Ihr Code gut verhält, egal was passiert.

Die Definition von "gut benehmen" richtet sich nach Ihren Bedürfnissen. Eine Technik, die ich gerne verwende, sind "Paranoia" -Ausnahmen. Wenn ich zu 100% sicher bin, dass ein bestimmter Fall niemals eintreten kann, programmiere ich immer noch eine Ausnahme, aber ich tue dies so, dass a) allen klar mitteilt, dass ich dies niemals erwarte, und b) klar angezeigt und protokolliert wird und Dies führt später nicht zu schleichender Korruption.

Beispiel Pseudocode:

file = File.open(">", "bla")  or raise "Paranoia: cannot open file 'bla'"

file.write("xytz") or raise "Paranoia: disk full?"

file.close()  or raise "Paranoia: huh?!?!?"

Dies zeigt deutlich, dass ich zu 100% sicher bin, dass ich die Datei immer öffnen, schreiben oder schließen kann, dh ich gehe nicht in den Rahmen einer aufwändigen Fehlerbehandlung. Wenn ich die Datei jedoch nicht öffnen kann, schlägt mein Programm dennoch kontrolliert fehl.

Die Benutzeroberfläche zeigt dem Benutzer solche Meldungen natürlich nicht an, sie werden intern zusammen mit dem Stack-Trace protokolliert. Auch dies sind interne "Paranoia" -Ausnahmen, die nur sicherstellen, dass der Code "stoppt", wenn etwas Unerwartetes passiert. Dieses Beispiel ist etwas umständlich, in der Praxis würde ich natürlich eine echte Fehlerbehandlung für Fehler beim Öffnen von Dateien implementieren, da dies regelmäßig vorkommt (falscher Dateiname, schreibgeschützter USB-Stick, was auch immer).

Ein sehr wichtiger verwandter Suchbegriff wäre, wie in anderen Antworten erwähnt, "ausfallen schnell" und ist sehr nützlich, um robuste Software zu erstellen.

Hier gibt es viele überkomplizierte Antworten. Sie haben diese Frage wahrscheinlich gestellt, weil Sie sich in Bezug auf diesen Artikelcode nicht richtig gefühlt haben, sich aber nicht sicher waren, warum oder wie Sie ihn beheben sollen. Meine Antwort lautet also, dass das Problem (wie immer) sehr wahrscheinlich in der Codestruktur liegt.

Zunächst der Methodenkopf:

public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)

Vergeben Termin zu welcher Zeile? Das sollte sofort aus der Parameterliste ersichtlich sein. Ohne weitere Kenntnisse, würde ich die Methode params wie folgt aussehen erwarten (Appointment app, CalendarRow row).

Als nächstes werden die "Eingabeprüfungen":

//1. Is rows equal to null? - This will be the case if this is the first appointment.
if (rows == null) {
    rows = new List<CalendarRow> ();
}

//2. Is rows empty? - This will be the case if this is the first appointment / some other unknown reason.
if(rows.Count == 0)
{
    rows.Add (new CalendarRow (0));
    rows [0].Appointments.Add (app);
}

Das ist Quatsch.

1. check) Der Methodenaufrufer sollte nur sicherstellen, dass er keine nicht initialisierten Werte innerhalb der Methode übergibt. Es ist die Verantwortung eines Programmierers (zu versuchen), nicht dumm zu sein.
2. check) Wenn ich nicht berücksichtige, dass die Übergabe rowsan die Methode wahrscheinlich falsch ist (siehe Kommentar oben), sollte es nicht die Verantwortung der aufgerufenen Methode sein AssignAppointmentToRow, Zeilen auf andere Weise zu manipulieren, als den Termin irgendwo zuzuweisen.

Aber das ganze Konzept, Termine irgendwo zuzuweisen, ist seltsam (es sei denn, dies ist ein GUI-Teil des Codes). Ihr Code scheint eine explizite Datenstruktur zu enthalten (oder versucht es zumindest), die einen Kalender darstellt (das heißt, List<CalendarRows><- das sollte als Calendarirgendwo definiert werden, wenn Sie diesen Weg einschlagen möchten, dann würden Sie Calendar calendarzu Ihrer Methode übergehen). Wenn Sie diesen Weg gehen, würde ich erwarten calendar, dass die mit Slots gefüllt werden, in denen Sie die Termine danach platzieren (zuweisen) (z. B. calendar[month][day] = appointmentwäre der entsprechende Code). Sie können aber auch die Kalenderstruktur ganz von der Hauptlogik streichen und nur angeben, List<Appointment>wo die AppointmentObjekte Attribute enthaltendate. Wenn Sie dann irgendwo in der GUI einen Kalender rendern müssen, können Sie diese "explizite Kalender" -Struktur direkt vor dem Rendern erstellen.

Ich kenne die Details Ihrer App nicht, daher trifft ein Teil davon möglicherweise nicht auf Sie zu, aber beide Überprüfungen (hauptsächlich die zweite) sagen mir, dass irgendwo etwas mit der Trennung von Bedenken in Ihrem Code nicht stimmt .

Nehmen wir der Einfachheit halber an, dass Sie diesen Code irgendwann in N Tagen (nicht später oder früher) oder überhaupt nicht mehr benötigen.

Pseudocode:

let C_now   = cost of implementing the piece of code now
let C_later = ... later
let C_maint = cost of maintaining the piece of code one day
              (note that it can be negative)
let P_need  = probability that you will need the code after N days

if C_now + C_maint * N < P_need*C_later then implement the code else omit it.

Faktoren für C_maint:

• Verbessert es den Code im Allgemeinen und macht ihn selbstdokumentierender und einfacher zu testen? Wenn ja, negativ C_mainterwartet
• Wird der Code dadurch größer (daher schwerer zu lesen, länger zu kompilieren, Tests durchzuführen usw.)?
• Stehen noch Überarbeitungen / Redesigns an? Wenn ja, Stoß C_maint. Dieser Fall erfordert eine komplexere Formel mit mehr Zeiten wie Variablen N.

Eine so große Sache, die den Code nur beschwert und möglicherweise nur in 2 Jahren mit geringer Wahrscheinlichkeit benötigt wird, sollte weggelassen werden, aber eine kleine Sache, die auch nützliche Behauptungen hervorbringt und 50%, dass sie in 3 Monaten benötigt wird, sollte implementiert werden .