C# OOP - Eure Meinung

[Tician]

Moinsen,

auch wenn es noch nicht funktioniert stelle ich mal meinen ersten vollständigen Versuch zur OOP vor. Nach wie vor hoffe ich das ich nicht meilenweit am Ziel vorbei geschossen bin.

Das Programm soll bestimmte csv-Dateien aus einem (in einer ini-Datei) festgelegten Pfad auslesen, die Anführungszeichen in den Dateien löschen und sie wieder in einem anderen Pfad speichern. Ich hoffe das sollte für den Anfang vom Schwierigkeitsgrad her einfach genug sein.

Mein Ziel ist mich der OOP Schrittweise zu nähern, daher bitte ich euch keine komplette Überarbeitung zu posten. Ich versuche - sofern mir möglich - die Vorschläge nacheinander umzusetzen und natürlich zu verstehen.

Ich habe 3 Klassen: Program (Main); Settings (um die 2 Zeilen in der ini-Datei auszulesen); CsvChanger (suchen, lesen, verändern und schreiben der Dateien)

Der Code:

class Program
    {
        static void Main(string[] args)
        {
            try
            {
                Settings settings1 = new Settings();
                settings1.ReadFile("settings.ini");
                settings1.GetSource();
                settings1.GetDestination();

                CsvChanger changer = new CsvChanger();
                changer.FindCsv(settings1.source);
                changer.ReadCsv();
                changer.ChangeCsv();
                changer.WriteFiles(settings1.destination);

                Console.ReadKey();
            }
            catch
            {
                Console.WriteLine("Unknown error");
            }
        }
    }


    class Settings
    {
        private string iniText;
        public string source { get; set; }
        public string destination { get; set; }

        public void ReadFile(string iniFile)
        {
            try
            {
                iniText = File.ReadAllText(iniFile);
            }
            catch
            {
                Console.WriteLine("settings.ini not found");
            }           
        }
        public void GetSource()
        {
            try
            {
                Regex rsource = new Regex("(?<=Source\\=).+");
                Match msource = rsource.Match(iniText);
                source = Convert.ToString(msource);
            }
            catch
            {
                Console.WriteLine("Could not find 'Source' in settings");
            }            
        }
        public void GetDestination()
        {
            try
            {
                Regex rdestination = new Regex("(?<=Destination\\=).+");
                Match mdestination = rdestination.Match(iniText);
                destination = Convert.ToString(mdestination);
            }
            catch
            {
                Console.WriteLine("Could not find 'Destination' in settings");
            }
        }
    }



    class CsvChanger
    {
        private string[] changedContent;
        private int fileCount;
        private string[] files;
        private string[] fileContent;
        //Settings settings2 = new Settings();
        string source;
        string destination;

        public void FindCsv(string source)
        {
            try
            {
                this.source = source;
                files = Directory.GetFiles(source, "GAR_EXPORT_*.csv");
                fileCount = files.Length;
                if (files == null)
                {
                    Environment.Exit(0);
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine("Error finding files");
                Console.WriteLine(Convert.ToString(ex));
            }
        }

        public void ReadCsv()
        {
            try
            {
                for (int x = 1; x <= fileCount; x++)
                {
                    fileContent[x] = File.ReadAllText(files[x]);
                }
            }
            catch
            {
                Console.WriteLine("Couldn't read CSV");
            }
        }

        public void ChangeCsv()
        {
            try
            {
                for (int x = 1; x <= fileCount; x++)
                {
                    changedContent[x] = fileContent[x].Replace("\"", "");
                }
            }
            catch
            {
                Console.WriteLine("Couldn't replace file-content");
            }
        }

        public void WriteFiles(string destination)
        {
            try
            {
                this.destination = destination;
                for (int x = 1; x <= fileCount; x++)
                {
                    using (StreamWriter sw = new StreamWriter(destination + Path.GetFileName(files[x])))
                    {
                        sw.Write(changedContent[x]);
                    }
                }
            }
            catch
            {
                Console.WriteLine("Error writing files to destination");
            }
        }
    }

 

Einen Vorschlag hatte ich schon bekommen: Die Get-Methoden haben kein return, allerdings sind sie trotzdem dafür da etwas auszulesen und in eine Variable zu speichern. Ich weiß nicht wie ich das sonst nennen soll^^

Warum das Programm nicht funktioniert:

            try
            {
                Regex rsource = new Regex("(?<=Source\\=).+");
                Match msource = rsource.Match(iniText);
                source = Convert.ToString(msource);
            }

Ich mache nichts anders als sonst auch aber folgendes passiert:

msource hat genau das was ich möchte, in der Überwachung sieht es so aus:

msource = "C:\\Users\\ich\\desktop\\"

allerdings sobald ich in einen string convertiere sieht source dann so aus:

source = "C:\\Users\\ich\\desktop\\\r"

und ich bekomme die Meldung "ungültige Zeichen in Pfad" als Exception. Ich habe keine Ahnung wo dieses '\r' her kommt und warum es in all meinen vorherigen Programmen in denen ich Regex zum ini-Datei auslesen benutzt habe einwandfrei funktioniert hat. Hat da jemand eine Idee?

Bearbeitet 16. Januar 2017 von Tician

[Whiz-zarD]

vor 8 Stunden schrieb Tician:

 

@Whiz-zarDDas mit den vielen try-catch Blöcken ist etwas doof, gehen die auch nach dem DRY-Prinzip? Weil dann hätte ich wieder einen einzigen in der Main und ich glaube das war nicht Sinn der Sache.

Doch, genau das ist der Sinn der Sache. Man will ja in unterschiedlichen "Umgebungen" auf Fehler unterschiedlich reagieren. z.B. will man in einer Konsolenanwendung die Fehlermeldung auf der Konsole ausgeben oder bei einer Desktop-Anwendung mit grafischer Oberfläche eine MessageBox oder bei einer Webanwendung irgendwo als Hinweis-Text.

Du hast aber die Exceptions schon in der CsvChanger-Klasse abgefangen und festgelegt, dass die Fehlermeldung auf der Konsole ausgegeben werden soll, was aber zur Folge hat, dass der Nutzer einer Desktop- oder Web-Anwendung gar nicht die Fehler mitbekommt. Vielleicht möchte man auch, dass der Nutzer entscheiden kann, im Falle eines Fehlers. z.B. du änderst Tausend CSV-Dateien und eine schlägt fehl. Man möchte vielleicht dem Nutzer die Möglichkeit bieten, die Aktion abzubrechen oder den Fehler zu ignorieren und weitermachen. Daher würde ich die ganzen Try-Catch-Blöcke weglassen und der Aufrufer soll die Fehler behandeln.

vor 8 Stunden schrieb Tician:

Ich könnte statt einer Ausgabe in der Console auch alles in eine log-Datei schreiben, dann könnte man das zumindest auch in Form-Anwendungen wieder verwenden.

Hier würden wir in den Bereich der sog. Dependency Injection kommen, denn in eine Datei zu loggen ist ja nicht die einzige Art des Loggings. Ein Logging kann auch auf der Konsole geschehen oder in eine Datenbank, als E-Mail, in eine Textbox einer grafischen Oberfläche, etc. Da sind also keine Grenzen gesetzt. Ein Benutzer einer Web-Anwendung hat durchaus ja keinen Zugriff auf eine Log-Datei. Anstatt also dass wir vorgeben, dass wir eine Datei schreiben, reichen wir als Abhängigkeit (engl: dependency) einen Logger in die Klasse über den Konstruktor rein. Dafür entwickelt man ein Interface und eine konkrete Implementierung und man arbeitet später dann nur noch mit dem Interface. Hier mal ein einfaches Beispiel für einen Logger:

Das Interface:

public interface ILogger
{
    void Log(string message)
}

Eine konkrete Implementierung, die auf der Konsole loggt:

public class ConsoleLogger : ILogger
{
    public void Log(string message)
    {
        Console.WriteLine(message);
    }
}

Die Klasse CsvChanger:

public class CsvChanger
{
    private ILogger logger; 

    public CsvChanger(ILogger logger) 
    { 
        this.logger = logger; 
    } 

    ...
}

Hier sieht man jetzt, dass ein Objekt in die Klasse als Abhängigkeit reingereicht wird, die das Interface ILogger implementiert. In der Main-Methode erzeugst du die Objekte, die der CsvChanger benötigt:

public static void Main(string[] args)
{
    ...

    ILogger logger = new ConsoleLogger();
    CsvChanger changer = new CsvChanger(logger);

    ...
}

Später, bei der Verwendung vom Logger in der CsvChanger-Klasse schreibst du dann einfach folgenden Code:

logger.Log("Das ist ein Text");

Jetzt ist die Klasse CsvChanger vom Logger unabhängig. Anstatt einen Konsolen-Logger kannst du jetzt nun auch einen Logger schreiben, der in eine Datei loggt und diesen dort reinreichen. Der Klasse ist das nun egal, da sie vom Logger nur das Interface kennt aber nicht die konkrete Implementierung.

vor 8 Stunden schrieb Tician:

Ich versteh es nicht wirklich, wenn ich mehrere Methoden haben soll und diese nur eine Aufgabe ausführen (so habe ich es jetzt gelernt) sollen dann muss ich die ja alle nacheinander aufrufen. Der einzige andere Weg der mir einfällt ist das eine Methode dann die nächste aufruft. Quasi eine Kette und ich müsste nur die erste Methode davon in der Main aufrufen. War es das was du meinstest?

Ich glaube, du hast es nun zu wortwörtlich genommen. Ja, eine Methode soll nur eine Aufgabe erledigt. Die Aufgabe wird aber durch den Methodennamen beschrieben. Wenn die Methode ChangeFile() heißt, dann kann die Methode sehr wohl die Datei lesen, ändern und speichern, weil der Methodenname genau dies ausdrückt: Die Methode soll eine Datei ändern. Sie soll aber neben dieser Aufgabe nichts anderes machen. Die Methode ChangeFile() wird dann intern in weitere Teilaufgaben unterteilt. Nämlich Lesen, Ändern und Speichern. Der Aufrufer von ChageFile() bekommt aber davon nichts mit. Dieses Thema hat aber eigentlich nichts mehr mit Objektorientierung zu tun, sondern mit Clean Code. Natürlich kann man ChangeFile() auch wie ein Spaghetticode implementieren aber das ist nicht wirklich lesbar und vorallem nicht wartbar. Ein Entwickler verbringt mehr Zeit mit Code lesen anstatt mit Code schreiben und daher sollte der Code so lesbar wie möglich sein. Wenn die Methode ChangeFile() lediglich nur aus drei Zeilen Code besteht, dann versteht man die Arbeitsweise der Methode besser als wenn sie aus 100 Zeilen bestehen würde. Wer dann genauer in einen der drei Arbeitsschritte abtauchen muss, der schaut sich dann die Implementierung der jeweiligen Methode an, usw. 

Bearbeitet 23. Januar 2017 von Whiz-zarD

[Tician]

Ich schau mir das gerade bei Microsoft an und es sieht aus wie eine Klasse, und lässt sich anhand deines Codes wohl auch wie eine benutzen, aber statt class steht oben interface dran... was ist denn der Unterschied zwischen einer Klasse und einem interface?

[Whiz-zarD]

Ein Interface ist, wie der Name schon sagt, eine Schnittstelle. Mit Interfaces definiert man die Methodenköpfe, die eine Klasse implementiert. Über Schnittstellen lassen sich verschiedenartige Implementationen über die selbe Schnittstelle realisieren, ohne dass der Aufrufer gar nicht wissen muss, um welche konkrete Klasse es sich handelt.

Wir hatten ja schon mal das IDisposable, was ebenfalls ein Interface ist. Das Interface besitzt lediglich nur die Methode Dispose(). Viele Klassen implementieren diese Schnittstelle. Ein anderes Beispiel wäre das IEnumerable<T>-Interface, was u.a. von Arrays und Listen implementiert wird und einen Enumerator zur Verfügung stellt und mit dem Enumerator haben wir dann die Möglichkeit, mit der foreach-Schleife über die Elemente zu iterieren. 

In meinem Beispiel habe ich nun ein Interface erstellt, was einen sehr simplen Logger darstellt. Über den Konstruktor vom CsvChanger reiche ich dann ein Objekt rein, was dieses Interface implementiert. Der CsvChanger weiß dann, dass dieses Objekt die Methode Log() besitzt und ruft diese Methode auf, Ansonsten weiß es gar nichts über dieses Objekt. Der CsvChanger ist somit unabhängig vom Logger, weil dieser Logger jetzt irgendeiner sein kann, der dieses Interface implementiert. Es kann, wie in meinem Beispiel, ein Logger sein, der die Logeinträge auf die Konsole loggt, oder in die Datenbank, als Datei oder E-Mail etc.

Vielfach liest man auch, dass Interfaces für Mehrfachvererbung gedacht sind aber das ist nicht richtig und ich würde auch von dieser Meinung Abstand halten. Wer Mehrfachvererbung braucht, der macht schon was falsch. Ich bin noch nie in eine Situation gekommen, wo ich mir gewünscht hätte, C# oder Java könnte echte Mehrfachvererbung, wie es bei C++ der Fall ist. Mehrfachvererbung hat für mich den Charakter, dass eine Klasse zu viel macht.

Bearbeitet 24. Januar 2017 von Whiz-zarD

[Tician]

Ich habe diese nette Aufgabe in der Zwischenprüfung von 2014 gefunden. Es ist nur eine Antwort richtig, aber nachdem was ich jetzt durchgemacht habe hätte ich 3-4 angekreuzt

[Whiz-zarD]

Wenn man es genau betrachtet, ist gar nichts richtig.

1. Strukturierte Programmierung führt nicht zwangsläufig zu übersichtlichen und änderungsfreundlichen Programmen.
2. Es gibt weitere Paradigmen, die zur Gattung der strukturierten Programmierung zählen.
3. Es erspart nicht das Testen der Logik. (Stichwort Unittests)
4. Einfügen von Kommentaren wird nicht verboten
5. Es erleichtert auch nicht die Fehlersuche. Schon mal der Compiler formal eh keine Fehler sucht. Außer Syntaxfehler.

Ich nehme aber mal an, dass Nummer 1 richtig sein soll, weil man damit Programm strukturiert aufbauen kann. Die Betonung liegt wohlgemerkt auf kann. Eine strukturierte Programmierung legt erst mal keine Regeln fest, wie entwickelt werden muss bzw. soll. Das liegt in der Disziplin der Entwickler selbst. Man kann auch mit einer Objektorientierten Sprache kompletten Murks bauen. So habe ich schon Klassen gesehen, die über 15.000 Zeilen besitzen oder Methoden, die eine zyklomatische Komplexität von über 1.000 besitzen oder Methoden, die über 10 ref-Parametern besitzen. All dies führt nicht zu einem übersichtlichen und änderungsfreundlichen Code.

Darum gibt es ja Tools, die einen Entwickler dazu zwingen, strukturell sauberen Code zu schreiben. z.B. StyleCop.
Der Compiler von der Sprache Go hat schon so ein Tool integriert. So ist es z.B. nicht möglich, sein Code zu kompilieren, wenn man eine Variable definiert hat aber nirgends verwendet.