Für-wenn Antimuster

Ich habe in diesem Blog-Beitrag über das For-If-Anti-Pattern gelesen und bin mir nicht ganz sicher, warum es ein Anti-Pattern ist.

foreach (string filename in Directory.GetFiles("."))
{
    if (filename.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))
    {
        return new StreamReader(filename);
    }
}

Frage 1:

Liegt es an der return new StreamReader(filename);Innenseite for loop? oder die Tatsache, dass Sie forin diesem Fall keine Schleife benötigen ?

Wie der Autor des Blogs betonte, ist die weniger verrückte Version davon:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
} 

Beide leiden unter einer Race-Bedingung, denn wenn die Datei vor der Erstellung der gelöscht wird StreamReader, erhalten Sie eine File­Not­Found­Exception.

Frage 2:

Um das zweite Beispiel zu korrigieren, würden Sie es ohne die if-Anweisung neu schreiben und stattdessen das StreamReadermit einem try-catch-Block umgeben, und wenn es einen File­Not­Found­Exceptionauslöst, behandeln Sie es entsprechend im catchBlock?

Dies ist ein Antimuster, da es die Form hat:

loop over a set of values
   if current value meets a condition
       do something with value
   end
end

und kann durch ersetzt werden

do something with value

Ein klassisches Beispiel hierfür ist Code wie:

for (var i=0; i < 5; i++)
{
    switch (i)
        case 1:
            doSomethingWith(1);
            break;
        case 2:
            doSomethingWith(2);
            break;
        case 3:
            doSomethingWith(4);
            break;
        case 4:
            doSomethingWith(4);
            break;
    }
}

Wenn Folgendes gut funktioniert:

doSomethingWith(1);
doSomethingWith(2);
doSomethingWith(3);
doSomethingWith(4);

Wenn Sie feststellen, dass Sie eine Schleife und ein ifoder ausführen switch, halten Sie an und überlegen Sie, was Sie tun. Überkomplizieren Sie die Dinge und können die gesamte Schleife und der Test einfach durch eine einfache "Just Do It" -Zeile ersetzt werden? Manchmal werden Sie jedoch feststellen, dass Sie diese Schleife ausführen müssen (mehr als ein Element kann beispielsweise der einen Bedingung entsprechen). In diesem Fall ist das Muster in Ordnung.

Deshalb ist es ein Anti-Pattern: Es nimmt das "Loop and Test" -Muster und missbraucht es.

Zu Ihrer zweiten Frage: Ja. Ein "try do" -Muster ist robuster als ein "test do do" -Muster in jeder Situation, in der Ihr Code nicht der einzige Thread auf dem gesamten Gerät ist, der den Status des zu testenden Elements ändern kann.

Das Problem mit diesem Code:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

ist, dass in der Zeit zwischen dem File.Existsund dem StreamReaderVersuch, diese Datei zu öffnen, ein anderer Thread oder Prozess die Datei löschen könnte. Sie erhalten also eine Ausnahme. Daher muss diese Ausnahme durch Folgendes geschützt werden:

try
{
    return new StreamReader("desktop.ini");
}
catch (File­Not­Found­Exception)
{
    return null; // or whatever
}

@ Flater wirft einen guten Punkt auf? Ist das selbst ein Antimuster? Verwenden wir Ausnahmen als Kontrollfluss?

Wenn der Code so etwas wie folgt lautet:

try
{
    if (!File.Exists("desktop.ini")
    {
        throw new IniFileMissingException();
        return new StreamReader("desktop.ini");
    }
}
catch (IniFileMissingException)
{
    return null;
}

dann würde ich tatsächlich Ausnahmen als verherrlichte Goto verwenden und es wäre in der Tat ein Anti-Muster. In diesem Fall arbeiten wir jedoch nur an dem unerwünschten Verhalten beim Erstellen eines neuen Streams. Dies ist also kein Beispiel für dieses Anti-Pattern. Aber es ist ein Beispiel dafür, wie man dieses Anti-Muster umgeht.

Was wir wirklich wollen, ist natürlich eine elegantere Art, den Stream zu erstellen. So etwas wie:

return TryCreateStream("desktop.ini", out var stream) ? stream : null;

Und ich würde empfehlen, diesen try catchCode in eine Dienstprogrammmethode wie diese einzuschließen, wenn Sie diesen Code häufig verwenden.

Frage 1: (Ist diese For-Schleife ein Antimuster?)

Ja, da Sie nicht selbst nach Elementen suchen müssen, die in einem abfragbaren Subsystem gespeichert sind.

In der Zusammenfassung kann das Dateisystem wie eine Datenbank auf Anfragen antworten. Bei der Interaktion mit einem abfragbaren Subsystem haben wir eine grundlegende Wahl, ob ein solches Subsystem seinen Inhalt für uns auflisten soll, damit wir den Abgleich außerhalb des Subsystems durchführen, oder ob die nativen Abfragefunktionen des Subsystems verwendet werden sollen.

Stellen wir uns für einen Moment vor, Sie suchen einen Datensatz in einer Datenbank anstelle einer Datei in einem Verzeichnis in einem Dateisystem. Möchten Sie lieber sehen

SELECT * FROM SomeTable;

und dann in einer Schleife (z. B. in C #) über den zurückgegebenen Cursor nach ID = 100 suchen oder das abfragbare Subsystem tun lassen, was es kann, um stattdessen genau das zu finden, wonach Sie suchen?

SELECT * FROM SomeTable WHERE ID = 100;

Ich sollte denken, dass die meisten von uns sich zu Recht dafür entscheiden würden, das Subsystem die genaue Abfrage von Interesse durchführen zu lassen. Die Alternative umfasst potenziell zahlreiche Roundtrips mit dem Subsystem, ineffiziente Gleichheitstests und den Verzicht auf die Verwendung von Indizes oder anderen Suchbeschleunigern, die uns sowohl von Datenbanken als auch von Dateisystemen zur Verfügung gestellt werden.

Frage 2: Um das zweite Beispiel zu korrigieren, würden Sie es ohne die if-Anweisung neu schreiben und stattdessen den StreamReader mit einem try-catch-Block umgeben. Wenn er eine FileNotFoundException auslöst, behandeln Sie ihn entsprechend im catch-Block?

Ja, denn genau so funktioniert diese bestimmte API - es ist nicht wirklich unsere Wahl, da dies eine Bibliotheksfunktion ist. Die if-Prüfung vor dem Aufruf bietet keinen zusätzlichen Wert: Wir müssen ohnehin try / catch verwenden, da (1) andere Fehler außerhalb von FileNotFound auftreten können und (2) die Race-Bedingung.

Frage 1:

Liegt es an der Rückgabe eines neuen StreamReader (Dateiname)? in der for-Schleife? oder die Tatsache, dass Sie in diesem Fall keine for-Schleife benötigen?

Der Streamreader hat nichts damit zu tun. Das Anti-Muster entsteht aufgrund eines klaren Absichtskonflikts zwischen dem foreachund dem if:

Was ist der Zweck der foreach?

Ich gehe davon aus, dass Ihre Antwort ungefähr so ​​lautet: "Ich möchte einen bestimmten Code wiederholt ausführen."

Wie viele Dateien werden voraussichtlich verarbeitet?

Da Sie nur einen bestimmten Dateinamen (einschließlich der Erweiterung) in einem bestimmten Ordner haben können, beweist dies, dass Ihr Code eine zutreffende Datei finden soll.

Dies wird auch dadurch bestätigt, dass Sie sofort einen Wert zurückgeben. Sie interessieren sich eigentlich nicht für ein zweites Match, selbst wenn es existieren würde.

Es gibt Situationen, in denen dies kein Anti-Muster ist.

• Wenn Sie auch in Unterverzeichnissen ( Directory.GetFiles(".", SearchOption.AllDirectories)) suchen , ist es möglich, mehr als eine Datei mit demselben Dateinamen (einschließlich Erweiterung) zu finden.
• Wenn Sie nach partiellen Dateinamenübereinstimmungen suchen (z. B. jede Datei, deren Name mit beginnt "Test_", oder jede "*.zip"Datei.

Beachten Sie, dass Sie in beiden Fällen mehrere Übereinstimmungen verarbeiten müssen und daher nicht sofort einen Wert zurückgeben müssen.

Frage 2:

Um das zweite Beispiel zu korrigieren, würden Sie es ohne die if-Anweisung neu schreiben und stattdessen den StreamReader mit einem try-catch-Block umgeben. Wenn er eine FileNotFoundException auslöst, behandeln Sie ihn entsprechend im catch-Block?

Ausnahmen sind teuer. Sie sollten niemals anstelle einer ordnungsgemäßen Flusslogik verwendet werden. Ausnahmen sind, wie der Name schon sagt, außergewöhnliche Umstände.

Aus diesem Grund sollten Sie das nicht entfernen if .

Gemäß dieser Antwort auf SoftwareEngineering.SE :

Im Allgemeinen ist die Verwendung von Ausnahmen für den Kontrollfluss ein Anti-Muster mit bemerkenswerten situations- und sprachspezifischen Hustenausnahmen.

Als kurze Zusammenfassung, warum es im Allgemeinen ein Anti-Muster ist:

• Ausnahmen sind im Wesentlichen ausgefeilte GOTO-Anweisungen
• Das Programmieren mit Ausnahmen führt daher dazu, dass Code schwieriger zu lesen und zu verstehen ist
• In den meisten Sprachen sind Kontrollstrukturen vorhanden, mit denen Sie Ihre Probleme ohne Ausnahmen lösen können
• Effizienzargumente sind für moderne Compiler eher umstritten, die tendenziell mit der Annahme optimieren, dass Ausnahmen nicht für den Kontrollfluss verwendet werden.

Lesen Sie die Diskussion in Wards Wiki, um weitere Informationen zu erhalten.

Ob Sie dies in einen Versuch / Fang einwickeln müssen, hängt stark von Ihrer Situation ab:

• Wie wahrscheinlich ist es, dass Sie auf eine Rennbedingung stoßen?
• Sind Sie tatsächlich in der Lage, mit dieser Situation umzugehen, oder möchten Sie, dass dieses Problem für den Benutzer auftritt, weil Sie nicht wissen, wie Sie damit umgehen sollen.

Nichts ist jemals eine Frage von "immer benutzen". Um meinen Standpunkt zu beweisen:

Separate Studien haben gezeigt, dass Sie weniger wahrscheinlich verletzt werden, wenn Sie einen Schutzhelm, eine Schutzbrille und kugelsichere Westen tragen.

Warum tragen wir nicht alle diese Sicherheitsausrüstung immer?

Die einfache Antwort ist, weil das Tragen Nachteile hat:

• Es kostet Geld
• Es macht Ihre Bewegung umständlicher
• Es kann sehr warm sein, es zu tragen.

Jetzt kommen wir voran: Es gibt Vor- und Nachteile . Mit anderen Worten, es ist nur dann sinnvoll, diese Ausrüstung zu tragen, wenn die Vorteile die Nachteile überwiegen .

• Bauarbeiter erleiden viel häufiger während ihrer Arbeit Verletzungen. Sie profitieren von einem Schutzhelm.
• Büroangestellte hingegen haben eine viel geringere Verletzungsgefahr. Schutzhelme sind es nicht wert.
• Ein SWAT-Teammitglied wird viel häufiger beschossen als ein Büroangestellter.

Sollten Sie den Anruf in einen Versuch / Fang einwickeln? Das hängt sehr davon ab, ob der Nutzen die Kosten für die Implementierung überwiegt.

Beachten Sie, dass andere möglicherweise argumentieren, dass es nur ein paar Tastenanschläge dauert, um es zu verpacken, daher sollte es offensichtlich getan werden. Aber das ist nicht das ganze Argument:

• Sie müssen entscheiden, was zu tun ist, sobald Sie die Ausnahme abfangen.
• Wenn in der gesamten Codebasis viele verschiedene Aufrufe an verschiedene Dateien erfolgen, bedeutet die Entscheidung, eine Datei in einen Versuch / Fang einzuschließen, im Allgemeinen, dass Sie alle diese Fälle umschließen müssen. Dies kann sich dramatisch auf den Aufwand auswirken, der für die Implementierung erforderlich ist.
• Es ist durchaus möglich, dass Sie absichtlich keine Ausnahme behandeln möchten .
  • Beachten Sie, dass Ihre Anwendung die Ausnahme irgendwann behandeln muss, dies jedoch nicht unbedingt unmittelbar nach dem Auslösen der Ausnahme.

Sie haben also die Wahl. Hat dies einen Vorteil? Denken Sie, dass es die Anwendung verbessert, mehr als den Aufwand für die Implementierung?

Aktualisieren - Von einem Kommentar, den ich auf die andere Antwort geschrieben habe, da ich denke, dass dies auch für Sie eine relevante Überlegung ist:

Es hängt sehr stark vom umgebenden Kontext ab.

• Wenn dem Öffnen des Streamreaders vorausgegangen wird if(!File.Exists) File.Create(), ist das Fehlen der Datei beim Öffnen des Streamreader in der Tat außergewöhnlich .
• Wenn der Dateiname aus einer Liste vorhandener Dateien ausgewählt wurde, ist sein plötzliches Fehlen erneut außergewöhnlich .
• Wenn Sie mit einer Zeichenfolge arbeiten, die Sie noch nicht anhand des Verzeichnisses getestet haben; dann ist das Fehlen der Datei ein vollkommen logisches Ergebnis und daher keine Ausnahme .