Wieso sollte man dich töten wollen?
Softwareentwicklung ist nun mal ein Reifeprozess. Niemand liest nur ein Buch und kann gleich wunderbar sauberen Code schreiben. Mein Code sah zum Anfang auch mies aus und selbst Robert C. Martin, der das Buch "Clean Code" geschrieben hat, sagt von sich aus, dass er nicht die Weisheit mit Löffeln gefressen hat und es auch bei seinen Code-Beispielen sicherlich noch Verbesserungspotenzial gibt aber nur durch Ausprobieren lernt man.
Du hast schon richtig erkannt, dass man fürs Einlesen der Datei eine eigene Klasse benötigt. Allerdings gehört die Logik nicht in den Konstruktor. Der Konstruktor dient zur Initialisierung der Klasse. Der Name der Klasse sollte auch die Aufgabe widerspiegeln, was die Klasse tut. "DateiEinlesen" ist vielleicht gut, aber geht es vielleicht noch konkreter? Ich weiß, dass es eine CSV-Datei ist. Vielleicht eher CsvReader? Wobei dieser Name auch wieder sehr allgemein ist. In der CSV-Datei steckt ja eine Tabelle. Welche Daten besitzt die Tabelle? Vielleicht kann man der Tabelle einen Namen geben. Eine CSV-Datei ist ja eine Art der Serialisierung. Das Verfahrung um so eine Tabelle in ein Objekt zu überführen, nennt man auch Deserialiserung. Das kann man ja erst mal im Hinterkopf behalten.
Zuerst würde ich mir aber erst mal eine geeignete Datenstruktur überlegen. In der CSV-Datei stecken ja Daten. Ich nehme jetzt mal als Beispiel, dass die CSV-Datei Daten zu Personen beinhaltet:
Name;Vorname;Geschlecht;Alter
Doe;John;Maennlich;38
Also würde ich erst mal eine Klasse für diese Daten erstellen:
public class Person
{
public string Name { get; set; }
public string Vorname { get; set; }
public Geschlecht Geschlecht { get; set; }
public int Alter { get; set; }
}
public enum Geschlecht
{
Maennlich
, Weiblich
}
Nun könnte ich mich darum kümmern, eine(!) Datei einzulesen. Ich habe eine Datenstruktur und ich weiß, dass ich eine CSV-Datei deserialisieren muss. Also könnte man die Klasse z.B. PersonCsvDeserializer nennen. In dieser Klasse soll es eine Methode geben, die Deserialize() heißt. Ich verzichte hier jetzt erst mal bewusst auf ein Interface, weil ich denke, dass es für dich bis hier hin schon kompliziert genug ist. Das Interface werde ich später noch mal erklären. Erst mal kümmern wir uns darum, was wir alles brauchen, um eine Datei zu deserialisieren.
Was muss die Klasse PersonCsvDeserializer alles wissen, um eine CSV-Datei deserialisieren zu können? Man könnte vielleicht im ersten Schritt auf die Idee kommen, dass die Klasse den Pfad und Dateinamen benötigt. Mit den Informationen aus dem letzten Absatz könnte ein erster Entwurf so aussehen:
public class PersonCsvDeserializer
{
public IEnumerable<Person> Deserialize(string fileName)
{
// ...
}
}
Als Rückgabewert habe ich IEnumerable<Person> gewählt, weil IEnumerable<T> ein sehr allgemeines Interface ist und einen Enumerator (auf deutsch: Aufzählung; in anderen Sprachen auch Iterator genannt) zur Verfügung stellt, mit dem wir über die Daten iterieren können (mit der foreach-Schleife). Sowohl IList<T>, ICollection<T>, IDictionary<T>, Array und weitere Klassen implementieren dieses Interface und mehr als über die Daten iterieren wollen wir nicht. Wenn wir später damit mehr machen wollen, können wir es leicht mit Linq in eine Collection, List, Array oder auch in ein Dictionary umwandeln. Die Deserialize()-Methode soll also eine Aufzählung von Personen zurückliefern.
Normalerweise macht man es anders, aber aus einfachheit behaupte ich mal frech, dass die erste Zeile in der CSV-Datei immer ein Header besitzt. In der Implementierung überspringe ich den Header per Linq mit der Skip()-Methode. Die Deserialize()-Methode soll also folgendes machen:
Die Datei lesen
Durch die Datenzeilen iterieren
Pro Datenzeile ein Person-Objekt erstellen
Die Person-Objekte als Aufzählung zurückliefern
Der erste Entwurf könnte daher folgendermaßen aussehen:
public class PersonCsvDeserializer
{
public IEnumerable<Person> Deserialize(string fileName)
{
IList<Person> result = new List<Person>();
foreach (string line in File.ReadAllLines(fileName).Skip(1))
{
string[] elements = line.Split(';');
result.Add(new Person
{
Name = elements[0],
Vorname = elements[1],
Geschlecht = (Geschlecht)Enum.Parse(typeof(Geschlecht), elements[2]),
Alter = Convert.ToInt32(elements[3])
});
}
return result;
}
}
Die Methode macht zwar was sie soll, aber ist sie wirklich übersichtlich? Nicht wirklich. Wir haben hier mehrere Ebenen miteinander vermischt. Wir können also mit dem Refactoring anfangen. z.B. das
File.ReadAllLines(fileName).Skip(1)
Wofür ist das genau gut? Wenn man den gesamten Kontext kennt, weiß man es zwar aber eigentlich liegt der Code-Abschnitt eine Ebene Tiefer. Es hantiert mit Dateien und hat mit der eigentlichen Aufgabe der Deserialiserung wenig zu tun. Also sollte man diesen Teil in eine separate Methode packen:
private IEnumerable<string> ReadDataFromFile(string fileName)
{
return File.ReadAllLines(fileName).Skip(1);
}
Somit wandert das Skip(1) in eine tiefere Ebene und interessiert uns in der Deserialize()-Methode nicht mehr. Als nächstes fällt aber auf, dass wir ein String mit Split() in ein Array teilen und aus diesem Array dann die einzelnen Personendaten herausfischen. Diesen Vorgang nennt man auch Parsing. Also könnten wir diesen Teil auch in eine Methode auslagern:
private Person Parse(string serializedData)
{
string[] elements = serializedData.Split(';');
return new Person
{
Name = elements[0],
Vorname = elements[1],
Geschlecht = (Geschlecht)Enum.Parse(typeof(Geschlecht), elements[2]),
Alter = Convert.ToInt32(elements[3])
};
}
Unsere Klasse sieht dann bis jetzt folgendermaßen aus:
public class PersonCsvDeserializer
{
public IEnumerable<Person> Deserialize(string fileName)
{
IList<Person> result = new List<Person>();
foreach (string serializedData in ReadDataFromFile(fileName))
{
Person person = this.Parse(serializedData);
result.Add(person);
}
return result;
}
private IEnumerable<string> ReadDataFromFile(string fileName)
{
return File.ReadAllLines(fileName).Skip(1);
}
private Person Parse(string serializedData)
{
string[] elements = serializedData.Split(';');
return new Person
{
Name = elements[0],
Vorname = elements[1],
Geschlecht = (Geschlecht)Enum.Parse(typeof(Geschlecht), elements[2]),
Alter = Convert.ToInt32(elements[3])
};
}
}
Nun ist Deserialize() doch recht gut lesbar. Wir lesen die Daten aus der Datei, parsen die Daten und erhalten ein Person-Objekt, welches wir dann in eine Liste packen und zum Schluss geben wir die Liste zurück.
Es gäbe hier noch weiteres Verbesserungspotenzial aber ich belasse es erst mal hierbei. Ein paar Hinweise gebe ich aber noch:
Fehler-Handling? Was passiert, wenn z.B. die Datei nicht existiert?
Ist das erzeugte Objekt List<Person> wirklich eine gute Wahl? Angenommen, wir haben es mit einer riesigen CSV-Datei (mehrere Gigabytes) zu tun, die größer ist, als unser Arbeitsspeicher. Hier schmeiße ich mal das "yield return"-Schlüsselwort in den Raum.
Auch ist das indexierte Zugreifen auf das Array in der Methode Parse() nicht wirklich glücklich gelöst. Was passiert nämlich, wenn mal eine Spalte in der Datei hinzukommt? Dann muss man ja auch den Code anpassen. Das will man aber eigentlich gar nicht.
Zu diskutieren wäre auch, ob die Variable fileName nicht doch besser eine Instanzvariable sein sollte, die per Konstruktor reingereicht wird. Es fällt ja auf, dass die Methoden Deserialize() und ReadDataFromFile() den Dateinamen benötigen. Also stellt fileName ja eine gewisse Abhängigkeit dar, die die Klasse benötigt, um arbeiten zu können.
Als Überlegung kannst du ja selber mal schauen, wie man mit solchen Situation umgehst.
Um später im Hauptpgramm alle Personen zu iterieren könntest du nun folgendes schreiben:
static void Main(string[] args)
{
string sourcePath = Environment.GetFolderPath(Environment.SpecialFolder.Desktop) + "\\blabla";
IEnumerable<string> fileNames = Directory.GetFiles(rootPath, "*.csv");
PersonCsvDeserializer deserializer = new PersonCsvDeserializer();
foreach(string fileName in fileNames)
{
IEnumerable<Person> persons = deserializer.Deserialize(fileName);
foreach (Person person in persons)
{
// ...
}
}
}
Nach dem selben Prinzip, wie bei der PersonCsvDeserializer-Klasse kannst du ja mal überlegen, wie man nun diesen Code refactoren an.
Ab hier wird es noch etwas technischer und tiefgreifender. Ich möchte dir noch zwei Techniken zeigen, die du aber erst mal nicht umsetzen brauchst.
"Inversion of Control" und "Dependeny Injection"
In der Klasse PersonCsvDeserializer fällt auf, dass die Klasse von einer Datei abhängig ist aber die Daten können vielleicht aus einer Datenbank kommen oder wir schreiben die CSV-Daten direkt in eine grafische Oberfläche. Möchte man jetzt für jeden Anwendungsfall eine eigene Klasse schreiben? Eigentlich nicht. Die Abhängigkeit zur Datei muss also aufgelöst werden. Das .Net-Framework bietet ja die abstrakte Klasse TextReader, die so ziemlich alles darstellen kann. Ein Reader, der eine Datei liest oder aus einem TCP-Stream oder aus einer Datenbank, etc. Anstatt also den Dateinamen reinzureichen, könnte man auch ein TextReader reinreichen.
Hier mal ein Beispiel, wie so eine Klasse aussehen könnte:
public class PersonCsvDeserializer
{
private TextReader reader;
private bool isHeaderSkipped;
public PersonCsvDeserializer(TextReader reader)
{
this.reader = reader;
}
public IEnumerable<Person> Deserialize()
{
string serializedData;
while ((serializedData = this.ReadNextData()) != null)
{
Person person = this.Parse(serializedData);
yield return person;
}
}
private string ReadNextData()
{
string serializedData = this.reader.ReadLine();
if (!this.isHeaderSkipped)
{
this.isHeaderSkipped = true;
return this.ReadNextData();
}
return serializedData;
}
private Person Parse(string serializedData)
{
string[] elements = serializedData.Split(';');
return new Person
{
Name = elements[0],
Vorname = elements[1],
Geschlecht = (Geschlecht)Enum.Parse(typeof(Geschlecht), elements[2]),
Alter = Convert.ToInt32(elements[3])
};
}
}
Die Main-Methode sieht dann so aus:
static void Main(string[] args)
{
string sourcePath = Environment.GetFolderPath(Environment.SpecialFolder.Desktop) + "\\blabla";
IEnumerable<string> fileNames = Directory.GetFiles(rootPath, "*.csv");
foreach(string fileName in fileNames)
{
using (TextReader reader = File.OpenText(fileName))
{
PersonCsvDeserializer deserializer = new PersonCsvDeserializer(reader);
IEnumerable<Person> persons = deserializer.Deserialize();
foreach (Person person in persons)
{
// ...
}
}
}
}
Zugegeben, in diesem Beispiel ist die Klasse PersonCsvDeserializer etwas komplizierter geworden aber es ist jetzt egal, woher die Daten stammen, solange wir ein TextReader in den Konstruktor schieben. Das reinrechen der Abhängigkeit in den Konstruktor nennt sich auch "Dependeny Injection". In diesem Beispiel habe ich auch das yield return verwendet. Da wir jetzt nur noch maximal den Speicher für ein Person-Objekt verbrauchen, könnte die Klasse eigentlich nun unendlich viele Daten deserialisieren. Ein Problem stellt aber immer noch die Indexierung des Arrays dar aber das überlasse ich jetzt dir.
Das Interface
Das letzte, was ich noch schreiben wollte, wäre ein geeignetes Interface für den Deserializer. Wollen wir jetzt mehrere Deserializer schreiben oder einen Deserializer als Abhängigkeit in eine Klasse reinreichen, ist ein Interface geeignet, damit es später egal ist, um welchen Deserializer es sich handelt. Man könnte sich ja auch vorstellen, dass die Daten nicht in einer CSV-Datei stecken, sondern in einer XML-Datei. Dafür wäre folgendes Interface recht nützlich
public interface IDeserializer<T>
{
IEnumerable<T> Deserialize();
}
Mit diesem Interface könnten wir sogar das hässliche using im Hauptprogramm wieder loswerden. Ich finde, das using stört im Lesefluss. Wir haben ja jetzt eine Klasse, die CSV-Daten aus unterschiedlichsten Quellen von Personen deserialisieren kann. Was hindert uns nun daran, einen weiteren Deserializer zu bauen, der aus Dateien deserialisiert? Beispiel:
public class PersonCsvFileDeserializer : IDeserializer<Person>
{
private string fileName;
public PersonCsvFileDeserializer(string fileName)
{
this.fileName = fileName;
}
public IEnumerable<Person> Deserialize()
{
using (TextReader reader = File.OpenText(fileName))
{
PersonCsvDeserializer deserializer = new PersonCsvDeserializer(reader);
return deserializer.Deserialize();
}
}
}
Das using wurde nach PersonCsvFileDeserializer und somit eine ebene tiefer verschoben. Wenn du Dependecy Injection verstanden hast, dann würde dir auffallen, dass die Zeile
PersonCsvDeserializer deserializer = new PersonCsvDeserializer(reader);
eigentlich böse ist, da es eine Abhängigkeit darstellt, die wiederum in den Konstruktor gehört. Ich habe sie aber erst mal hier drinnengelassen, weil das sonst wieder bedeuten würde, dass das using wieder ins Hauptprogramm rein müsste. Eigentlich müsste man sich eine Fabrik-Methode ausdenken, die den PersonCsvFileDeserializer zusammenbaut. Die habe ich hier aber weggelassen. Die kannst du dir ja ausdenken.
Das Hauptprogramm würde dann so aussehen:
static void Main(string[] args)
{
string sourcePath = Environment.GetFolderPath(Environment.SpecialFolder.Desktop) + "\\blabla";
IEnumerable<string> fileNames = Directory.GetFiles(rootPath, "*.csv");
foreach(string fileName in fileNames)
{
PersonCsvFileDeserializer deserializer = new PersonCsvFileDeserializer(fileName);
IEnumerable<Person> persons = deserializer.Deserialize();
foreach (Person person in persons)
{
// ...
}
}
}
Das wäre doch schon wieder ein Schritt übersichtlicher.
Wie du also siehst, haben wir allein nur für das Einlesen von den CSV-Dateien drei Klassen:
Person
PersonCsvDeserializer
PersonCsvFileDeserializer
und ein Interface:
IDeserializer<T>
geschrieben. Man braucht also kein mega großes Projekt, um mehrere Klassen zu schreiben. Es reicht auch schon was ganz einfaches. Man sollte sich immer bewusst machen, dass Klassen immer nur eine Aufgabe machen sollten und Methoden Teilaspekte dieser Aufgabe sind und sie sollten auch nicht mehr machen, als eine Sache. Es macht auch nichts, wenn man zum Anfang Spagetticode schreibt und diesen später nach und nach einem Refactoring unterzieht. Niemand ist perfekt und niemand schreibt perfekten Code. Man fängt also immer erst mal an und arbeitet sich Schritt für Schritt an eine geeignete und saubere Lösung. Selbst meine Lösung ist mit Sicherheit nicht perfekt und ich habe auch nicht die Weisheit mit Löffeln gefressen. Wenn du mein Beitrag richtig verfolgt haben solltest, hast du vielleicht auch gemerkt, dass ich erst mal eine Lösung geschrieben habe und sie dann nach und nach verfeinert und verbessert habe. Das Wissen kommt erst mit Erfahrung und Erfahrung sammelt man nur, indem man es ausprobiert und darüber mit anderen diskutiert. Also trau dich.
So, das reicht auch fürs erste. Ich denke, das ist erst mal genug Input.
RipperFox hat eine Reaktion von Chief Wiggum erhalten in Projektantrag: Aufbau eines Modems
RipperFox reagierte auf Whiz-zarD in C# OOP Probleme
RipperFox hat eine Reaktion von Eleu erhalten in VLAN`s die nicht geroutet werden sollen
RipperFox hat eine Reaktion von thereisnospace erhalten in Warum BWL in der BS?
RipperFox hat eine Reaktion von allesweg erhalten in SQL-Abfrage für Sortierung
RipperFox hat eine Reaktion von tschulian erhalten in Debian 8 - Probleme mit ÖFFENTLICHER IP in der DMZ
RipperFox hat eine Reaktion von CrankyCrany erhalten in Debian 8 - Probleme mit ÖFFENTLICHER IP in der DMZ
RipperFox hat eine Reaktion von Klebrig erhalten in Subnetting E-Klasse
