Laden...

Fragen zur korrekten objektorientierten Programmierung

Erstellt von R3turnz vor 8 Jahren Letzter Beitrag vor 8 Jahren 13.631 Views
R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren
Fragen zur korrekten objektorientierten Programmierung

Hallo,
ich habe gerade mein erstes richtiges Projekt vertiggestellt. Es ist ein Tool um eine Passwort-Liste einzulesen und damit sein Passwort zu überprüfen:

Program.cs:


    delegate StreamReader Loadmethode(string path);
    class Program
    {
        
        static string targetedpassword = null;
        static Directory_System tempfiles;
       
       
        
        static void Main(string[] args)
        {
            Console.Title = "Password-Checker";
            try
            {
                tempfiles = new Directory_System(Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "password_checker_temp"));
                Console.WriteLine("Created Temp-Folder:\n" + tempfiles.DirectoryPath);
            }
           catch
            {
                Console.WriteLine("Cant create Temp-Folder!");
            }
        if(tempfiles != null)
            {
                Console.WriteLine("This programm will read a passwordlist (.zip or .txt file)\nand check about the accordance to one password of the list.");
                Console.Write("Path to the password-list:");
                string path = Console.ReadLine();

                FileInfo info = null;





                info = new FileInfo(path);


                if (info != null && info.Exists)
                {
                    Loadmethode LoadmethodeHandler = null;
                    string detectedmethod = null;
                    if (info.Extension == ".zip")
                    {
                        detectedmethod = ".zip";
                        LoadmethodeHandler = ziploadsetup;

                    }
                    else if (info.Extension == ".txt")
                    {
                        detectedmethod = ".txt";
                        LoadmethodeHandler = Load_Methods.loadtxt;
                    }
                    if (detectedmethod != null)
                    {
                        Console.WriteLine("Detected your file as a:{0} file.", detectedmethod);
                    }
                    else
                    {
                        Console.WriteLine("Can not detect your data type.");
                        LoadmethodeHandler = null;
                        Console.Write("Methods:\n0.zip\n1.txt\nChoose:");

                        if (Console.ReadKey().Key == ConsoleKey.D0)
                        {
                            LoadmethodeHandler += ziploadsetup;
                        }

                        else
                        {
                            LoadmethodeHandler += Load_Methods.loadtxt;
                        }
                    }

                    StreamReader reader = LoadmethodeHandler(path);


                    if (reader != null)
                    {
                        Console.Write("Your Password:");
                        targetedpassword = Console.ReadLine();
                        Console.Write("Press any key to start reading...");
                        Console.ReadKey();
                        Console.Clear();
                        Console.WriteLine("{0,-20}{1}", "password", "accordance");
                        bool founded = false;
                        while (reader.Peek() != -1)
                        {
                            string readedline = reader.ReadLine();
                            if (readedline == targetedpassword)
                            {
                                Console.WriteLine("{0,-20}{1}", readedline, "TRUE");
                                founded = true;
                                break;
                            }
                            else
                            {
                                Console.WriteLine("{0,-20}{1}", readedline, "FALSE");
                            }

                        }
                        if (founded == true)
                        {
                            Console.WriteLine("Founded your password! Better chosse another one.");

                        }
                        else
                        {
                            Console.WriteLine("Did not found your password.");
                        }
                        reader.Close();

                    }

                }
                else
                {
                    Console.WriteLine("Your file does not exists.");
                }
                Console.WriteLine("Cleaning up the temp programm data.");
                tempfiles.Dispose();
                Console.ReadKey();
            }    
       
        }
        public static StreamReader ziploadsetup(string path)
        {
            Console.WriteLine();
            StreamReader reader = null;
            FileStream fs = null;
            try
            {
                fs = new FileStream(path, FileMode.Open);
            }


            catch
            {
                Console.WriteLine("Can not open your path");
            }
            ZipArchive archive = null;
            if (fs != null)
            {
                try
                {
                    archive = new ZipArchive(fs, ZipArchiveMode.Update);

                }
                catch
                {
                    Console.WriteLine("Can not open as a Zip-File.");
                }
            }
            if (archive != null)
            {
                Console.WriteLine("Zip-loading:");
                Console.WriteLine("Entries in path:");
                for (int i = 0; i <= (archive.Entries.Count - 1); i++)
                {
                    Console.WriteLine("{0}.{1}", i, archive.Entries[i].Name);

                }


                Console.WriteLine("Choose your Textdocument\n(The program will try to open any document as an .txt file):");
                string documentchoose = Console.ReadLine();
                int a = Parser_Methods.Parse_string_to_int(documentchoose, 0, (archive.Entries.Count - 1));
                if (a != -1)
                {
                    Console.Write("The programm supports to zip loading methods:\n0.Loading in RAM/{0}\n1.Extracting to file and reading.\nChoose:", archive.Entries[a].Length + " Bytes");
                    string loadingmethode = Console.ReadLine();
                    int b = Parser_Methods.Parse_string_to_int(loadingmethode, 0, 1);
                    if (b != -1)
                    {
                        switch (b)
                        {
                            case 0:
                                Console.WriteLine("The file is maybe to large and can overload your RAM:{0}.\nDo you really want to start loading? Y/N", archive.Entries[a].Length + " Bytes");
                                if (Console.ReadKey().Key == ConsoleKey.Y)
                                {
                                    reader = Load_Methods.load_zip_in_ram(archive.Entries[a]);
                                    Console.WriteLine("\nLoaded successfull:\n" + Path.Combine(path, archive.Entries[a].FullName));
                                }
                                else
                                {
                                    Console.WriteLine("Exiting...");
                                }
                                break;
                            case 1:
                                reader = Load_Methods.load_zip_from_extractet_file(archive.Entries[a],tempfiles.DirectoryPath);
                                break;
                        }
                    }
                    else
                    {
                        Console.WriteLine("Can not parse your input.");

                    }

                }
                else
                {
                    Console.WriteLine("Can not parse your input.");
                }


            }
            return reader;
        }


    }


Load_Methods.cs:


static class Load_Methods
    {
        public static StreamReader loadtxt(string path)
        {
            Console.WriteLine();

            StreamReader reader = null;
            try
            {
                reader = new StreamReader(path);

            }
            catch
            {
                Console.WriteLine("Can not open as .txt!");

            }
            Console.WriteLine("Succesfully loaded as .txt!");
            return reader;
        }
  
        public static StreamReader load_zip_in_ram(ZipArchiveEntry entry)
        {
            Stream stream = entry.Open();
            StreamReader reader = new StreamReader(stream);

            return reader;
        }
        public static StreamReader load_zip_from_extractet_file(ZipArchiveEntry entry,string destination )
        {
            StreamReader reader = null;
            try
            {

                Directory.CreateDirectory(destination);
                string filedestination = Path.Combine(destination, "temp.txt");
                Console.WriteLine("Writing to this file:\n" + filedestination);
                entry.ExtractToFile(filedestination);

                reader = new StreamReader(filedestination);
                Console.WriteLine("Completed!");
            }
            catch
            {
                Console.WriteLine("Error while extracting or reading out of temp file!");
            }
            return reader;
        }



    }

Directory_System.cs:


        public class Directory_System: IDisposable
        {
            private string directorypath;

            public string DirectoryPath
            {
                get { return directorypath; }
                set
            {
                if(Directory.Exists(value))
                directorypath = value;
                else if(!Directory.Exists(value))
                {
                    try
                    {
                        Directory.CreateDirectory(value);
                        directorypath = value;
                    }
                catch
                    {
                        throw new Exception();                  
                    }
                    }
            }
            }

        public Directory_System(string destinationpath)
            {
            this.DirectoryPath = destinationpath;
        }
        bool disposed = false;
        public void Dispose()
        {
            if (!disposed)
            {

                Dispose(true);
                GC.SuppressFinalize(this);
                disposed = true;
            }
        }
        protected virtual void Dispose(bool disposing)
        {
            if(disposing)
            {
                Directory.Delete(DirectoryPath, true);
                this.DirectoryPath = null;
                
            }
            else
            {
                Directory.Delete(DirectoryPath, true);
            }
        }
        ~Directory_System()
        {
            Dispose(false);
        }
    }


Parser_Methods.cs:


 static class Parser_Methods
    {
        public static int Parse_string_to_int(string value, int min, int max)
        {
            int output = 0;
            bool parsesuccess = int.TryParse(value, out output);
            if (parsesuccess && output >= min && output <= max)
            {

            }
            else
            {
                output = -1;
            }
            return output;

        }
    }

Nun zu meiner Frage: Habe ich die Objektorientierung richtig angewandt?
Es wäre zwar nett wenn jemand den Code durchsehen würde, aber hier ein paar Fragen die ich mit stelle:
-Ab wann sollte ich Methoden in eigne Klassen verschieben? (z.B. Parser_Methods.cs)
-Sollte man für Directory_System eine eigene Klasse machen, oder einfach einen string in Main() mit dem Pfad dazu verwalten?
-Ist die Verwendung von Dispose() richtig statt einfach z.B. Clean_up_Directory bereitzustellen?

-Sollte ich in Load_zip_from_extract_file den Pfad zum Programm-Ordner als einfachen string übergeben, oder das Directory_System Objekt mit der Pfadeigenschaft weitergeben?

Vielen Dank im voraus für die Antworten, klar ist es ein bisschen Aufwand den Code durchzusehen, ich habe mich in meinem ersten richtigen Objekt hier aber bemüht die Varaiblen-Namen möglichst einfach lesbar zu halten.

Grüße!

PS: Hoffe das es nun besser verständlich ist, der Code dürfte nun normal lesbar sein, und ich hab die Fragen nochmmal anders formuliert, um sie besser verständlich zu machen....Ich poste einfach mal hier, da es meiner ANsicht besser passt als in die Grundlagen zu C# Rubrik

T
2.219 Beiträge seit 2008
vor 8 Jahren

Auch wenn dies dein erstes Projekt ist, solltest du den Code überarbeiten.
Ohne den Code im Detail zu prüfen, solltest du die Methoden bei der Benennung an die C# Konventionen anpassen.
Auch solltest du deine StreamReader und co. wieder schließen bzw. besser sogar direkt in einem using Block packen.

Weitere Tipps können die noch andere geben, die sich den Code hoffentlich im Detail anschauen.
Eine sinnvolle OO Programmierung kann ich hier leider nicht erkennen, da deine zusatzklassen aus meiner Sicht kaum Sinn machen.

Hier wäre es sinnvoller die Methoden direkt in der Main Klasse umzusetzen.
Da dies auch keine Methoden sind, die du andersweitig wiederverwenden dürftest, macht eine Aufsplittung nur minimalen Sinn.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

P
1.090 Beiträge seit 2011
vor 8 Jahren

Ich hab jetzt deinen Code überflogen und mir sind direkt ein paar Sachen aufgefallen.

Deine Methoden sind teils deutlich zu lang und zu tief Verschachtelt. Was die Lesbarkeit/Wartbarkeit erschwert. (Die Teilabschnitte solltest du zu mindestens noch mal in Methoden, wenn nicht sogar Klassen auslagern. Google mal nach SingelResponsibel Prinzip. Bei der Gelegenkeit KISS, DRY und YAGNI können auch nicht schaden)

Du verwendest sehr of static, ob wohl es nicht wirklich nötig ist. Es bietet sich hier teils an eigenen Klassen/Objekte zu erzeugen.

Dispose muss mit Finalizer implementiert werden und sollte dazu da sein Ressourcen freizugeben. Das Löschen von Daten sollte nicht im Dispose gemacht werden, hier ist eine Eigenen Methode besser.

In der Laod_Methodes Klasse greift du auf die Console zu, das ist nicht so schon da es einer Klasse "egal" sein sollte ob sie von einen Consolen oder Forms Anwendung aufgerufen wird.

In einen Catch-Block erzeugst du einen Neue Exception, da kannst du besser den Block weglassen.

Die Methode loadtxt gibt, einen StreamReader zurück was nicht so schon ist. Da er Dispose Implementiert und es schöner ist wenn einen Klasse die ein Objekt erzeugt es auch Disposed.

Das sind so die Punkte die mir beim Überfliegen aufgefallen sind.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

F
10.010 Beiträge seit 2004
vor 8 Jahren

Dispose muss mit Finalizer implementiert werden

Das ist nicht richtig.
Finaliser sollten nach Möglichkeit überhaupt nicht benutzt werden, es sei denn es werden unmanaged Resourcen benutzt die nicht durch das beenden des Programs freigegeben werden.

Bei allem anderen ( auch bei Files ) ist es nicht nötig, sogar schlecht ( performance,speicher) den funaliser zu implementieren.

P
1.090 Beiträge seit 2011
vor 8 Jahren

MSDN:Implementieren der Methoden "Finalize" und "Dispose" zum Bereinigen von nicht verwalteten Ressourcen

Beachten Sie, dass Sie auch bei Bereitstellung einer expliziten Steuerung mithilfe von Dispose eine implizite Bereinigung mithilfe der Finalize-Methode bereitstellen müssen. Finalize bietet eine Sicherung, um permanente Ressourcenverluste zu verhindern, falls der Programmierer Dispose nicht aufruft.

Ich kann natürlich die Dispose Schnittstelle in jeder Klasse Implementieren, auch wenn Sie dort nicht nötig ist. Dann ist ein Finalizer natürlich nicht nötig. Grundlegend gehören aber Dispose und Finilizer zusammen, damit im zweifel der GC das Aufräumen erledigen kann.

Dispose ohne Finalizer, deutet meist darauf hin, das der Finilizer vergessen wurde zu Implementiern (es gibt erstaunlich viele Leute, die nicht Wissen, dass sie ihn Implementieren müssen) oder Dispose an der Stelle eigendlich Unnötig ist. Dazu gibt es natürlich Ausnahmen.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

16.806 Beiträge seit 2008
vor 8 Jahren

Das MSDN Beispiel ist schon ein Allroundttalent, vor allem dann mit unverwalteten Ressourcen.
Man kann es einfacher machen; ich implementiere es aber auch immer mit Finalizer.

Das Löschen von Dateien gehört aber definitiv nicht in Dispose.

F
10.010 Beiträge seit 2004
vor 8 Jahren

IDisposable wird zu 90% implementiert um eine deterministische Laufzeit eines Objektes zu haben.
Die meisten Leute wissen nicht das es in den Fällen meist vollkommen unnötig und kontraproduktiv ist den Finalizer zu implementieren.

Nur wenn man eine unverwaltete Resource hat, die nicht bei Programmende freigegeben wird, muss man den Finalizer implementieren, ansonsten macht es Windows selber.
Und was es für das Laufzeitverhalten und den GC bedeutet wenn unnötig viele Finalizer vorhanden sind ist auch nachzulesen.

Siehe auch http://stackoverflow.com/questions/898828/finalize-dispose-pattern-in-c-sharp

3.003 Beiträge seit 2006
vor 8 Jahren
  • deine Benennung von lokalen Variablen, Membervariablen, Klassen, Methoden, Parametern etc. entspricht fast nie den empfohlenen Schreibweisen für C#. Unterstriche insbesondere haben mMn in Namen nichts zu suchen.
    Das mag kleinlich erscheinen, aber es macht anderen Programmierern, die deinen Code verstehen wollen, das Leben unnötig schwer. Ungefähr aus demselben Grund, wieso man sich bei Schriftsprache auf eine Rechtschreibung geeinigt hat. Apropos: dein Englisch hat, wenn ich mich nicht verguckt habe, in jedem einzelnen Satz mindestens einen Fehler - dann bleib bei Benutzerinteraktionen lieber deutsch.

  • Deklarationen von Membervariablen macht man nicht, wenn einem gerade einfällt, dass man eine braucht, irgendwo im Code. Es gibt Konventionen und Geschmäcker, was das betrifft, aber zum Beispiel kann man so Ordnung in seine Klassen bringen:


using xyz;

class MyClass
{
     #region Member
     private int _myNumber;
     #endregion
 
     #region Konstruktoren
     public MyClass(int number)
     {
          _myNumber = number;
     }
     #endregion

     #region Properties
     public string MyName { get; set; }
     #endregion

     #region Methods
     public virtual string EchoNumber() 
     {
          return _myNumber.ToString();
      }
     #endregion
}

  • einander ausschließende logische Bedingungen werden mit if(condition) else ausgedrückt, nicht mit if(condition) else if (!condition)

  • noch einmal if-else: wenn man eine leere Anweisung im if hat, macht man was verkehrt. (Ich beziehe mich auf die Methode Parse_string_to_int)

  • noch einmal if-else: wenn im if-Zweig ein break oder return aufgerufen wird, dann kann und sollte man die else-Anweisung weglassen und den alternativ auszuführenden Code direkt hinschreiben.

  • noch einmal if-else: wie schon erwähnt wurde, ist deine Verschachtelungstiefe generell zu hoch. Das macht den Code unlesbar und unwartbar.

  • Objekte, die IDisposable implementieren (ich rede nicht von deinen eigenen, sondern zB von StreamReader), sollten immer innerhalb von usings benutzt werden. Wurde schon erwähnt.

  • Variablen, die direkt bei ihrer Deklaration instanziert werden, sollten implizit mit var deklariert werden.

  • Variablen eines Werttyps müssen nur initialisiert werden, wenn der Initialwert nicht der Standardwert ist. bool myVar = false; ist unnötig.

  • unnötig sind auch Vergleiche mit bool: if(condition == true) -> if(condition)

  • switch-Anweisungen sollten ein default enthalten, wenn die case-Anweisungen nicht alle Möglichkeiten abdecken.

  • Wenn im catch lediglich ein throw steht, dann ist der ganze try-catch-Block unötig (wurde schon erwähnt).

  • wenn eine Ausnahme abgefangen wurde, die die korrekte Ausführung des Programms verhindert (zB wenn man kein tmp-Verzeichnis anlegen kann, das man aber braucht), dann MUSS das Programm kontrolliert beendet werden. Stattdessen schluckst du den Fehler weg und machst weiter, bis es unvermeidlich kracht.

Lesbarkeit aus allen diesen Gründen maximal 2/10.

Objektorientierung: ich sehe keine. Was ich sehe, sind einige Methoden, die du statisch ausgelagert hat. Statisch bedeutet: keine Instanzierung der Klasse. Das bedeutet: es gibt keine Objekte von diesem Typ, also auch keine Objektorientierung.

Ausnahme bildet Directory_System, die bei näherem Hinschauen lediglich dafür sorgt, dass ein Verzeichnis angelegt wird, falls es nicht existiert. Mit guten Willen kann man das als Urahn einer DAL sehen. Schade ist, dass du den prinzipiell nicht falschen Gedanken dadurch versaust, dass du IDisposable missbrauchst (ich vermute, du hast da was missverstanden). Wurde ja schon erwähnt.

Ein objektorientiertes Programm würde seine Abläufe durch die Interaktion verschiedener Objekte, die jeweils gewisse Zuständigkeiten haben (single responsibility wurde schon erwähnt) umsetzen. Dein Programm hingegen benutzt einen rein linearen Ablauf und lagert lediglich einige Methoden aus. Dazu kommt das mangelhafte Verständnis von Kontrollstrukturen (if-else, s.o.).

Nimm das bitte als Kritik an deinem Code, nicht an dir 😉. Wir haben alle mal angefangen, und ich habe schon schlechtere erste Versuche gesehen. Wenn dein Ziel war, objektorientiert zu arbeiten, dann hast du dieses Mal daneben geschossen.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

P
1.090 Beiträge seit 2011
vor 8 Jahren

IDisposable wird zu 90% implementiert um eine deterministische Laufzeit eines Objektes zu haben.

Dispose wird Implementiert, damit Ressourcen freigegeben werden. Mit einem deterministischen Laufzeitverhalten hatte es nichts zu schaffen. Da kannst du das Objekt auch einfach NULL setzen oder in einer Methode verwenden.

p.s.
Vielleicht verstehe, ich dich da auch falsch was du mit "deterministische Laufzeit eines Objektes zu haben" meinst. Wäre dann nett wenn du es erläutern würdest.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Hallo,
ich werde das komplette Projekt nochmal komplett neu beginnen. Dafür brauche ich aber noch ein paar Inforamtionen:
-In Main ist static unnütz sehe ich ein.
-Aber in Load_Method und Parser_Methods sehe ich keinen Sinn von nicht static.. Die bereitgestellten Methoden funktionieren ohne oder mit static fast gleich. Ich könnte mir aber z.B. vorstellen hier einen ein string Feld mit dem Programm-Ordner Pfad zu erstellen, aber lohnt es sich?
Ich kann eurer Diskussion über Dispose bzw. den Finalizer nicht ganz folgen...:
->Der Destruktor wird doch in die Finalize Methode umgewandelt.In dieser sollen Fremdresourcen freigegeben werden?
->Was verstehe ich unter Fremdresourcen? Bisher verstehe ich auch ein File als Fremdresource.
->Die Dispose Methode ist da um dieses Ereigniss auslösen zu können. Was soll ich hier freigeben bzw. wann lohnt sich die Implementierung.

-Also für meine Directory Klasse ist es besser wenn ich einfach eine Delete Methode bereitstelle?
-Soll ich nun die Load_Methods Klasse auflösen und einfach alles in Main() implementieren, oder diese belassen.
-Ich kann mir die Umsertzung nicht vorstellen:

Die Methode loadtxt gibt, einen StreamReader zurück was nicht so schon ist. Da er Dispose Implementiert und es schöner ist wenn einen Klasse die ein Objekt erzeugt es auch Disposed.

-Das Englisch werde ich nochmal anschauen...
-Was von welcher Methode soll ich aufteileen?

LG
LG

W
955 Beiträge seit 2010
vor 8 Jahren

Dispose wird Implementiert, damit Ressourcen freigegeben werden. Mit einem deterministischen Laufzeitverhalten hatte es nichts zu schaffen. Da kannst du das Objekt auch einfach NULL setzen oder in einer Methode verwenden. Stell dir vor du verwendest eine Datenbankverbindung. Dein Connection-Objekt braucht vllt nicht viel Speicher aber der Server-Process der DB könnte vllt 50MB an RAM benötigen (z.B. Sortierspeicher) Mag sein dass der Client wenig Ressourcen verbraucht der Serverprozess jedoch nicht. Deshalb möchtest du möglichst schnell nach der Beendigung der Arbeit die Verbindung schliessen damit die Serverressourcen frei werden, also nicht warten bis der GC irgendwann zuschlägt. Ein Dispose() ist besser als Close() da wenn es korrekt verwendet wird auch bei Exceptions geschlossen wird wenn es nicht mehr benötigt wird. Mit deterministisch ist also das bewusste Festsetzen des Zeitpunktes (also deterministisch) gemeint wann die Ressource wieder freiggegeben werden soll

P
1.090 Beiträge seit 2011
vor 8 Jahren

Hallo R3turnz,

grundlegend hast du ja ein paar Links bekommen und hinweise wo nach du mal googlen solltest. Das hilft dir zu einem besseren Verständnis und wir brauchen hier im Forum ja nicht nicht, das wiedergeben was andere schon ausführlich ins Netz gestellt haben. 😉

Was static angeht. Es ist einer Erweiterung der OOP und da du nicht wirklich auf einen Objekt (Instanz) Arbeitest, ist es schöner eine Objekt zu benutzen. Außer dem ist es bei Bedarf einfacher von einer Klassen Methode zu einer Statischen zu kommen (Wenn es überhaupt geht) als umgekehrt.

Mit Dispose hast du oben zur MSDN einen Link, einfach mal in Ruhe durchlesen.

Besser eine Delete Methode als, Dispose verwenden. 😉

Die Load_Methods Klasse sollte nicht aufgelöst werden. Sie sollte dir nur nicht den StreamReader zurückgeben. Sie sollte dir ein Objekt der Daten wider geben. Damit bist du nicht mehr so abhängig von der Datenquelle. (Erklärt hoffentlich ein bisschen das "Zitat").

Was du wie von welcher Methode auslagern sollst, würde ein bisschen den Umfang sprengen. Schau einfach erst mal das deine Methoden nicht zu Lang sind. Wenn nach einer IF Bedingung mehrere Zeilen Code kommen. Ist es ein guter Punkt, diese in eine Klasse/Methode auszulagern.
Vieleicht mal an ein Buch denken (hab ich von Code Complied). Du hast den Namen des Buches, der sagt worum es geht, dann hast du ein Inhaltsverzeichnis mit den einzelnen Kapiteln, die vielleicht noch in Unterkapitel aufgeteilt sind. Und da Passiert erst das eigendlich wichtige.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Sorry, aber ich verstehe immer noch nicht was du mit Objekt von Daten zürückgeben meinst. Soll ich den Streamreader als Eigenschaft bereitstellen, und dann zum schließen einfach die Close Methode der Eigenschaft aufrufen 😉

LG

P
1.090 Beiträge seit 2011
vor 8 Jahren

In der Datei Stehen doch Daten in welcher Form auch immer (wenn ich es richtig mitbekommen habe eine Liste von Passwörtern, wäre dann als Rückgabe (i)List<string>). Konvertiere an der Stelle die Daten in ein passendes Format und gibt die Geladenen Daten als Objekte zurück. (Könnte Kunde, User, Artikel oder so sein).

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

4.931 Beiträge seit 2008
vor 8 Jahren

Hallo,

gemeint ist, daß der StreamReader nur ein internes Detail deiner Klasse sein sollte, und du stattdessen Datenobjekte zurückgeben solltest (z.B. eine Liste von Passwörtern)
Eine mögliche Methode könnte m.E. folgende sein:


public List<string> GetPasswordList(string filename)

P
1.090 Beiträge seit 2011
vor 8 Jahren

Stell dir vor du verwendest eine Datenbankverbindung. Dein Connection-Objekt braucht vllt nicht viel Speicher aber der Server-Process der DB könnte vllt 50MB an RAM benötigen (z.B. Sortierspeicher) Mag sein dass der Client wenig Ressourcen verbraucht der Serverprozess jedoch nicht. Deshalb möchtest du möglichst schnell nach der Beendigung der Arbeit die Verbindung schliessen damit die Serverressourcen frei werden, also nicht warten bis der GC irgendwann zuschlägt. Ein Dispose() ist besser als Close() da wenn es korrekt verwendet wird auch bei Exceptions geschlossen wird wenn es nicht mehr benötigt wird. Mit deterministisch ist also das bewusste Festsetzen des Zeitpunktes (also deterministisch) gemeint wann die Ressource wieder freiggegeben werden soll

Das Funktioniert nur wenn du ein Using verwendest, da ein Dispose ja nicht bei einer Exception ausgelöst wird. Und was machst du wenn das Using vergessen wurde? Finilizer implementieren, damit im zweifel der GC aufräumen kann.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

F
10.010 Beiträge seit 2004
vor 8 Jahren

Das Funktioniert nur wenn du ein Using verwendest, da ein Dispose ja nicht bei einer Exception ausgelöst wird. Und was machst du wenn das Using vergessen wurde? Finilizer implementieren, damit im zweifel der GC aufräumen kann.

Nein, denn hier hilft weder der Finalizer noch macht er irgendeinen Sinn, denn er ist eben nicht Determinsitisch.
Das einzige was hilft, ist dem Kollegen der das vergessen hat auf die Finger zu hauen.

P
1.090 Beiträge seit 2011
vor 8 Jahren

Nein, denn hier hilft weder der Finalizer noch macht er irgendeinen Sinn, denn er ist eben nicht Determinsitisch.
Das einzige was hilft, ist dem Kollegen der das vergessen hat auf die Finger zu hauen.

Klar hilft der Finilizer auf witte's Antwort bezogen, da er im Zweifel die Ressourcen weg räumt. Den Kollegen auf die Finger zu hauen, der kein using benutzt kann aber nicht schaden. 😉

Ein Deterministisches verhalten, bekommst du nur für verwaltete Ressourcen oder Klassen hin, die diese Benutzen. Ansonsten sorgt der GC fürs aufräumen. Und wann der Läuft, kannst du nicht bestimmen. (Außer man ruft GC.Collect auf, was so manche andere Nachteile hat.)

Grundlegend schweifen wir hier vom eigentlichen Thema ab. Wir können von mir aus gerne die Diskussion an einer anderen Stelle vorführen.

Ich würde aber gerne an den Punkt, festhalten das es besser ist Dispose mit Finalizer zu Implementieren als ohne.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Es geht um die Implementierung der Klassen im Zusammenhang mit Simple Responsibility Principle.
Der Grundstatz ist so wie ich es verstanden habe: Jede Klasse hat eine Aufgabe.

Ich habe nun die Programmstruktur mit diesem Grundsatz durchdacht:
Das Program soll die möglichkeit haben die Liste in den Arbeitsspeicher zu laden. Das ist für mich eine Aufgabe. Der Rückgabewert dieser Methode wäre ein Stream-Reader. Oben wurde aber gesagt man sollte nur Daten, also z.B. eine Liste zurückgeben.Um dies umzusetzen müsste ich einfach in der Klasse den Stream Reader als Feld bereitstellen und dann die Methode GetPasswords implemntieren, die die Passwörter ausliest. Das Auslesen wäre für mich aber schon eine andere Aufgabe, die dann ja schon wieder in eine andere Klasse gehört. Wie soll ich das lösen?

LG

P
1.090 Beiträge seit 2011
vor 8 Jahren

@R3turnz,

ich konnte deine Frage jetzt nicht direkt verstehen.

Mach doch mal die Vorgeschlagenen Änderungen an deinem Quellcode und poste sie (vielleicht mit Zip als Anhang)

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Habe den Beitrag editiert 🙂

T
2.219 Beiträge seit 2008
vor 8 Jahren

@R3turnz
Ich würde dir empfehlen dem Code zu verwerfen und von vorn anzufangen.
Dann kannst du den Code einmal im richtigen Format schreiben, wie es in C# von den Konventionen her richtig ist.
Ebenfalls solltest du deinen StreamReader nicht durch deinen Code hangeln oder zwischen Speichern.
Auch deine Klassen solltest du auf ein Minimum zusammen schrumpfen.

Auch mit static und anderen Grundlagen solltest du dich nochmal einlesen und verstehen was der Sinn und Zweck davon ist.
static ist bei Main sogar eine Pflicht!

Ansonsten solltest du deinen Code kapseln um keine Konstrukte zu haben, die nur Teilfunktionen für anderen Komponenten/Methoden bereitstellen.
Im Normalfall könnte man dein Projekt mit der Program und einer Parser Klasse lösen.
Du könntest dann noch eine Klasse bereitstellen, die eine Repräsention deiner Daten in der Datei wiederspiegeln.
Aber mehr als drei Klassen sollten es nicht sein.

@Palin/FZelle
Ich denke es ist nicht wirklich förderlich für das Thema auch mit dem Finalizer hier auszulassen.
Das gehört hier nur untergeordnet zum Thema und ob/wann ein Finalizer sinnvoll ist kann man im Netz nachlesen.
Da bringt es auch nichts sich hier endlos die Köpfe einzuhauen.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Ich habe jetzt eine Version entwickelt, in der wie T-Virus meinte nur Program und eine Parser Klasse existieren. Sie ist überhaupt nicht getestet, noch habe bisher auf richtiges Englisch geachtet.


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
using System.IO.Compression;
namespace Password_Checker_Rework
{
    delegate string[] LoadMethode(string sourcepath);
    class Program
    {
        string applicationdirectory = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "passwordchecker");

        static void Main(string[] args)
        {
            Program program = new Program();
            #region ProgrammFolderCreation
            DirectoryInfo programmfolderinfo = new DirectoryInfo(program.applicationdirectory);

            if (!programmfolderinfo.Exists)
            {
                programmfolderinfo.Create();
                Console.WriteLine("Created Program-Folder.");
            }
            else if (programmfolderinfo.Exists)
            {
                Console.WriteLine("Loaded Program-Folder.");
            }

            #endregion
            Console.Write("Filepath to sourcelist:");
            string sourcepath = Console.ReadLine();
            if (File.Exists(sourcepath))
            {
                FileInfo sourcefileinfo = new FileInfo(sourcepath);

                Console.WriteLine("Detecting file-type...");
                LoadMethode LoadmethodeHandler = null;
                if (sourcefileinfo.Extension == ".zip")
                {
                    LoadmethodeHandler += program.LoadPasswordsfromzip;
                }

                else if (sourcefileinfo.Extension == ".txt")
                {
                    LoadmethodeHandler += program.LoadPasswordsfromtxt;
                }
                else
                {
                    Console.WriteLine("Your data-type is not supported!");
                }
                if (LoadmethodeHandler != null)
                {
                    string[] passwords = LoadmethodeHandler(sourcefileinfo.FullName);
                    if(passwords != null)
                    {
                        Console.Write("Your Password:");
                        string password = Console.ReadLine();

                        bool success = program.CheckPasswords(passwords, password);
                        if(success)
                        {
                            Console.WriteLine("Founded your password!");

                        }
                        else
                        {
                            Console.WriteLine("Did not find your password!");
                        }
                    }
                }

            }
            else
            {
                Console.WriteLine("Your file does not exist.");
            }
            Console.WriteLine("Cleaning up programm data...");
            Directory.Delete(programmfolderinfo.FullName, true);
            Console.ReadKey();
        }

        private string[] LoadPasswordsfromtxt(string sourcepath)
        {
            Console.WriteLine("Opening .txt file...");
            StreamReader reader = new StreamReader(sourcepath);
            Console.WriteLine("Reading Passwords...");
            string[] output = ReadPasswords(reader);
            return output;
        }

        private string[] LoadPasswordsfromzip(string sourcepath)
        {
            string[] output = null;
            FileStream fs = new FileStream(sourcepath, FileMode.Open);
            System.IO.Compression.ZipArchive archive = new ZipArchive(fs);
            for (int i = 0; i < archive.Entries.Count - 1; i++)
            {
                Console.WriteLine(i + "." + archive.Entries[i].Name);
            }
            Console.Write("Choose:");
            string choosenentry = Console.ReadLine();
            Parser parser = new Parser();
            int choosenentryint = parser.Parsestringtofile(choosenentry, 0, archive.Entries.Count - 1);
            if (choosenentryint != -1)
            {
                ZipArchiveEntry entry = archive.GetEntry(archive.Entries[choosenentryint].FullName);
                Console.WriteLine("Extracting to temp-folder...");
                string extractedfilepath = Path.Combine(applicationdirectory, "passwords.txt");
                entry.ExtractToFile(extractedfilepath);
                Console.WriteLine("Opening file...");

                output = LoadPasswordsfromtxt(extractedfilepath);
            }
            else
            {
                Console.WriteLine("Could not parse your input.");
            }
            return output;
        }
        private string[] ReadPasswords(StreamReader reader)
        {
            List<string> passwords = new List<string>();
            while (reader.Peek() != -1)
            {
                passwords.Add(reader.ReadLine());
            }
            return passwords.ToArray();

        }
        private bool CheckPasswords(string[] passwords,string password)
        {
            Console.WriteLine("Press any key to start reading...");
            Console.ReadKey();
            Console.WriteLine("{0,-20}{1}","PASSWORD", "ACCORDANCE");
            bool success = false;
            for(int i = 0;i <= passwords.Length - 1;i++)
            {
                if(passwords[i] == password)
                {
                    success = true;
                    Console.WriteLine("{0,-20}{1}",passwords[i],"TRUE");
                    break;
                }
                else
                {
                    Console.WriteLine("{0,-20}{1}",passwords[i],"FALSE");
                }
            }
            return success;
        }

    }
}

Parser.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework
{
    class Parser
    {
public int Parsestringtofile(string value,int min,int max)
        {
            int output;
            bool parsesuccess = int.TryParse(value, out output);
            if(parsesuccess)
            {
                if(output <= max && output >= min)
                {
                    return output;
                }
                else
                {
                    return -1;
                }
            }
else
            {
                return -1;
            }

        }


    }
}


Ist mir diese Version besser gelungen?
Wo wäre eine Klasse jetzt noch sinnvoll zu implementieren? Für den Program-Ordner?

LG

16.806 Beiträge seit 2008
vor 8 Jahren

Naja, da ist schon ziemlich viel, was man anders machen kann.
zB kann man den Inhalt ganz einfach mit File.ReadAllLines() einlesen und kann somit ~10 Zeilen deiner fehlerhaften Stream-Funktionalität (du verwirfst nirgends den Stream, Stichwort usings)einsparen.

Ja und dann ziemlich viel strukturell, was man anders löst.

  • Viele Codestellen kann man effizienter und kürzer umsetzen (wie eben ReadAllLines.. und ähnliches)
  • Aufbau des Codes nach Verantwortlichkeiten
  • Lösen der harten Kopplung und Verwenden von IoC.

Aber da sind wir bei Themen, die eben deutlich über die erste Schulstunde hinaus gehen... =)

Als Beispiel

        if (!programmfolderinfo.Exists)
            {
                programmfolderinfo.Create();
                Console.WriteLine("Created Program-Folder.");
            }
            else if (programmfolderinfo.Exists)
            {
                Console.WriteLine("Loaded Program-Folder.");
            }

geht logisch korrekter und übersichtlicher via

         if (programmfolderinfo.Exists)
            {
                Console.WriteLine("Loaded Program-Folder.");
            }
            else
            {
                programmfolderinfo.Create();
                Console.WriteLine("Created Program-Folder.");
            }
2.207 Beiträge seit 2011
vor 8 Jahren

Hallo R3turnz,

wieso genau machst du "programmfolderinfo"? Das erstellst du und räumst es danach wieder ab. Oder überseh ich was? 🤔 Egal.

Die Klasse "Parser" kann man auch einfacher schreiben:

class Parser
{
    public int ParseStringToFile(string value,int min,int max)
    {
        int output;
        bool parsesuccess = int.TryParse(value, out output);
        if(parsesuccess && output <= max && output >= min)
        {
            return output;
        }
        return -1;
    }
}

Aus dem Methodennamen "ParseStringToFile" kann ich nichts ableiten, schon gar nicht den Rückgabetyp "int". "IrgendwasToFile" suggeriert mir eher, dass die Methode was in ein File persistiert. Dann macht aber wieder "min" und Max" keinen Sinn. Die Methode gibt dir eher einen Index zurück. Und du willst prüfen, ob erstens: Der "value" eine Zahl ist, zweitens: Ob sie grösser als min ist und drittens: Ob sie kleiner als Max ist. Somit wäre das kein Parser, sondern ein Validator oder ähnlich. Der prüft, ob die Eingabe nach deinen Kriterien valide ist.

Etwa so (Das ist nur eine von tausend Möglichkeiten):

class Validator
{
    public int InputIndexIsValid(string value, int arrayLength)
    {
        int output;
        if(!Int.TryParse(value, out output))
        {
            return false;
        }
        if(output <= arrayLength - 1 && output >= 0)
        {
            return false;
        }
        return true;
    }
}

(Selbst das geht noch kürzer 😉 ) Aber das kannst du a) gut debuggen und b) gut testen.

Mit einem bool als Rückgabewert kannst du dann fragen:

Validator validator = new Validator();
if (!validator.InputIndexIsValid(choosenentry, archive.Entries.Count))
{
    Console.WriteLine("Could not parse your input.");
    return null;
}

int index;
Int.TryParse(value, out index)
ZipArchiveEntry entry = archive.GetEntry(archive.Entries[index].FullName);
//...

Alles jetzt mal aus der Hüfte geschossen und ungetestet. Ich will nur deutlich machen: Benenn die Variablen/Methoden etc. richtig. Mach es dir so einfach wie möglich deinen Code morgen auch noch zu verstehen.

Auch was du mit dem delegate willst, verstehe ich nicht so ganz. Schreib dir einfach eine Methode, die dir eine Liste der Passwörter gibt.

Etwa so (wieder eine von tausend Möglichkeiten):


string sourcePath = Console.ReadLine();

if (File.Exists(sourcePath))
{
	List<string> passwords = GetPasswords(sourcePath);
}
private List<string> GetPasswords(string sourcePath)
{
	FileInfo sourcefileinfo = new FileInfo(sourcePath);

	Console.WriteLine("Detecting file-type...");

	if (FileIsZip(sourcefileinfo))
	{
		return LoadPasswordsfromzip(...);
	}

	if (FileIsTxt(sourcefileinfo))
	{
		return LoadPasswordsfromtxt(...);
	}

	Console.WriteLine("Your data-type is not supported!");
	return new List<string>();
}

private bool FileIsZip(FileInfo sourcefileinfo)
{
	return sourcefileinfo.Extension == ".zip";
}

private bool FileIsTxt(FileInfo sourcefileinfo)
{
	return sourcefileinfo.Extension == ".txt";
}

Gruss

Coffeebean

3.003 Beiträge seit 2006
vor 8 Jahren

Sie ist überhaupt nicht getestet, noch habe bisher auf richtiges Englisch geachtet.
Ist mir diese Version besser gelungen?

😁 Das war nur, weil's mir beim Schreiben gerade auffiel.

Wo wäre eine Klasse jetzt noch sinnvoll zu implementieren? Für den Program-Ordner?

Nein. Vielleicht hilft dir ein Diagramm weiter. Oben die beiden bilden die Struktur ab, so wie sie jetzt bei dir ist. Unten ein Vorschlag von mir, wie du das Laden und Prüfen der Passwörter entkoppeln kannst (was ist, wenn du Passwörter aus einer DB laden musst? Oder per FTP? Oder per Service? Schreibst du jedesmal in dein Hauptprogramm eine neue Methode?)

Welche Klassen du brauchst, siehst du häufig, wenn du dir Szenarien zur Erweiterung deines Programms vorstellst, und welche Änderungen nötig wären.

Noch eines. Was mir generell an deinem Code aufstösst, ist die Tatsache, dass du alles in die Main() klatschst. Die sollte eigentlich so (oder ähnlich) aussehen:


//vollständiges Listing des Einstiegspunkts der Konsolenanwendung
class Program
{
   static void Main(string[] args)
   {
        var appDir = (args != null && !string.IsNullOrEmpty(args[0])) ? args[0] : "defaultAppDir";

         var myClient = new PasswordCheckClient();
         myClient.Process(appDir);
   }
}

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

16.806 Beiträge seit 2008
vor 8 Jahren
private bool FileIsZip(FileInfo sourcefileinfo)
{
    return sourcefileinfo.Extension == ".zip";
}

Wenn ich die Datei *.Zip nenne, dann ist sie immer noch eine ZIP-Datei; aber Dein Programm erkennt es nicht mehr.

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Vielen Dank für die Antworten, vorallem für die Grafik von LaTino... Ich habe aber noch drei Fragen dazu:
-In Proccess ist der Ablauf und der User-Input?
-Wie soll ich den applictationdirectory-pfad in die ZipFilePasswordLoader bekommen?
-Wenn ich mit dem Interface eine Klasse zum Laden einer Datenband bereitstelllen möchte brauche ich kein path parameter, sonder andere.Die Frage ist mehr theoretisch, aber wie würde ich soetwas umsetzten?

LG und nochmal vielen Dank...

3.003 Beiträge seit 2006
vor 8 Jahren

Vielen Dank für die Antworten, vorallem für die Grafik von LaTino... Ich habe aber noch drei Fragen dazu:
-In Proccess ist der Ablauf und der User-Input?

Richtig, sowie die Ausgaben. Möchte man die Interaktion mit dem User (den View, sozusagen) später auslagern, würde man dann dort ansetzen.

-Wie soll ich den applictationdirectory-pfad in die ZipFilePasswordLoader bekommen?

So, wie Objekte miteinander kommunizieren. Du kannst im Konstruktor eine private Membervariable setzen. Du kannst ZipFilePasswordLoader eine öffentliche Property geben und diese setzen. Du kannst ein Event benutzen, das gefeuert wird, bevor es ans eigentliche Laden geht, du kannst...ach, eigentlich kannst du alles mögliche machen 😉.

-Wenn ich mit dem Interface eine Klasse zum Laden einer Datenband bereitstelllen möchte brauche ich kein path parameter, sonder andere.Die Frage ist mehr theoretisch, aber wie würde ich soetwas umsetzten?

Richtig erkannt. Ich würde (vermutlich) eine Factory bemühen, also eine zusätzliche Klasse, der ich ein Konfigurationsobjekt übergebe (zB den Dateinamen, oder für die Datenbank notwendige Informationen) und die anhand der Struktur dieses Objekts erkennt, was für einen Loader sie liefern muss, und den erstellt, konfiguriert und zurückgibt. Im jetzigen Anwendungsfall würde man ihr den sourcepath rüberschieben, sie würde schauen, was für ein Dateityp das ist (am besten NICHT anhand der Endung, wie Abt oben schon zu Recht geschrieben hat), und dann einen ZipLoader oder einen TextLoader liefern, der schon auf die Datei zeigt, die er lesen soll.

Dein Projekt ist relativ simpel, und für Übungszwecke kann man solche Sachen machen, aber im realen Leben würde es jetzt schon sehr an der Grenze zum Overengineering herumschlittern 😉. Komplexität und Wartbarkeit immer im Blick behalten.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

16.806 Beiträge seit 2008
vor 8 Jahren

Die Endung ist in 99,9% der Fälle als erster Kriterium schon OK - aber die Groß-/Kleinschreibung darf kein Rolle spielen.

3.003 Beiträge seit 2006
vor 8 Jahren

Ist richtig, und unter Windows sicher noch deutlich über 99,9%. Den Typ nicht am Namen festzumachen ist eine Angewohnheit aus der Webentwicklung (PHP), wo man sich niemals auf Dateiendungen verlassen konnte. Spielt hier wirklich keine Rolle, das stimmt.

LaTino
(ist man so paranoid wie ich, würde man's über SHGetFileInfo aus der registry lesen)

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

16.806 Beiträge seit 2008
vor 8 Jahren

(ist man so paranoid wie ich, würde man's über
>
aus der registry lesen)

Was mit DNX nicht mehr funktionieren wird 😃

3.003 Beiträge seit 2006
vor 8 Jahren

Argl, das erinnert mich...kommt mit in unser Migrationsticket, da fehlt es nämlich noch, wenn ich mich nicht irre. Soll noch einer sagen, die OT-Kommentare seien sinnlos^^

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Ich hoffe das wird jetzt der letzte Versuch, dieses Program hinzubekommen.
Program.cs


    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Text;
    using System.Threading.Tasks;
    using System.IO;
    namespace Password_Checker_Rework_2
    {
        class Program
        {
            static void Main(string[] args)
            {
                var appDir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData),"passwordchecker");
                var passwordcheckerclient = new PasswordCheckClient();
                passwordcheckerclient.Proccess(appDir);
            }
        }
    }


PasswordCheckerClient.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
namespace Password_Checker_Rework_2
{
    class PasswordCheckClient
    {
        private string passwordfilepath;

        public string Passwordfilepath
        {
            get { return passwordfilepath; }
            set { passwordfilepath = value; }
        }
        private PasswordChecker passwordchecker;
        public void Proccess(string applicationdirectory)
        {
            if(Directory.Exists(applicationdirectory))
            {
                Console.WriteLine("Loaded Programm-Directory...");
            }
            else if(!Directory.Exists(applicationdirectory))
            {
                Console.Write("Creating Program-Directory...");
                Directory.CreateDirectory(applicationdirectory);
                Console.WriteLine("Done...");
            }
            Console.Write("PasswordChecker\nPath:");
            Passwordfilepath = Console.ReadLine();

            

           
            FileInfo sourcefileinfo = new FileInfo(Passwordfilepath);
            if(sourcefileinfo.Exists)
            {
                Console.Write("Your password:");
                string password = Console.ReadLine();
               
                PasswordChecker checker = new PasswordChecker(password,Passwordfilepath);
                if (FileisZip(sourcefileinfo))
                {
                    ZipPasswordLoader loader = new ZipPasswordLoader(applicationdirectory);
                    checker.CheckPasswords(loader);
                }
                else if (FileisText(sourcefileinfo))
                {
                    TextPasswordLoader loader = new TextPasswordLoader();
                    checker.CheckPasswords(loader);
                }
                else
                {
                    Console.WriteLine("Your format is not supported!");
                }

            }
         else
            {
                Console.WriteLine("Yout file does not exists!");
            }
            Console.WriteLine("Cleaning up application directory...");
            Directory.Delete(applicationdirectory, true);
            Console.ReadKey();
                   
        }
        private bool FileisZip(FileInfo sourcefileinfo)
        {
            return sourcefileinfo.Extension.ToUpper() == ".ZIP";
        }
        private bool FileisText(FileInfo sourcefileinfo)
        {
            return sourcefileinfo.Extension.ToUpper() == ".TXT";
        }
    }
}


PasswordChecker.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework_2
{
    class PasswordChecker
    {

        private string passwordtocheck;

        public string Passwordtocheck
        {
            get { return passwordtocheck; }
            set { passwordtocheck = value; }
        }
        private string sourcepath;

        public string Sourcepath
        {
            get { return sourcepath; }
            set { sourcepath = value; }
        }

        public PasswordChecker(string passwordtocheck,string sourcepath)
        {
            this.Passwordtocheck = passwordtocheck;
            this.Sourcepath = sourcepath;
        }
        public void CheckPasswords(IPasswordLoader Loader)
        {
            Console.WriteLine("Press any key to start checking...");
            Console.ReadKey();
            string[] passwordlist;
            passwordlist = Loader.Load(Sourcepath);
            if (passwordlist != null)
            {
                bool foundedpassword = false;
                for (int i = 0; i <= passwordlist.Length - 1; i++)
                {
                    if (Passwordtocheck == passwordlist[i])
                    {
                        Console.WriteLine("{0,-20}{1}", passwordlist[i], "TRUE");
                        foundedpassword = true;
                        break;
                    }

                    else
                    {
                        Console.WriteLine("{0,-20}{1}", passwordlist[i], "FALSE");
                    }
                }
                if (foundedpassword)
                    Console.WriteLine("Founded your password! Better choose another one...");
                else
                    Console.WriteLine("Don't founded your password!");
            }
            
        }
                


    }
}


IPasswordLoader.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework_2
{
    interface IPasswordLoader
    {
        string[] Load(string sourcepath);



    }
}


TextPasswordLoader.cs:



using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
namespace Password_Checker_Rework_2
{
    class TextPasswordLoader : IPasswordLoader
    {



        public string[] Load(string sourcepath)
        {
            Console.WriteLine("Started reading....");
            List<string> output = new List<string>();
            var reader = new StreamReader(sourcepath);
            while(reader.Peek() != -1)
            {
                output.Add(reader.ReadLine());
            }
            Console.WriteLine("Finished Reading...");
            reader.Close();
            return output.ToArray();
        }
    }
}


ZipPasswordLoader.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO.Compression;
using System.IO;
namespace Password_Checker_Rework_2
{



    class ZipPasswordLoader : IPasswordLoader
    {
        private string applicationdirectory;
        public ZipPasswordLoader(string applicationdirectory)
        {
            this.applicationdirectory = applicationdirectory;
        }
        public string[] Load(string sourcepath)
        {
            List<string> output = new List<string>();
            FileStream fs = new FileStream(sourcepath, FileMode.Open);
            ZipArchive archive = new ZipArchive(fs);
            Console.WriteLine("Entries in Archive:");

            for (int i = 0;i <= archive.Entries.Count - 1;i++)
            {
                Console.WriteLine(i+"."+archive.Entries[i].Name);
            }
            string choosenentry = Console.ReadLine();
            Validator validator = new Validator();

            if(validator.InputIndexIsValid(choosenentry, archive.Entries.Count))
            {
                int validatedchoosenentry = int.Parse(choosenentry);
                ZipArchiveEntry entry = archive.Entries[validatedchoosenentry];
                Console.WriteLine("Extracting entry to application directory...");
                string extractedfilepath = Path.Combine(applicationdirectory, entry.Name);
                entry.ExtractToFile(extractedfilepath);
                Console.WriteLine("Completed!");
                StreamReader reader = new StreamReader(extractedfilepath);
                while(reader.Peek() != -1)
                {
                    output.Add(reader.ReadLine());

                }
                reader.Close();
                Console.WriteLine("Completed reading!");   
            }
            else
            {
                Console.WriteLine("Can not validate Input");
            }
            return output.ToArray();
        }
    }
}


Validator.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework_2
{
    class Validator
    {
      
           public bool InputIndexIsValid(string value, int arrayLength)
        {
            int output;
            if (!int.TryParse(value, out output))
            {
                return false;
            }
            if (output <= arrayLength - 1 && output >= 0)
            {
                return false;
            }
            return true;
        }
    


    }
}


Eigentlich wollte ich jegliche Nutzerinput in Proccess() haben. Dann würde aber das sourcepath Parameter in ZipLoader keinen Sinn machen, da ich dann einfach der Klasse über den Konstruktor das Zip Archive Entry übergeben... Was ist in diesem Fall besser?

LG

1.040 Beiträge seit 2007
vor 8 Jahren

Schaue dir bitte nochmal deine if-else-Zweige an.

if(Directory.Exists(applicationdirectory))
{
}
else if(!Directory.Exists(applicationdirectory))
{
}

Genau genommen schreibst du das:

if(Directory.Exists(applicationdirectory))
{
}
else
{
    if(!Directory.Exists(applicationdirectory))
    {
    }
}

Das zweite if ist nicht notwendig, da im else-Zweig schon die Prüfung fehlgeschlagen ist.

Evtl. kennst du es aus einer anderen Programmiersprache anders, hier wird es aber nicht benötigt. =)

3.003 Beiträge seit 2006
vor 8 Jahren

Jetzt hast du ein Interface und programmierst immer noch gegen die Implementierungen und nicht gegen das Interface. Refakturiert:


if(sourcefileinfo.Exists)
{
    Console.Write("Your password:");
    var password = Console.ReadLine();

    var checker = new PasswordChecker(password,Passwordfilepath);
    IPasswordLoader passwordLoader;

    if(FileIsZip(sourcefileinfo)) passwordLoader = new ZipPasswordLoader(applicationdirectory);
    else if(FileIsText(sourcefileinfo)) passwordLoader = new TextPasswordLoader();
    else 
    {
        passwordLoader= new NullPasswordLoader(); //kann man machen, muss man nicht. NullPasswordLoader macht einfach: gar nix.
        Console.WriteLine("Unsupported password file type.");
     }
     
     checker.Check(passwordLoader); //und hier ist der Unterschied: der Compiler kennt zu diesem Zeitpunkt nicht den konkreten Typ des passwordLoader, sondern weiß nur noch, dass der halt eine Schnittstelle implementiert, und das reicht ihm.
}

Zur eigentlichen Frage: gut, dass du die Benutzerinteraktion dort gern raushaben willst. Eine der eleganteren Methoden, das zu erreichen, wäre über ein Event.

  • Event in der Schnittstelle deklarieren
  • Feuert nur im ZipFileLoader und sagt sinngemäß "ich brauch jetzt mal Input, woher ist mir egal"
  • wird in Process() abonniert, bevor Checker.Check() aufgerufen wird

Also, wenn man den Codeschnipsel oben erweitert:


//...
    checker.InputNeeded += (sender, args) => 
    {
        var zipFileLoader = sender as ZipFileLoader;
        if(zipFileLoader == null) return;
        zipFileLoader.ChosenEntry = Console.Read();

    }
    checker.Check(loader);

//ZipFileLoader:
    public string ChosenEntry { get; set; }
    //...
    for (int i = 0;i <= archive.Entries.Count - 1;i++)
    {
        Console.WriteLine(i+"."+archive.Entries[i].Name);
    }
    if(InputNeeded != null) InputNeeded(this, null);
    //danach steht in ChosenEntry dann der Wert, den Process rüberschiebt

Ziel ist, dass in deinen Klassen außer in Process() keine Console erwähnt wird. Du könntest auch ohne Event arbeiten und die Ein- und Ausgaben per delegate auslagern an eine Methode im Client. Gibt wie gesagt einige Möglichkeiten.

LaTino
Edit: übrigens schon viel besser lesbar. Mehr camelCase einsetzen und an deinen If-Elses arbeiten, es wird besser.

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

S
417 Beiträge seit 2008
vor 8 Jahren

(ist man so paranoid wie ich, würde man's über
>>
aus der registry lesen)
Was mit DNX nicht mehr funktionieren wird 😃){gray}

Als kleine Korrektur: Es wird nur unter der .NET Core Runtime nicht mehr funktionieren. Solange man das "normale .NET" nutzt geht das auch weiterhin mit dnx (bzw. aktuell net451).

16.806 Beiträge seit 2008
vor 8 Jahren

Ja, danke fürs präzisieren 😉

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

@LaTino Das Problem mit der Event-Lösung ist, dass Password-Checker das Event garnicht besitzt.
Soll ich denn IPasswordLoader einfach in ein ZipFilePasswordLoader konvertieren? Das wäre für mich nicht so schön, bin aber wie allen schon aufgefallen ist kein Profi.. Also kann man das so lösen?

Ich bin gerade dabei alle Consoles aus den Klassen zu entfernen, dabei ist mir aufgefallen:
-Die Klassen sind nun allgemeiner, egal ob sie aus der Console oder aus einer FensterAnwendung gestartet werden..
-Das Problem ist aber, dass ich wesentlich weniger Auskunft über den Fehler bzw. das Ereigniss geben kann.. Soll ich eine Exception schmeißen, und in der Message den Grund mitgeben und bei einem Ereigniss also z.B. Das Extrahieren wurde beendet eine event feuern, bei dem dann in Proccess einfach die Ausgabe erfolgt?

LG

PS: Vielen Dank für die ganze Hilfe, hilft mir extrem weiter. Ich habe aber erst jetzt gemerkt wie wichtig Praxis ist!

16.806 Beiträge seit 2008
vor 8 Jahren

Du bist ja noch recht jung, hast also genug Zeit für Praxis.
Aber Interfaces nutzt man in der Regel direkt; man versteckt nur die genaue Implementierung, sodass man sie einfach austauschen kann.

i.d.R. leitet man Exceptions nach oben weiter und filtert nicht vorher.

3.003 Beiträge seit 2006
vor 8 Jahren

@LaTino Das Problem mit der Event-Lösung ist, dass Password-Checker das Event garnicht besitzt.

Ja, das war der Teil, wo du selbst ran musst 😉.

Soll ich denn IPasswordLoader einfach in ein ZipFilePasswordLoader konvertieren? Das wäre für mich nicht so schön, bin aber wie allen schon aufgefallen ist kein Profi.. Also kann man das so lösen?

Nein, nix konvertieren. Das ausgenutzte Prinzip nennt sich Polymorphismus und ist ein Grundprinzip der OOP. Ein anschauliches Bild dafür, dass meine Studis immer kapiert haben, ist:
Die Schnittstelle beschreibt ein Auto: dass es ein Lenkrad hat, und mit welchen Eingaben man nach links fährt oder bremst.
Die konkreten Klassen (ZipFilePasswordLoader und TextFilePasswordLoader) beschreiben einen Ford und einen Audi. Beide tun hinter den Kulissen verschiedene Dinge (der Audi zB bescheisst beim Abgastest...ah, anderes Thema), aber wenn man die Schnittstelle "Auto" kennt, kann man mit beiden fahren. Man kann sogar alle Markenzeichen abkleben und immer noch damit fahren, und genau das macht der Kompiler hier.


IMeinInterface meineVariable; //meineVariable ist für den Compiler nur eine Implementierung der Schnittstelle, welche, ist ihm egal
meineVariable = userDecision ? new Klasse1() : new Klasse2();

An der Stelle hat der Compiler zur Laufzeit hat keine Ahnung, was meineVariable eigentlich ist. Muss er auch nicht. Und so können wir ihm jederzeit, auch zur Laufzeit, das Objekt gegen ein anderes austauschen, ohne dass ihm das was ausmacht. Hauptsache, das Objekt implementiert die Schnittstelle.

Schnapp dir mal ein Buch zur OOP. Die Head First-Reihe finde ich persönlich sehr gut, hier im Forum gibt's auch noch Empfehlungen.

Ich bin gerade dabei alle Consoles aus den Klassen zu entfernen, dabei ist mir aufgefallen:
-Die Klassen sind nun allgemeiner, egal ob sie aus der Console oder aus einer FensterAnwendung gestartet werden..

...was genau das war, was mit "Entkopplung" gemeint ist. OOP soll dir ermöglichen, deinen Code häufiger wiederverwenden und einfacher warten zu können.

-Das Problem ist aber, dass ich wesentlich weniger Auskunft über den Fehler bzw. das Ereigniss geben kann.

Wie Abt schon schrieb: wirf Ausnahmen dort, wo sie entstehen. Du kannst sie in der Aufrufliste durchaus weiterreichen (buchstäblich weiterwerfen mit throw, das ist mit nach "oben" durchreichen gemeint). Debugge dein Programm mal, erzeuge absichtlich eine Ausnahme und schau dir im Visual Studio die Aufrufliste an. Dann kannst du dort bestimmen, an welchem Punkt du die Ausnahme behandeln willst (bspw. einen Logeintrag schreiben und das Programm beenden). Dafür gibt es - natürlich - auch wieder Pattern, also Arten, wie man Ausnahmen organisieren kann: eigenes Thema für sich.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Und die nächste Version 😉
Program.cs:


    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Text;
    using System.Threading.Tasks;
    using System.IO;
    namespace Password_Checker_Rework_2
    {
        class Program
        {
            static void Main(string[] args)
            {
                var appDir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData),"passwordchecker");
                var passwordcheckerclient = new PasswordCheckClient();
                passwordcheckerclient.Proccess(appDir);
            }
        }
    }

PasswordCheckerClient:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
namespace Password_Checker_Rework_2
{
    class PasswordCheckClient
    {
        private string passwordfilepath;

        public string Passwordfilepath
        {
            get { return passwordfilepath; }
            set { passwordfilepath = value; }
        }
        private PasswordChecker passwordchecker;
        public void Proccess(string applicationdirectory)
        {
            if (Directory.Exists(applicationdirectory))
            {
                Console.WriteLine("Loaded Programm-Directory...");
            }
            else if (!Directory.Exists(applicationdirectory))
            {
                Console.Write("Creating Program-Directory...");
                Directory.CreateDirectory(applicationdirectory);
                Console.WriteLine("Done...");
            }
            Console.Write("PasswordChecker\nPath:");
            Passwordfilepath = Console.ReadLine();




            FileInfo sourcefileinfo = new FileInfo(Passwordfilepath);


            if (sourcefileinfo.Exists)
            {
                Console.Write("Your password:");
                var password = Console.ReadLine();

                var checker = new PasswordChecker(password, Passwordfilepath);
                IPasswordLoader passwordLoader;


                if (FileisZip(sourcefileinfo))
                {
                    passwordLoader = new ZipPasswordLoader(applicationdirectory);
                    passwordLoader.InputNeeded += ZipFileInputNeeded;
                }
                else if (FileisText(sourcefileinfo))
                {
                    passwordLoader = new TextPasswordLoader();
                }
                else
                {
                    passwordLoader = null;
                    Console.WriteLine("Unsupported password file type.");
                }

                try
                {
                    Console.WriteLine("Started Reading...");
                    checker.CheckPasswords(passwordLoader);

                }
                catch (Exception ex)
                {
                    Console.WriteLine(ex.Message);
                }
            }



            else
            {
                Console.WriteLine("Your file does not exists!");
            }
                Console.WriteLine("Cleaning up application directory...");
                Directory.Delete(applicationdirectory, true);
                Console.ReadKey();

            
        }
        private bool FileisZip(FileInfo sourcefileinfo)
        {
            return sourcefileinfo.Extension.ToUpper() == ".ZIP";
        }
        private bool FileisText(FileInfo sourcefileinfo)
        {
            return sourcefileinfo.Extension.ToUpper() == ".TXT";
        }
        private void ZipFileInputNeeded(object sender,EventArgs args)
        {
            ZipPasswordLoader loader = sender as ZipPasswordLoader;
            Console.Write("Choose entry:");
            loader.Choosenentry = Console.ReadLine();
        }

    }
}


PasswordChecker.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework_2
{
    
    class PasswordChecker
    {

        private string passwordtocheck;

        public string Passwordtocheck
        {
            get { return passwordtocheck; }
            set { passwordtocheck = value; }
        }
        private string sourcepath;

        public string Sourcepath
        {
            get { return sourcepath; }
            set { sourcepath = value; }
        }

        public PasswordChecker(string passwordtocheck,string sourcepath)
        {
            this.Passwordtocheck = passwordtocheck;
            this.Sourcepath = sourcepath;
        }
        public void CheckPasswords(IPasswordLoader Loader)
        {
            string[] passwordlist;
            passwordlist = Loader.Load(Sourcepath);
            if (passwordlist != null)
            {
                Console.WriteLine("Finished reading...");
                bool foundedpassword = false;
                for (int i = 0; i <= passwordlist.Length - 1; i++)
                {
                    if (Passwordtocheck == passwordlist[i])
                    {
                        Console.WriteLine("{0,-20}{1}", passwordlist[i], "TRUE");
                        foundedpassword = true;
                        break;
                    }

                    else
                    {
                        Console.WriteLine("{0,-20}{1}", passwordlist[i], "FALSE");
                    }
                }
                if (foundedpassword)
                    Console.WriteLine("Founded your password! Better choose another one...");
                else
                    Console.WriteLine("Don't founded your password!");
            }
            
        }
                


    }
}


IPasswordLoader.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework_2
{
    delegate void InputNeededEventHandler(object sender, EventArgs args);
    interface IPasswordLoader
    {
       
        string[] Load(string sourcepath);
        event InputNeededEventHandler InputNeeded;


    }
}


Zippasswordloader.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO.Compression;
using System.IO;
namespace Password_Checker_Rework_2
{



    class ZipPasswordLoader : IPasswordLoader
    {
        private string applicationdirectory;
        public ZipPasswordLoader(string applicationdirectory)
        {
            this.applicationdirectory = applicationdirectory;
        }
        private string choosenentry;

        public string Choosenentry
        {
            get { return choosenentry; }
            set { choosenentry = value; }
        }
        public event InputNeededEventHandler InputNeeded;

        public string[] Load(string sourcepath)
        {
            List<string> output = new List<string>();
           
                FileStream fs = new FileStream(sourcepath, FileMode.Open);
                ZipArchive archive = new ZipArchive(fs);
                Console.WriteLine("Entries in Archive:");

                for (int i = 0; i <= archive.Entries.Count - 1; i++)
                {
                    Console.WriteLine(i + "." + archive.Entries[i].Name);
                }
                if (Choosenentry != null) InputNeeded(this, null);
                
                Validator validator = new Validator();

                if (validator.InputIndexIsValid(choosenentry, archive.Entries.Count))
                {
                    int validatedchoosenentry = int.Parse(choosenentry);
                    ZipArchiveEntry entry = archive.Entries[validatedchoosenentry];

                    string extractedfilepath = Path.Combine(applicationdirectory, entry.Name);
                    entry.ExtractToFile(extractedfilepath);

                    StreamReader reader = new StreamReader(extractedfilepath);
                    while (reader.Peek() != -1)
                    {
                        output.Add(reader.ReadLine());

                    }
                    reader.Close();

                }
                else
            {
                throw new Exception("Can not validate your input!");
            }
            
            return output.ToArray();
        }
    }
}


TextPasswordLoader.cs:



using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
namespace Password_Checker_Rework_2
{
    class TextPasswordLoader : IPasswordLoader
    {
        public event InputNeededEventHandler InputNeeded;


        public string[] Load(string sourcepath)
        {
            
            List<string> output = new List<string>();
            try
            {
                var reader = new StreamReader(sourcepath);
                while (reader.Peek() != -1)
                {
                    output.Add(reader.ReadLine());
                }

                reader.Close();
            }
            catch
            {
                throw new Exception("Error while loading .txt file.");
            } 
            return output.ToArray();
        }
    }
}


Validator.cs:


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Password_Checker_Rework_2
{
    class Validator
    {
      
           public bool InputIndexIsValid(string value, int arrayLength)
        {
            int output;
            if (!int.TryParse(value, out output))
            {
                return false;
            }
            if (output <= arrayLength - 1 && output >= 0)
            {
                return false;
            }
            return true;
        }
    


    }
}


Ist meine Umsetzung okay? Ich habe mich entschieden in Proccess und LoadPassworts die Konsole zu verwenden...

LG

LG

3.003 Beiträge seit 2006
vor 8 Jahren

Im Sinn der ursprünglichen Frage - OOP - geht das in Ordnung. An der Form solltest du dennoch unbedingt arbeiten und es dir jetzt noch vernünftig angewöhnen, so lange du noch, sagen wir mal "formbar" bist 😉. Dazu gehört alles, was schon erwähnt wurde:


private string passwordfilepath; //private member camel case, gern mit führendem _ oder m_
private PasswordChecker passwordchecker; //dito, zudem Memberdeklarationen zentral an einer Stelle der Klasse
FileInfo sourcefileinfo = new FileInfo(Passwordfilepath); //implizit mit var deklarieren
public string Passwordtocheck //Properties mit Pascal Case, gilt auch für andere Properties
else if (!Directory.Exists(applicationdirectory)) //stattdessen einfach else {}
StreamReader reader = new StreamReader(extractedfilepath); //stattdessen: using(var reader = new StreamReader(extractedFilePath)) {}, Close() wird dann automatisch vom Compiler gemacht, selbst wenn im Codeblock eine Exception kommt

Gibt noch einige andere Kleinigkeiten. Dein Event ist nicht korrekt implementiert, dort bitte selbstständig recherchieren, wie man das macht. (Außerdem würde ich das Event in der Schnittstelle deklarieren.)

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

R
R3turnz Themenstarter:in
125 Beiträge seit 2016
vor 8 Jahren

Okay, ich habe alle Variaben angespasst und überall wo es möglich war ein var eingesetzt. Den Namen der Event Mezhode habe ich angepasst: PasswordLoader_InputNeeded. Was meinst du mit nicht richtig implementiert?
Ich habe einen delegaten mit EventHandler am Ende:


delegate void InputNeededEventHandler(object sender, EventArgs e);

Das Event:(In der Schnittstelle und in den Loader Klassen)


public event InputNeeded;

Und ich habe in der abbonierenden Klasse eine Methode die Methode mit Objekt_Ereignisname benannt 😉

Wo habe ich einen Fehler?

LG

3.003 Beiträge seit 2006
vor 8 Jahren

a) den Delegaten brauchst du nicht
b) du löst das Event nicht korrekt aus (so wird's krachen, wenn es nicht abonniert ist)


if (Choosenentry != null) InputNeeded(this, null);
//sollte sein:
var handler = InputNeeded;
if(handler != null) 
   handler(this, null);

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)