Ist es ein schlechter Stil, eine Bedingung redundant zu überprüfen?

Ich komme oft zu Stellen in meinem Code, an denen ich immer wieder eine bestimmte Bedingung überprüfe.

Ich möchte Ihnen ein kleines Beispiel geben: Angenommen, es gibt eine Textdatei, die Zeilen enthält, die mit "a" beginnen, Zeilen, die mit "b" beginnen, und andere Zeilen, und ich möchte eigentlich nur mit den ersten beiden Arten von Zeilen arbeiten. Mein Code würde ungefähr so ​​aussehen (mit Python, aber als Pseudocode lesen):

# ...
clear_lines() # removes every other line than those starting with "a" or "b"
for line in lines:
    if (line.startsWith("a")):
        # do stuff
    elif (line.startsWith("b")):
        # magic
    else:
        # this else is redundant, I already made sure there is no else-case
        # by using clear_lines()
# ...

Sie können sich vorstellen, dass ich diesen Zustand hier nicht nur überprüfe, sondern vielleicht auch in anderen Funktionen und so weiter.

Betrachten Sie es als Rauschen oder fügt es meinem Code einen Wert hinzu?

Dies ist eine äußerst übliche Praxis, und der Umgang damit erfolgt über Filter höherer Ordnung .

Im Wesentlichen übergeben Sie der Filtermethode eine Funktion zusammen mit der Liste / Sequenz, nach der Sie filtern möchten, und die resultierende Liste / Sequenz enthält nur die gewünschten Elemente.

Ich bin mit der Python-Syntax nicht vertraut (obwohl sie eine solche Funktion enthält, wie im obigen Link gezeigt), aber in c # / f # sieht es so aus:

c #:

var linesWithAB = lines.Where(l => l.StartsWith("a") || l.StartsWith("b"));
foreach (var line in linesWithAB)
{
    /* line is guaranteed to ONLY start with a or b */
}

f # (setzt ienumerable voraus, andernfalls würde List.filter verwendet):

let linesWithAB = lines
    |> Seq.filter (fun l -> l.StartsWith("a") || l.StartsWith("b"))

for line in linesWithAB do
    /* line is guaranteed to ONLY start with a or b */

Um es klar zu sagen: Wenn Sie bewährten Code / Muster verwenden, ist dies ein schlechter Stil. Wenn Sie die Liste im Speicher so ändern, wie Sie es über clear_lines () scheinen, verlieren Sie die Thread-Sicherheit und alle Hoffnungen auf Parallelität, die Sie hätten haben können.

Ich musste kürzlich einen Firmware-Programmierer im Motorola S-Record- Format implementieren , der dem, was Sie beschreiben, sehr ähnlich ist. Da wir etwas Zeitdruck hatten, ignorierte mein erster Entwurf Redundanzen und machte Vereinfachungen basierend auf der Teilmenge, die ich tatsächlich in meiner Anwendung verwenden musste. Es hat meine Tests problemlos bestanden, ist aber schwer gescheitert, sobald es jemand anderes versucht hat. Es gab keine Ahnung, wo das Problem lag. Es kam den ganzen Weg durch, scheiterte aber am Ende.

Ich hatte also keine andere Wahl, als alle redundanten Überprüfungen durchzuführen, um einzugrenzen, wo das Problem lag. Danach habe ich ungefähr zwei Sekunden gebraucht, um das Problem zu finden.

Ich brauchte vielleicht zwei Stunden mehr, um es richtig zu machen, verschwendete aber auch einen Tag der Zeit anderer Leute mit der Fehlerbehebung. Es ist sehr selten, dass einige Prozessorzyklen einen Tag der verschwendeten Fehlerbehebung wert sind.

Wenn Sie jedoch Dateien lesen, ist es häufig von Vorteil, Ihre Software so zu gestalten, dass sie zeilenweise gelesen und verarbeitet wird, anstatt die gesamte Datei in den Speicher einzulesen und im Speicher zu verarbeiten. Auf diese Weise funktioniert es auch bei sehr großen Dateien.

Sie können für den elseFall eine Ausnahme auslösen. Auf diese Weise ist es nicht redundant. Ausnahmen sind Dinge, die nicht passieren sollen, aber trotzdem überprüft werden.

clear_lines() # removes every other line than those starting with "a" or "b"
for line in lines:
    if (line.startsWith("a)):
        # do stuff
    if (line.startsWith("b")):
        # magic
    else:
        throw BadLineException
# ...

Bei der vertraglichen Gestaltung muss jede Funktion ihre Aufgabe wie in der Dokumentation beschrieben erfüllen. Jede Funktion verfügt also über eine Liste von Vorbedingungen, dh Bedingungen an den Eingängen der Funktion sowie Nachbedingungen, dh Bedingungen der Ausgabe der Funktion.

Die Funktion muss ihren Kunden garantieren, dass, wenn die Eingaben die Vorbedingungen erfüllen, die Ausgabe den in den Nachbedingungen beschriebenen entspricht. Wenn mindestens eine der Voraussetzungen nicht eingehalten wird, kann die Funktion tun, was sie will (Absturz, Ergebnis zurückgeben, ...). Daher sind Vor- und Nachbedingungen eine semantische Beschreibung der Funktion.

Dank des Vertrags ist eine Funktion sicher, dass ihre Clients sie korrekt verwenden, und ein Client ist sicher, dass die Funktion ihre Arbeit korrekt ausführt.

Einige Sprachen bearbeiten Verträge nativ oder über ein spezielles Framework. Für die anderen ist es am besten, die Vor- und Nachbedingungen dank Asserts zu überprüfen, wie @Lattyware sagte. Aber ich würde das nicht als defensive Programmierung bezeichnen, da sich dieses Konzept meiner Meinung nach mehr auf den Schutz vor Eingaben des (menschlichen) Benutzers konzentriert.

Wenn Sie Verträge ausnutzen, können Sie die redundant überprüfte Bedingung vermeiden, da entweder die aufgerufene Funktion einwandfrei funktioniert und Sie keine doppelte Überprüfung benötigen oder die aufgerufene Funktion nicht funktioniert und die aufrufende Funktion sich wie gewünscht verhalten kann.

Der schwierigere Teil besteht dann darin, zu definieren, welche Funktion für was verantwortlich ist, und diese Rollen streng zu dokumentieren.

Sie brauchen die clear_lines () am Anfang eigentlich nicht. Wenn die Zeile weder "a" noch "b" ist, werden die Bedingungen einfach nicht ausgelöst. Wenn Sie diese Zeilen entfernen möchten, machen Sie das else zu einer clear_line (). Derzeit führen Sie zwei Durchgänge durch Ihr Dokument durch. Wenn Sie clear_lines () am Anfang überspringen und dies als Teil der foreach-Schleife tun, halbieren Sie Ihre Verarbeitungszeit.

Es ist nicht nur ein schlechter Stil, es ist auch rechnerisch schlecht.

Wenn Sie tatsächlich etwas tun möchten, wenn Sie eine ungültige Zeichenfolge finden (z. B. Debug-Text ausgeben), würde ich sagen, dass dies absolut in Ordnung ist. Ein paar zusätzliche Zeilen und ein paar Monate später, wenn es aus einem unbekannten Grund nicht mehr funktioniert, können Sie sich die Ausgabe ansehen, um herauszufinden, warum.

Wenn es jedoch sicher ist, es einfach zu ignorieren, oder Sie sicher sind, dass Sie niemals eine ungültige Zeichenfolge erhalten, ist der zusätzliche Zweig nicht erforderlich.

Persönlich bin ich immer dafür, mindestens eine Trace-Ausgabe für unerwartete Zustände einzugeben - es macht das Leben viel einfacher, wenn Sie einen Fehler mit angehängter Ausgabe haben, der Ihnen genau sagt, was schief gelaufen ist.

... Angenommen, es gibt eine Textdatei, die Zeilen enthält, die mit "a" beginnen, Zeilen, die mit "b" beginnen, und andere Zeilen, und ich möchte eigentlich nur mit den ersten beiden Arten von Zeilen arbeiten. Mein Code würde ungefähr so ​​aussehen (mit Python, aber als Pseudocode lesen):

# ...
clear_lines() # removes every other line than those starting with "a" or "b"
for line in lines:
    if ...

Ich hasse if...then...elseKonstruktionen. Ich würde das ganze Problem vermeiden:

process_lines_by_first_character (lines,  
                                  'a' => { |line| ... a code ... },
                                  'b' => { |line| ... b code ... } )