Ich muss oft eine Klasse implementieren, die eine Aufzählung / Sammlung von etwas ist. Betrachten wir für diesen Thread das erfundene Beispiel von IniFileContentdenen ist eine Aufzählung / Sammlung von Linien.

Der Grund, warum diese Klasse in meiner Codebasis vorhanden sein muss, ist, dass ich vermeiden möchte, dass Geschäftslogik über den gesamten Ort verteilt wird (= Kapselung der where), und dies auf möglichst objektorientierte Weise.

Normalerweise würde ich es wie folgt implementieren:

public sealed class IniFileContent : IEnumerable<string>
{
    private readonly string _filepath;
    public IniFileContent(string filepath) => _filepath = filepath;
    public IEnumerator<string> GetEnumerator()
    {
        return File.ReadLines(_filepath)
                   .Where(l => !l.StartsWith(";"))
                   .GetEnumerator();
    }
    public IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

Ich entscheide mich für die Implementierung, IEnumerable<string>weil es die Verwendung bequemer macht:

foreach(var line in new IniFileContent(...))
{
    //...
}

Allerdings frage ich mich, ob dies die Klassenabsicht "beschattet"? Wenn man sich IniFileContentdie Oberfläche ansieht , sieht man nur Enumerator<string> GetEnumerator(). Ich denke, es ist nicht offensichtlich, welchen Service die Klasse tatsächlich anbietet.

Betrachten Sie dann diese zweite Implementierung:

public sealed class IniFileContent2
{
    private readonly string _filepath;
    public IniFileContent2(string filepath) => _filepath = filepath;
    public IEnumerable<string> Lines()
    {
        return File.ReadLines(_filepath)
                   .Where(l => !l.StartsWith(";"));
    }
}

Was weniger bequem verwendet wird (übrigens new X().Y()fühlt sich das Sehen so an, als ob beim Klassendesign etwas schief gelaufen ist):

foreach(var line in new IniFileContent2(...).Lines())
{
    //...
}

Aber mit einer klaren Oberfläche IEnumerable<string> Lines(), die deutlich macht, was diese Klasse tatsächlich kann.

Welche Implementierung würden Sie fördern und warum? Ist es implizit eine gute Praxis, IEnumerable zu implementieren, um eine Aufzählung von etwas darzustellen?

Ich suche keine Antworten darauf, wie man:

• Unit-Test diesen Code
• Machen Sie eine statische Funktion anstelle einer Klasse
• Machen Sie diesen Code anfälliger für zukünftige Entwicklungen der Geschäftslogik
• Leistung optimieren

Blinddarm

Hier ist die Art von echtem Code , der in meiner Codebasis lebt und implementiert wirdIEnumerable

public class DueInvoices : IEnumerable<DueInvoice>
{
    private readonly IEnumerable<InvoiceDto> _invoices;
    private readonly IEnumerable<ReminderLevel> _reminderLevels;
    public DueInvoices(IEnumerable<InvoiceDto> invoices, IEnumerable<ReminderLevel> reminderLevels)
    {
        _invoices = invoices;
        _reminderLevels = reminderLevels;
    }
    public IEnumerator<DueInvoice> GetEnumerator() => _invoices.Where(invoice => invoice.DueDate < DateTime.Today && !invoice.Paid)
                                                               .Select(invoice => new DueInvoice(invoice, _reminderLevels))
                                                               .GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

Ich überprüfe Ihren Ansatz, bevor ich einen völlig anderen Ansatz vorschlage. Ich bevorzuge den anderen Ansatz, aber es scheint wichtig zu erklären, warum Ihr Ansatz Fehler aufweist.

Ich entscheide mich für die Implementierung, IEnumerable<string>weil es die Verwendung bequemer macht

Die Bequemlichkeit sollte die Korrektheit nicht überwiegen.

Ich frage mich, ob Ihre MyFileKlasse mehr Logik als diese enthalten wird. denn das wird die Richtigkeit dieser Antwort beeinträchtigen. Ich interessiere mich besonders für:

.Where(l => ...) //some business logic for filtering

Wenn dies komplex oder dynamisch genug ist, verstecken Sie diese Logik in einer Klasse, deren Name nicht anzeigt, dass der Inhalt gefiltert wird, bevor er dem Verbraucher bereitgestellt wird.
Ein Teil von mir hofft / geht davon aus, dass diese Filterlogik fest codiert sein soll (z. B. ein einfacher Filter, der kommentierte Zeilen ignoriert, z. B. wie INI-Dateien Zeilen #als Kommentar betrachten) und kein dateispezifischer Regelsatz.

public class MyFile : IEnumerable<string>

Es ist wirklich krass, wenn ein Singular ( File) einen Plural ( IEnumerable) darstellt. Eine Datei ist eine einzelne Entität. Es besteht aus mehr als nur seinem Inhalt. Es enthält auch Metadaten (Dateiname, Erweiterung, Erstellungsdatum, Änderungsdatum, ...).

Ein Mensch ist mehr als die Summe seiner Kinder. Ein Auto ist mehr als die Summe seiner Teile. Ein Gemälde ist mehr als eine Sammlung von Farben und Leinwand. Und eine Datei ist mehr als eine Sammlung von Zeilen.

Wenn ich davon ausgehe, dass Ihre MyFileKlasse niemals mehr Logik als nur diese Aufzählung von Zeilen enthält (und Wherenur einen einfachen statischen fest codierten Filter anwendet), dann haben Sie hier eine verwirrende Verwendung des "Dateinamens" und seiner beabsichtigten Bezeichnung . Dies kann leicht behoben werden, indem die Klasse in umbenannt wird FileContent. Es behält Ihre beabsichtigte Syntax bei:

foreach(var line in new FileContent(@"C:\Folder\File.txt"))

Dies ist auch aus semantischer Sicht sinnvoller. Der Inhalt einer Datei kann in separate Zeilen unterteilt werden. Dies setzt immer noch voraus, dass der Inhalt der Datei Text und nicht binär ist, aber das ist fair genug.

Wenn Ihre MyFileKlasse jedoch mehr Logik enthält, ändert sich die Situation. Dies kann auf verschiedene Arten geschehen:

• Sie verwenden diese Klasse, um die Metadaten der Datei darzustellen, nicht nur deren Inhalt.

Wenn Sie dies tun beginnen, dann die Datei repräsentiert die Datei in dem Verzeichnis , das mehr als nur ihr Inhalt ist.
Der richtige Ansatz hier ist dann, was Sie getan haben MyFile2.

• Der Where()Filter verfügt über eine komplizierte Filterlogik, die nicht fest codiert ist, z. B. wenn verschiedene Dateien unterschiedlich gefiltert werden.

Wenn Sie dies tun, haben Dateien eigene Identitäten, da sie über einen eigenen benutzerdefinierten Filter verfügen. Dies bedeutet, dass Ihre Klasse mehr ein FileTypeals ein geworden ist FileContent. Die beiden Verhaltensweisen müssen entweder getrennt oder mithilfe der Komposition (was Ihren MyFile2Ansatz begünstigt ) oder vorzugsweise beider (getrennte Klassen für das FileTypeund FileContentVerhalten und dann beide in der MyFileKlasse zusammengesetzt) kombiniert werden .

Ein ganz anderer Vorschlag.

So wie es aussieht, existieren sowohl Ihre MyFileals MyFile2auch nur, um Ihnen eine Hülle um Ihren .Where(l => ...)Filter zu geben. Zweitens erstellen Sie effektiv eine Klasse, um eine statische Methode ( File.ReadLines()) zu verpacken. Dies ist kein guter Ansatz.

Abgesehen davon verstehe ich nicht, warum Sie sich entschieden haben, Ihre Klasse zu machen sealed. Wenn überhaupt, würde ich erwarten, dass die Vererbung das größte Merkmal ist: verschiedene abgeleitete Klassen mit unterschiedlicher Filterlogik (vorausgesetzt, sie ist komplexer als eine einfache Wertänderung, da die Vererbung nicht nur zum Ändern eines einzelnen Werts verwendet werden sollte).

Ich würde deine ganze Klasse umschreiben als:

foreach(var line in File.ReadLines(...).Where(l => ...))

Der einzige Nachteil dieses vereinfachten Ansatzes besteht darin, dass Sie den Where()Filter jedes Mal wiederholen müssen, wenn Sie auf den Inhalt der Datei zugreifen möchten. Ich stimme zu, dass dies nicht wünschenswert ist.

Es scheint jedoch übertrieben, dass Sie beim Erstellen einer wiederverwendbaren Where(l => ...)Anweisung auch die Implementierung dieser Klasse erzwingen File.ReadLines(...). Sie bündeln mehr, als Sie wirklich brauchen.

Anstatt zu versuchen, die statische Methode in eine benutzerdefinierte Klasse zu packen, ist es meiner Meinung nach viel passender, wenn Sie sie in eine eigene statische Methode einschließen:

public static IEnumerable<string> GetFilteredFileContent(string filePath)
{
    return File.ReadLines(filePath).Where(l => ...);
}

Angenommen, Sie haben verschiedene Filter, können Sie den entsprechenden Filter als aa-Parameter übergeben. Ich zeige Ihnen ein Beispiel, das mehrere Filter verarbeiten kann, die in der Lage sein sollten, alles zu verarbeiten, was Sie dazu benötigen, und gleichzeitig die Wiederverwendbarkeit zu maximieren:

public static class MyFile
{
    public static Func<string, bool> IgnoreComments = 
                  (l => !l.StartsWith("#"));

    public static Func<string, bool> OnlyTakeComments = 
                  (l => l.StartsWith("#"));

    public static Func<string, bool> IgnoreLinesWithTheLetterE = 
                  (l => !l.ToLower().contains("e"));

    public static Func<string, bool> OnlyTakeLinesWithTheLetterE = 
                  (l => l.ToLower().contains("e"));

    public static IEnumerable<string> ReadLines(string filePath, params Func<string, bool>[] filters)
    {
        var lines = File.ReadLines(filePath).Where(l => ...);

        foreach(var filter in filters)
            lines = lines.Where(filter);

        return lines;
    }
}

Und seine Verwendung:

MyFile.ReadLines("path", MyFile.IgnoreComments, MyFile.OnlyTakeLinesWithTheLetterE);

Dies ist nur ein Beispiel, das beweisen soll, dass statische Methoden sinnvoller sind als das Erstellen einer Klasse.

Lassen Sie sich nicht von den Besonderheiten der Implementierung der Filter einfangen. Sie können sie implementieren, wie Sie möchten (ich persönlich mag die Parametrisierung Func<>wegen ihrer inhärenten Erweiterbarkeit und Anpassungsfähigkeit an Refactoring). Da Sie jedoch kein Beispiel für die Filter angegeben haben, die Sie verwenden möchten, habe ich einige Annahmen getroffen, um Ihnen ein praktikables Beispiel zu zeigen.

Sehen new X().Y()fühlt sich an, als ob etwas mit dem Klassendesign schief gelaufen ist.

In Ihrem Ansatz hätten Sie es schaffen können, new X().Ywas weniger kratzend ist.

Ich denke jedoch, dass Ihre Abneigung gegen new X().Y()den Punkt, dass Sie sich wie eine Klasse fühlen, hier nicht gerechtfertigt ist, aber eine Methode ist; die nur ohne Klasse dargestellt werden kann, indem sie statisch ist.

Meiner Meinung nach sind die Probleme bei beiden Ansätzen:

1. Sie kapseln File.ReadLines , was das Testen von Einheiten schwieriger macht als nötig.
2. Bei jeder Aufzählung der Datei muss eine neue Klasseninstanz erstellt werden, um den Pfad als zu speichern _filepath.

Daher würde ich vorschlagen, es zu einer statischen Methode zu machen, die entweder übergeben wird IEnumerable<string>oder Streamden Dateiinhalt darstellt:

public static GetFilteredLines(IEnumerable<string> fileContents)
    => fileContents.Where(l => ...);

Dann heißt es via:

var filteredLines = GetFilteredLines(File.ReadLines(filePath));

Dies vermeidet unnötige Belastung des Haufens und erleichtert das Testen der Methode durch Einheiten erheblich.

Das Beispiel aus der Praxis, das Sie bewiesen haben, DueInvoiceseignet sich sehr gut für das Konzept, dass es sich um eine Sammlung fälliger Rechnungen handelt. Ich verstehe vollkommen, wie erfundene Beispiele Menschen dazu bringen können, sich mit den von Ihnen verwendeten Begriffen und dem Konzept, das Sie vermitteln möchten, auseinanderzusetzen. Ich war selbst mehrere Male am frustrierenden Ende.

Das heißt, wenn der Zweck der Klasse ausschließlich darin besteht, eine zu sein IEnumerable<T>und keine andere Logik liefert, muss ich die Frage stellen, ob Sie eine ganze Klasse benötigen oder einfach eine Methode aus einer anderen Klasse bereitstellen können. Zum Beispiel:

public class Invoices
{
    // ... skip all the other stuff about Invoices

    public IEnumerable<Invoice> GetDueItems()
    {
         foreach(var line in File.ReadLines(_invoicesFile))
         {
             var invoice = ReadInvoiceFrom(line);
             if (invoice.PaymentDue <= DateTime.UtcNow)
             {
                 yield return invoice;
             }
         }
    }
}

Dies yield returnfunktioniert, wenn Sie nicht einfach eine LINQ-Abfrage umbrechen oder die Logik einbetten können. Die andere Option besteht einfach darin, die LINQ-Abfrage zurückzugeben:

public class Invoices
{
    // ... skip all the other stuff about invoices

    public IEnumerable<Invoice> GetDueItems()
    {
        return from Invoice invoice in GetAllItems()
               where invoice.PaymentDue <= DateTime.UtcNow
               select invoice;
    }
}

In beiden Fällen benötigen Sie keine vollständige Wrapper-Klasse. Sie müssen nur eine Methode angeben, und der Iterator wird im Wesentlichen für Sie behandelt.

Das einzige Mal, dass ich eine vollständige Klasse für die Iteration benötigte, war, als ich in einer lang laufenden Abfrage Blobs aus einer Datenbank ziehen musste. Das Dienstprogramm diente einer einmaligen Extraktion, damit wir die Daten an eine andere Stelle migrieren konnten. Es gab einige Verrücktheiten bei der Datenbank, als ich versuchte, den Inhalt mit zu streamen yield return. Aber das ging weg, als ich meinen Brauch tatsächlich implementierte, IEnumerator<T>um besser zu kontrollieren, wann Ressourcen bereinigt wurden. Dies ist eher die Ausnahme als die Regel.

Kurz gesagt, ich empfehle, nicht IEnumerable<T>direkt zu implementieren, wenn Ihre Probleme auf eine der im obigen Code beschriebenen Arten gelöst werden können. Speichern Sie den Aufwand für die explizite Erstellung des Enumerators, wenn Sie das Problem nicht auf andere Weise lösen können.

Denken Sie an Konzepte. Welche Beziehung besteht zwischen einer Datei und ihrem Inhalt? Das ist eine "hat" -Beziehung , keine "ist" -Beziehung .

Folglich sollte die Datei Klasse hat eine Methode / Eigenschaft für den Inhalt zurück. Und das ist immer noch bequem anzurufen:

public IEnumerable<string> GetFilteredContents() { ... }

foreach(string line in myFile.GetFilteredContents() { ... }