Open flokosoft opened 2 years ago
@flokosoft Hallo Florian, entschuldige bitte die Wartezeit. Wie schon im Frage und Antworten Bereich auf Udemy angekündigt, sehen wir uns deine Lösung an.
Zuerst etwas zu MVC, wenn du das MVC-Pattern zu Übungszwecken, oder um die Aufgabe zu erweitern anwenden möchtest, ist das ok. Es wäre aber für die Aufgabe selbst nicht erforderlich. Was ich damit sagen will ist, dass der Focus für die eigentliche Aufgabe hier schnell verloren gehen kann, besonders wenn die Erweiterung der Aufgabe größer ist, als sie selbst.
Ich mache es so als würde das Programm ablaufen, so gehen wir Methode für Methode durch:
Main()
Die entsprechenden Objekte werden Instanziiert und die Methode start()
wird auf dem controller
aufgerufen.
controller.start()
Die Methode ist sehr strukturiert, man kann sofort erkennen was sie macht.
Methodenbezeichner fangen immer mit Großbuchstaben an, jedes weitere Wort im Bezeichner auch (CamelCase-Notation).
Methodenbezeichner sollten so formuliert sein, dass man erkennt das sie was machen. Z.B. MacheDies(), StarteVerschlüsslungNachRot13(), du kannst ruhig lange Methodennamen verwenden, der Quellcode wird dann besser lesbar.
Noch ein Tipp zu der Klasse selbst, die Felder _consoleCryptView
und _cryptModel
können Schreibgeschützt sein, da sie nicht verändert werden, das macht es sicherer.
private readonly ConsoleCryptView _consoleCryptView;
HerzlichWillkommen()
alles ok
ZeichenKetteVomBenutzerHolen()
Es gibt ein Prinzip in OOP, das heißt Single-Responsibility-Prinzip SRP, Prinzip der eindeutigen Verantwortlichkeit. Diese Methode macht gleich drei Sachen, Benutzereingabe holen, alle Buchstaben zu Großbuchstaben umwandeln, und Umlaute austauschen.
Das kann man Prüfen, indem man alles in den Methodenbezeichner aufnimmt, was die Methode letztendlich macht. Wenn dann MachDasUndDasOderDasUndDas()
herauskommt, liegt oft ein Verstoß gegen SRP vor.
Ich würde hier SRP auch auf Klassenebene Anwenden, d.h. man muss sich Gedanken machen, was in welche Klasse gehört. In diesem Fall wäre zu überlegen, was soll die View machen. Ich würde es definieren, als Interaktion mit dem Benutzer, daher würde ich die Vorbereitung des Strings zur Verschlüsselung, ToUpper()
, .Replace()
mit in das Model nehmen. Die Klasse Regex ist nicht unbedingt erforderlich, die Methode Replace()
, kann auch auf dem String aufgerugen werden, strBenutzerEingabe.Replace("Ü", "UE");
VerschluesselungLaeuft()
alles ok :-)
Rot13Crypt()
Das du Kommentare benutzt, um den Funktionsablauf erstmal zu strukturieren ist ok. Bei der Implementierung verschafft man sich oft so einen Überblick und nimmt die Kommentare nach der Implementierung wieder heraus. Es ist also anzustreben den Quellcode, die Bezeichner der Variablen etc. so zu wählen, dass die Kommentare überflüssig werden. Das ist anfangs etwas schwieriger, aber dein Quellcode wird langfristig besser lesbar. Ein Präfix wie str
oder int
vor dem Bezeichner der Variablen ist nicht erforderlich, aus dem Bezeichner und dem Kontext im Quellcode sollte zu erahnen sein um was für ein Datentyp es sich handelt. Meines Erachtens sollte man das Präfix immer weglassen, denn es verschlechtert die Lesbarkeit des Quellcode.
Das Erstellen von Objekten in der Klasse von deren Typ das Objekt ist:
List<CryptModel> positionDerBuchstaben = newList<CryptModel>();
CryptModel a = new CryptModel();
Versuche es vorerst zu vermeiden solche Objekte zu erstellen, es gibt Anwendungsfälle wo es erwünscht ist und es ist auch legitim. Es Kann aber zu einem ungewollten rekursiven Verhalten führen. Z.B. würde man diese Klasse Instanziieren, hatte man ein StackOverflow:
public class CryptModelRecursive
{
private CryptModelRecursive objectA;
public CryptModelRecursive()
{
objectA = new CryptModelRecursive();
}
}
Du brauchst keine List<CryptModel> ...
, um einen Index mit dem jeweiligen Buchstaben zu erhalten, ein Array würde hier ausreichen. Ich habe damals eine ähnliche Lösung implementiert. Bei Interesse hier Klicken Aber auch dies ist nicht erforderlich, wenn du dir die Lösung von Jan anguckst.
Ob du eine Zahl oder eine Buchstaben hast kannst du mit Char.IsLetter()
herausfinden. Verwende bitte keinen Try-Catch-Block um deine Programmfluss zu steuern, dieser sollte nur für die Behandlung von Exceptiones benutzt werden.
Zwei Kleinigkeiten sind noch zu nennen, ß und Sonderzeichen werden in deinem Algorithmus nicht berücksichtigt. Bei allen Sonderzeichen in der Eingabe, bekommt man ein "M" in der Ausgabe.
VerschluesselteAusgabe()
Mit der Anweisung Console.WriteLine("Algorithmus angewandt: {0}", _cryptModel.Rot13Crypt());
rufst du erneut die Methode Rot13Crypt()
auf. Im Umkehrschluss bedeutet das, du kannst die Anweisung _cryptModel.Rot13Crypt();
in der Methode start()
weglassen, das Programm funktioniert trotzdem, die Methode wird also zwei mal hintereinander aufgerufen. Vielleicht wäre es besser den Rückgabewert von Rot13Crypt()
zwischen zu speichern und diesen dann der Methode VerschluesselteAusgabe()
zu übergeben. Dann benötigst du das _cryptModel im View nicht und hättest die Koppelung der Klassen minimiert, was aus OOP-Sicht sehr vorteilhaft ist.Ich hoffe du bist jetzt nicht zu sehr verunsichert und hegst einen Groll gegen mich, das was du gemacht hast ist nicht der einfachste Weg Objektorientierung zu lernen. Es ist so, als wolltest du dein Meisterstück zum Anfang der Lehre machen. Ein Design-Pattern umzusetzen ist oft keine leichte Aufgabe und erfordert viel Grundkenntnisse und Erfahrung. Wenn du möchtest, würde ich dir empfehlen, dich mit folgenden Grundlagen der OOP zu beschäftigen.
Das ist schon eine ganze Menge, wenn du Hilfe, oder Quellen zu den Themen benötigst, dann einfach mir schreiben, entweder mit @MK-NEUKO hier im Issue weiter schreiben, oder auf den anderen bekannten Kanälen.
Viel Grüße Michael von LernMoment.de
Hier meine Lösung: Zum Git
Lösung und alles was nach dem Video "Aufgabenbeschreibung" kommt, werde ich mir jetzt angucken.
Mehr Infos hab ich unter "Frage und Antworten" niedergeschrieben.
Liebe Grüße, Florian