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

[thereisnospace]

vor 10 Minuten schrieb Tician:

Verstehe ich das richtig, ich muss um die Source aus der ini-Datei zu bekommen eine Methode machen und brauche für die Rückgabe nochmal eine Methode?

    class Settings
    {
        public string source { get; set; }
      
      public Settings() {
        this.source = GetSource();
      }

        public String GetSource()
        {
            try
            {
                Regex rsource = new Regex("(?<=Source\\=).+");
                Match msource = rsource.Match(iniText);
                return Convert.ToString(msource);
            }
            catch
            {
                Console.WriteLine("Could not find 'Source' in settings");
            }            

Ich würde im Konstruktor (den du bisher noch nicht hast) die Source über die Methode setzen.

[thereisnospace]

Das Schöne ist, dass du über diese Programmierweise, diese Zeilen einfach löschen kannst!
Die Informationen sind bereits durch die Initialisierung in dem Objekt settings1 vorhanden.

Nun kannst du über settings1 bereits auf die Variable source zugreifen

Settings settings1 = new Settings();

indem du settings1.source nimmst.
Das ist ein Vorteil der OOP

                Settings settings1 = new Settings();

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

                Console.ReadKey();
            }

So würde das dann schon funktionieren

Bearbeitet 16. Januar 2017 von Gottlike

[Whiz-zarD]

Dein Problem mit dem '\r' ist folgendes:
Du hast ein String, der aus mehreren Zeilen besteht. Das Zeilenende unter Windows wird standardgemäß mit "\r\n" dargestellt. Das sind nicht-druckbare Steuerzeichen. Das \r steht für "cariage return" und das \n für "new line". Reguläre Ausdrücke werden aber standardgemäß pro Zeile ausgewertet und das eine Zeile wirklich beendet ist, leitet nur das \n ein. Somit bleibt \r stehen.

Wenn du es unbedingt mit einen regulären Ausdruck ermitteln möchtest, dann musst du es so schreiben:

Regex rsource = new Regex("(?<=Source\\=)[^\r]+");

Aber reguläre Ausdrücke würde ich persönlich vermeiden. Die versteht doch kein Schwein. Ich würde durch jede Zeile der Ini-Datei iterieren und jede Zeile per Split() mit dem '='-Zeichen teilen. So habe ich den Schlüssel und den Wert getrennt und kann damit arbeiten. 

Versuche aber mal die Settings und das Laden der Settings zu trennen. Also dass du eine Klasse Settings hast:

public class Settings
{
    public string source { get; set; }
    public string Destination { get; set; }
}

Und eine Klasse z.B. mit dem Namen SettingsLoader, die eine Methode bereitstellt, die die Settings aus der Ini-Datei lädt und dir eine Instanz vom Typ Settings zurückliefert. Also z.B:

public class SettingsLoader
{
    public Settings Load(string fileName)
    {
        ...
    }
}

So trennst du die Datenstruktur von der Logik. Angenommen, du hast neben der Ini-Datei als Quelle für die Einstellungen jetzt noch eine Datenbank. Dann müsstest du ja die Settings-Klasse um weitere Methoden aufblähen, die die Daten aus der Datenbank lesen und dann kommt vielleicht die dritte (z.B. aus Textboxen einer WinForms-Anwendung), vierte und fünfte Quelle. Dann wird die Klasse sehr riesig und unübersichtlich. Das möchte man aber nicht. Man sollte das Ziel verfolgen, eine Klasse so klein wie möglich zu halten. Einige sagen, eine Klasse darf nicht mehr als 200 Zeilen besitzen aber ich finde so dogmatisch sollte man es nicht halten, aber man sollte sich daran orientieren. Um also mehrere Quellen abzugrasen sollte man pro Quelle eine Klasse schreiben, die die Einstellungen einliest und zurückgibt.

Bearbeitet 16. Januar 2017 von Whiz-zarD

[Saheeda]

Ich würde mir für die Klasse "Settings" einen Konstruktor bauen, der drei String-Variablen für die drei Felder entgegen nimmt.

Am Ende der Methode "Load" würde ich mir ein Objekt "Settings" bauen (dabei die drei Variablen setzen" und dieses Settings-Objekt dann zurückgeben.

 

Also ungefähr so:

 

public Settings Load(string filename)
{
    [...]

	string source = Convert.ToString(msource).Replace("\r", "");
	string destination = Convert.ToString(mdestination).Replace("\r", "");							
			
	return new Settings(iniText, source, destination);			
}
	

 

[thereisnospace]

Versuch bei Methodenaufrufen wie

        public void GetSource()

einen anderen Rückgabewert als void (f.e. bei dir einen Pfad also String) zu nehmen.

Dann kannst du der Variable source ihren Wert über den Methodenaufruf geben.

source=Settings.GetSource();

(Die Instanziierung und Initialisierung habe ich mir gespart)

[Tician]

Verstehe ich das richtig, ich muss um die Source aus der ini-Datei zu bekommen eine Methode machen und brauche für die Rückgabe nochmal eine Methode?

Ne die Variable müsste über get und set ja schon selbst etwas haben... ich bin verwirrt. Wie setze ich denn die source-variable nachdem ich die ini-Datei ausgelesen habe?

Bearbeitet 16. Januar 2017 von Tician

[Tician]

        public string GetSource()
        {
            try
            {
                Regex rsource = new Regex("(?<=Source\\=).+");
                Match msource = rsource.Match(iniText);
                return Convert.ToString(msource);
            }
            catch
            {
                Console.WriteLine("Could not find 'Source' in settings");
            }            
        }

Ich hatte es ähnlich versucht und kam auf dasselbe Ergebnis:

"Settings.GetSource(): nicht alle Codepfade geben einen Wert zurück"

Deswegen bin ich auch verwirrt.

Bearbeitet 16. Januar 2017 von Tician

[thereisnospace]

vor 3 Minuten schrieb Tician:

        public string GetSource()
        {
            try
            {
                Regex rsource = new Regex("(?<=Source\\=).+");
                Match msource = rsource.Match(iniText);
                return Convert.ToString(msource);
            }
            catch
            {
                Console.WriteLine("Could not find 'Source' in settings");
                return "";
            }            
        }

Ich hatte es ähnlich versucht und kam auf dasselbe Ergebnis:

"Settings.GetSource(): nicht alle Codepfade geben einen Wert zurück"

Das liegt daran, dass der Catch-Block keinen Rückgabewert hat. So sollte es widerrum funktionieren (auch wenn das kein vernünftiges Exception Handling ist).

[Tician]

Apropo Exception handling. Da ich alles in meiner Main aufrufe, könnte ich nicht alle Exceptions weglassen bis auf das hier?

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

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

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

Oder gilt das nicht?

[thereisnospace]

Klar würde das funktionieren. Aber der Anwender bzw. nächste Entwickler freut sich, wenn er nach dem Fehler bzw. der Methode sucht, welche die Exception ausgelöst hat.

[Tician]

Also gut^^

Ich musste jetzt auch das ReadFile() in den Konstruktor nehmen sonst funktionieren die Getter nicht weil die ini-Datei fehlt.

Neuer Code sieht so aus:

    class Program
    {
        static void Main(string[] args)
        {
            string source;
            string destination;
            try
            {
                Settings settings1 = new Settings();
                source = settings1.GetSource();
                destination = settings1.GetDestination();

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

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




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

        public Settings()
        {
            ReadFile("settings.ini");
            this.source = GetSource();
            this.destination = GetDestination();
        }

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




    class CsvChanger
    {
        private string[] changedContent;
        private int fileCount;
        private string[] files;
        private string[] fileContent;
        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");
            }
        }
    }

Das Problem mit dem '\r' beim konvertieren zu einem string bleibt :/

Bearbeitet 16. Januar 2017 von Tician

[thereisnospace]

So sieht die Klasse schon viel besser aus!
Bin nicht so bewandt mit Regex und der .Net Welt, das Problem überlasse ich wen anders

[Tician]

Immerhin das sagt mir ich bewege mich in die richtige Richtung, das freut mich

[Tician]

@Gottlike

Ich denke du kannst mir nochmal helfen:

Main:                

		Settings settings1 = new Settings();
                source = settings1.GetSource();
                destination = settings1.GetDestination();

Beim Zeilenweisen durchgehen rufe ich GetSource() sowohl im Konstruktor als auch noch einmal über die Methode selbst auf. Es wird also 2 mal durchlaufen. Ist das Sinn der Sache und normal? Oder kann man das so hinbasteln das GetSource() nur ein einziges mal durchläuft und ich die Variable in meiner Program class zur verfügung habe?

[Tician]

Gecheckt! So hatte ich es ja auch als allererstes aus irgendeinem Grund dachte ich ich müsste es ändern...

[Tician]

Das Seltsame an den Regex ist das ich das bisher immer benutzt habe und auch immer mit mehreren Zeilen in der ini-Datei und nie hatte ich dieses '\r' beim konvertieren mit drin.

@Whiz-zarD Könnte ich ein kurzes Beispiel haben was Datenstruktur und was Logik ist? Ist Datenstruktur alles was ich deklariere, während Logik die Ausführung von Vorgängen ist?

public class SettingsLoader
{
    public Settings Load(string fileName)
    {
        ...
    }
}

Das habe ich noch nie gesehen, ich weiß gar nicht was es macht. Wie nennt sich das, nach was kann ich schauen? Kann man das ohne Fachbegriffe beschreiben oder gibt es eine andere Schreibweise wie man das schreiben könnte um es zu verstehen (wie get und set auszuschreiben)?

Ich kenne bisher nur Klassen mit eigenen Methoden (mit oder ohne Rückgabe eines mir bekannten Datentyps) und den Konstruktor. Und das ich Variablen über die Klammern in die Methode einer anderen Klasse 'übergeben' kann.

Als Rückgabewert eine Klasse zu haben ist mir neu und verwirrt mich gerade etwas, ich versuche ein Beispiel zu finden um es zu verstehen aber irgendwie ist das recht schwierig.

Bearbeitet 18. Januar 2017 von Tician

[Whiz-zarD]

Eine Datenstruktur ist ein Objekt zur Speicherung und Organisation von Daten. Dabei sollten solche Objekte so "dumm" wie möglich gehalten werden.
Im Jahr 2000 brachte Martin Fowler den Begriff "POJO" in den Raum. Das steht für "Plain old Java Object". Abgelitten für die .Net-Welt gibt es den Begriff POCO, was für "Plain Old CLR Object" steht. Solche Objekte besitzen halt nur die Daten. Beispiel: Du willst Daten zu einer Person speichern:

public class Person
{
    public string Vorname { get; set; }
    public string Nachname { get; set; }
    public DateTime Geburtstag { get; set; }
}

Das ist jetzt die Datenstruktur für eine Person. Wenn du nun eine Menge an Personen hast, kannst du jetzt z.B. ein Array oder eine Liste mit Personen definieren. Beispiel:

Person[] personen = new Person[10];
IList<Person> personen2 = new List<Person>();

So transporierst du lediglich nur die Daten der Person durch die Gegend. Der Vorteil von solchen Klassen ist, dass sie komplett keine Abhängigkeiten besitzen und können wiederverwendet werden. Angenommen du würdest in der Klasse Methoden hinzufügen, die die Daten aus einer Datenbank liest. Dann wäre die Klasse von der Datenbank abhängig.
Analog kann man es ja mit deiner Settings-Klasse sehen. Du hast dort Methoden, die die Setttigs aus einer Ini-Datei ausliest. 

Unter Logik versteht man das, was du letzendlich machen möchtest. z.B. die Settings aus einer Ini-Datei oder Datenbank lesen.

[Tician]

Also gut, wie gesagt ich habe keine Ahnung wie man das benutzt und habe mal sämtliche Errors und Warnungen ignoriert und es so hingebastelt wie ich dachte das es (so oder so ähnlich) funktionieren würde:

class Program
    {
        static void Main(string[] args)
        {
		SettingsLoader loader = new SettingsLoader();
            Settings settings1 = loader.Load("settings.ini");
	}
}




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




    class SettingsLoader
    {
        string filename;
        string iniText;

        public Settings Load(string filename)
        {
            this.filename = filename;
            iniText = File.ReadAllText(filename);

            Regex rsource = new Regex("(?<=Source\\=).+");
            Match msource = rsource.Match(iniText);            

            Regex rdestination = new Regex("(?<=Destination\\=).+");
            Match mdestination = rdestination.Match(iniText);


            return Convert.ToString(mdestination).Replace("\r", "");
            return Convert.ToString(msource).Replace("\r", "");
            return iniText;
        }
    }

Das Hauptproblem ist das ich null Ahnung von der Rückgabe habe, das ich nicht mehrmals "return" haben kann ist mir klar, das soll auch mehr symbolisch zeigen was ich eigentlich machen möchte, ich brauche schließlich 3 Variablen brauche. Außerdem sind meine getter und setter so irgendwie nutzlos... sorry ich blicks nicht ._.

[thereisnospace]

vor 37 Minuten schrieb Tician:

Das Hauptproblem ist das ich null Ahnung von der Rückgabe habe, das ich nicht mehrmals "return" haben kann ist mir klar, das soll auch mehr symbolisch zeigen was ich eigentlich machen möchte, ich brauche schließlich 3 Variablen brauche. Außerdem sind meine getter und setter so irgendwie nutzlos... sorry ich blicks nicht ._.

Als Rückgabewert eine Klasse zu haben ist mir neu und verwirrt mich gerade etwas, ich versuche ein Beispiel zu finden um es zu verstehen aber irgendwie ist das recht schwierig.

Du hast hier als Rückgabewert keine Klasse, sondern ein explizites Objekt dieser Klasse.

Ich würde dir nochmal ans Herz legen diese Seite bezüglich Klassen/Objekte anzugucken. Klassen sind nur "Baupläne" für Objekte.

SettingsLoader loader = new SettingsLoader();

Hier erstellst du sogesehen mithilfe des Bauplans ein Objekt mit dem Namen loader. Dieses Objekt beinhaltet dann die Methoden und Attribute aus diesem Bauplan.

Bearbeitet 18. Januar 2017 von Gottlike

[Whiz-zarD]

vor einer Stunde schrieb Saheeda:

Ich würde mir für die Klasse "Settings" einen Konstruktor bauen, der drei String-Variablen für die drei Felder entgegen nimmt.

Und genau das würde ich nicht tun. Du machst dich da zu sehr abhängig und der Struktur der Klasse. Jeder, die die Klasse initialsiert, muss im Vorwege alles kennen. Du kannst die Klasse nicht Stück für Stück befüllen. Außerdem bietet C#  da schon eine elegante Objektinitialisierung:

return return new Settings
{
    IniText = iniText,
    Source = source,
    Destination = destination
};

 

[Tician]

Also das Programm funktioniert wieder und soweit war ich mit meiner Idee gar nicht weg *freu*

Da sollen noch ein paar Dinge dazu kommen, aber die würde ich später hinzufügen wenn das Programm so wie es ist mal an die durchschnittlichen Ansprüchen der OOP ran kommt.

Stand der Dinge:

    class Program
    {
        static void Main(string[] args)
        {
            try
            {
                SettingsLoader loader = new SettingsLoader();
                Settings settings1 = loader.Load("settings.ini");

                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
    {
        public string iniText;
        public string source { get; set; }
        public string destination { get; set; }
    }




    class SettingsLoader
    {
        string filename;
        string iniText;

        public Settings Load(string filename)
        {
            this.filename = filename;
            iniText = File.ReadAllText(filename);

            Regex rsource = new Regex("(?<=Source\\=).+");
            Match msource = rsource.Match(iniText);            

            Regex rdestination = new Regex("(?<=Destination\\=).+");
            Match mdestination = rdestination.Match(iniText);

            return new Settings
            {
                source = Convert.ToString(msource).Replace("\r", ""),
                destination = Convert.ToString(mdestination).Replace("\r", ""),
                iniText = iniText
            };
        }
    }




    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 (fileCount == 0)
                {
                    Console.WriteLine("Keine Dateien gefunden. Bitte Taste drücken...");
                    Console.ReadKey();
                    Environment.Exit(0);
                }
                Console.WriteLine("{0} Dateien gefunden!", fileCount);
            }
            catch
            {
                Console.WriteLine("Error finding files");
            }
        }

        public void ReadCsv()
        {
            try
            {
                fileContent = new string[fileCount];

                for (int x = 0; x < fileCount; x++)
                {
                    fileContent[x] = File.ReadAllText(files[x]);
                }
                Console.WriteLine("Dateien erfolgreich eingelesen!");
            }
            catch
            {
                Console.WriteLine("Couldn't read CSV");
            }
        }

        public void ChangeCsv()
        {
            try
            {
                changedContent = new string[fileCount];

                for (int x = 0; x < fileCount; x++)
                {
                    changedContent[x] = fileContent[x].Replace("\"", "");
                }
                Console.WriteLine("Anführungszeichen erfolgreich ersetzt!");
            }
            catch
            {
                Console.WriteLine("Couldn't replace file-content");
            }
        }

        public void WriteFiles(string destination)
        {
            try
            {
                this.destination = destination;
                for (int x = 0; x < fileCount; x++)
                {
                    using (StreamWriter sw = new StreamWriter(destination + Path.GetFileName(files[x])))
                    {
                        sw.Write(changedContent[x]);
                    }
                }
                Console.WriteLine("Dateien in Ziel-Pfad geschrieben! \n Fertig :)");
            }
            catch
            {
                Console.WriteLine("Error writing files to destination");
            }
        }
    }

@GottlikeDen Link kenne ich schon (immerhin besser als der vorherige für Delphi :P) und bis auf die Überladungen (bzw wie man die schreiben kann) kenne ich alles schon. Allgemein das mit der Rückgabe habe ich zwar in diesem Thread erst verstanden, aber das dann statt den mir bekannten Datentypen (int, byte, string,...) plötzlich mit einer Klasse hat mich verwirrt, aber auch das habe ich denke ich jetzt verstanden.

Bearbeitet 18. Januar 2017 von Tician

[Whiz-zarD]

In der Klasse SettingsLoader fällt auf, dass du filename als private Member definiert hast, aber das brauchst du gar nicht. Also könntest du diesen auch löschen. iniText hat auch in Settings nichts zu suchen. Außerdem macht die Load()-Methode eigentlich noch zu viel. Schauen wir uns doch mal an, was sie macht:

1. Sie liest die Ini-Datei ein
2. Sie ermittelt Source
3. Sie ermittelt Destination
4. Sie gibt Settings zurück

Das sind quasi vier Aufgaben und die ersten drei davon könnte man in separate Methoden packen, um den Code genauso lesbar zu machen, wie meine Aufzählung:

class SettingsLoader
{
    string iniText;

    public Settings Load(string filename)
    {
        this.iniText = this.ReadFile(filename);

        return new Settings
        {
            Source = this.GetSourceFromIniText();
            Destination = this.GetDestinationFromIniText();
        }
    }
    
    ...
}

Die Implementierungen der Methoden kannst du mal versuchen.
Hierbei kannst du ja auch mal versuchen, das DRY-Prinzip (Don't Repeat Yourself) einzuhalten. Vielleicht fällt dir ja auf, dass das Lesen von Source und Destination quasi die selbe Aufgabe ist. Lediglich nur der reguläre Ausdruck ändert sich.

Außerdem fällt in der Klasse CsvChanger auf, dass du zu viele Try-Catch-Blöcke drinnen hast. Die Catch-Blöcke geben auch immer was auf der Console aus. Damit ist die Klasse von Console abhängig. Für den Anfang kannst du es ja auch so lassen aber stelle dir mal die Frage, ob das so gut ist bzw. warum das nicht so gut sein könnte?

Die Klasse CsvChanger würde auch einfacher zu lesen sein, wenn sie nur eine Datei bearbeiten würde. In der Hauptmethode könnte man ja die Dateien ermitteln und mittels einer Schleife iterieren und jedes Mal den CsvChanger aufrufen. Damit verlagert man die Logik weiter zum Aufrufer und die Klasse wird dadurch kompakter. Außerdem sehe ich, dass du dir mit Arrays ganz schön einen abbrichst.  Informiere dir mal ein bisschen über Generics und dann über die Klasse List<T>. Ich verwende Arrays nur, wenn ich schon zur Entwicklungszeit exakt weiß, wie viele Elemente ein Array braucht. z.B. ein Schachbrett: Es hat immer 8x8 bzw. 64 Felder und das wird sich auch in hundert Jahren nicht ändern. Eine Anzahl von Dateien in einem Ordner kann sich sekündlich ändern und daher ist ein Array vielleicht nicht die beste Wahl. Außerdem schaue dir mal die foreach-Schleife an. Dann wirst du auch die hässlichen for-Schleifen los.  

Bearbeitet 18. Januar 2017 von Whiz-zarD

[Tician]

Hatte gestern Berufsschule und bin etwas genervt weil ich das hier durchziehen möchte, aber immer neue Anforderungen kommen und ich versuche die Zeit zu finden mir diese Dinge für die OOP anzueignen, aber momentan sieht es so aus das Programme schnell fertig werden müssen und diese schnell untereinander zu schreiben dauert 1/5 von der Zeit die ich brauchen würde um das ganze mit OOP zu basteln.

Zeitgleich kommt hinzu das ich durch meine gewählte Fachrichtung auch anderes zu tun habe, mein eigenes kleines Netzwerk schimmelt schon vor sich hin (von meiner Ausbildungsnachweise jede Woche könnte man definitiv meinen ich bin FIAE )

@Whiz-zarDDeine Vorschläge sind super! Ich denke ich habe es auch verstanden und modelliere den Code um sobald ich kann!

[lilith2k3]

Zu der eigentlichen Frage, zum Thema: Objektorientierung.

Nein. Das ist keine Objektorientierung. Das ist Programmieren mit "Klassen". Was per se erstmal nicht schlimm ist. 

Das Problem ist, dass Deine "Objekte" zwar alle Funktionalität zusammen mit irgendwelchen Daten vereinen - was in Richtung OOP geht - aber die eigentliche Kern-Idee von "Objekten" als eigenständigen Funktionseinheiten ist nicht umgesetzt. 

Du behandelst Deine Objekte eher wie "Sklaven" - um es mal so auszudrücken. Du definierst eine Klasse »Settings«, die eigentlich dazu dienen sollen, Dir Informationen (von wo auch immer) bereitzustellen. Dabei gehst Du zunächst hin und rufst die Methode »ReadFile« von extern auf. Das ist aus zweierlei Gründen "schlecht": 

1) Du "leakst" nach außen die Information, dass Du die Settings aus einem File liest. Das ließe sich besser kapseln, indem Du bspw. den Filenamen über einen Konstruktorparameter übergibst. Dann ist zwar immer noch offensichtlich, dass es sich um ein File handelt, aber schon mal ein Schritt in die richtige Richtung. Desweiteren würde ich den "String" irgendwie kapseln wollen, da es sich um keinen String im herkömmlichen Sinn, sondern um einen Pfad handelt; also sowas wie https://msdn.microsoft.com/en-us/library/system.io.fileinfo(v=vs.110).aspx oder so. Aber mein C# ist schon 5 Jahre alt

2) Und wesentlich "problematischer" finde ich, dass Du dem Objekt von außen erzählen musst, was es wann zu tun hat:

* Lese Ini. 

* Hole diesen Wert

* Hole jenen Wert

Das ist per se nicht schlimm - nur nicht objektorientiert. 

Objektorientiert würde es dadurch, dass Du dem Objekt die Aufforderung zukommen lässt, diesen oder jeden Konfigurationswert zurückzugeben.

Das gleiche gilt für Deinen "Changer". 

 

Im Grunde sollte alles soweit in die Objekte gekapselt werden, dass Du in der Main nur noch folgende Sachen machst:

a ) Initialisieren der Settings

b ) Initialisieren Deines "Changers" mit den "Settings" im Konstruktor

c) aufrufen der Methode, die den "Job" ausführt.

Wenn es weiterhilft: Es gibt die weitverbreitete Metapher, dass Methodenaufrufe als "Nachrichten" an ein Objekt zu verstehen sind.

So, wie Du in die Kneipe gehst und ein Bier bestellst (»Bitte ein Bier«) und nicht:

»Also. Erstmal gehst Du zum Tresen. Dann suchst Du mir ein Glas aus. Wenn's schmutzig ist, spüle es. Dann gehst Du zum Zapfhahn ...«

Ich denke, es wird klar, was ich meine. Ansonsten: Viel Spaß beim Coden  

[Tician]

Eins nach dem anderen, auch wenn mich das gerade etwas frustriert das das was ich unter OOP verstanden habe wohl keine ist

Was habe ich gemacht:

- 'filename' aus Settingsloader entfernt, jetzt direkt den Dateinamen statt Variable

- 'iniText' aus Settings entfernt

- Neue Klasse Regex erstellt (und festgestellt Regex als Klassenname ist ne bescheuerte Idee) -> umbenannt in RegexSuche, festgestellt ich brauche keine Klasse dafür -> Methode in SettingsLoader hinzugefügt (nach dem DRY-Prinzip)  

@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. 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.

- Suche nach Dateien in der Main in eine Liste

- iterieren durch die Liste

- CsvChanger.FindCsv gelöscht

- CsvChanger-Methoden angepasst

- try-catch-Blöcke gelöscht, einzigen Block mit einem Streamwriter versehen.

 

@lilith2k3Ich 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?

 

Und hier wieder die überarbeitete Version:

    class Program
    {
        static void Main(string[] args)
        {
            List<string> filelist = new List<string>();

            try
            {
                SettingsLoader loader = new SettingsLoader();
                Settings settings1 = loader.Load();

                filelist = Directory.GetFiles(settings1.source, "GAR_EXPORT_*.csv").ToList();

                foreach(string file in filelist)
                {
                    CsvChanger changer = new CsvChanger();
                    changer.ReadCsv(file);
                    changer.ChangeCsv();
                    changer.WriteFiles(settings1.destination);
                }     
                Console.ReadKey();
            }
            catch (Exception ex)
            {
                using (StreamWriter writer = new StreamWriter("\\\\fileserver\\it-pm\\Monitoring\\temporaer.txt", true))
                {
                    writer.WriteLine(DateTime.Now.ToString() + " " + ex.Message);
                }
            }
        }
    }





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




    class SettingsLoader
    {
        string iniText;
        string suche;

        public Settings Load()
        {
            iniText = File.ReadAllText("settings.ini");

            Match msource = GetLineInIni("(?<=Source\\=).+");            
            Match mdestination = GetLineInIni("(?<=Destination\\=).+");

            return new Settings
            {
                source = Convert.ToString(msource).Replace("\r", ""),
                destination = Convert.ToString(mdestination).Replace("\r", ""),
            };
        }

        public Match GetLineInIni(string suche)
        {
            this.suche = suche;
            Regex reg = new Regex(suche);
            Match mat = reg.Match(iniText);
            return mat;
        }
    }




    class CsvChanger
    {
        private string fileContent;
        string source;
        string destination;

        string file;
        string changedContent;

        public void ReadCsv(string file)
        {
            this.file = file;
            fileContent = File.ReadAllText(file);
            Console.WriteLine("Dateien erfolgreich eingelesen!");
        }

        public void ChangeCsv()
        {
            changedContent = file.Replace("\"", "");
            Console.WriteLine("Anführungszeichen erfolgreich ersetzt!");
        }

        public void WriteFiles(string destination)
        {
            this.destination = destination;
            using (StreamWriter sw = new StreamWriter(destination + Path.GetFileName(file)))
            {
                sw.Write(changedContent);
            }
            Console.WriteLine("Dateien in Ziel-Pfad geschrieben!");
        }
    }