Da du nach konstruktiver Kritik gefragt hast, schreibe ich mal eine Kleinigkeit dazu.
CancellationToken
Du nutzt einen CancellationToken und übergibst ihn in ExecuteSshCommandAsync, verwendest ihn aber nicht. Du kannst den CancellationToken überall entfernen und das Programm würde immer noch funktionieren oder du fragst mit IsCancellationRequested ab, ob ein Abbruch angefragt wurde.
- private async Task ExecuteSshCommandAsync([...])
- {
- [...]
- using (var SshClient = new SshClient(conInfo))
- {
- [...]
- while (!asyncResult.IsCompleted || !reader.EndOfStream)
- {
- if (cancellationToken.IsCancellationRequested) // <-------
- {
- MessageBox.Show($"The following command couldn't be executed:\n{command}");
- break;
- }
- [...]
- }
- [...]
- }
- }
Alles anzeigen
Code Duplikation
Fast 60% von deinem gesamten Code ist Wiederholung für praktisch das Selbe. Statt jedem einzelnen Menü-Item einen eigenen Event-Handler zu geben, kannst du stattdessen einen einzigen nutzen und die Tag-Eigenschaft (im UI-Designer, rechts im Eigenschaften-Fenster) der einzelnen Menü-Items für die Commands verwenden. In diesem Beispiel siehst du, wie man es sehr übersichtlich zusammenfassen kann:
- private async void ToolStripMenuItem_Click(object sender, EventArgs e)
- {
- ToolStripMenuItem menuItem = sender as ToolStripMenuItem;
- string command = menuItem.Tag as string;
- if (command != null)
- using (var cancellationToken = new CancellationTokenSource())
- await ExecuteSshCommandAsync(command, cancellationToken.Token);
- }
Ressourcen
Ich empfehle dir, Ressourcen auch als Ressourcen zu definieren. Dazu gehören zum Beispiel Bilder. Das kannst du machen, indem du bei deiner Projektmappe im Ressourcen Ordner die Bilder einzeln anklickst oder alle markierst und im Eigenschaften-Fenster den Build-Vorgang auf Resource stellst.
Google Login
Der Login kann komplett übersprungen werden. Sogar auf mehrere Arten:
- Ich kann die Datei in der ein angemeldeter Nutzer gespeichert wird einfach mit sifdouhdi93ufh füllen und das Programm würde mich als eingeloggten Nutzer erkennen.
- Ich könnte eine eigene Methode injizieren, welche Application.Run() ausführt und diese als Einstiegspunkt festlegen.
- Ich kann das Programm einfach auseinandernehmen und die Abfrage komplett löschen / meine eigene Datenbank einbinden.
- Bei dem bisschen Code wäre Reverse Engineering 101 ausreichend um zur passenden Instruktion zu springen.
- Selbst Cheat Engine könnte den Job problemlos übernehmen.
Am Ende kommt ein Vorschlag, wie du es besser machen könntest.
Vorschlag: Costura.Fody
Wenn du dein Ordner sauber halten willst und nicht so viele DLLs haben möchtest die das Programm untergehen lassen, kannst du über Extras -> NuGet-Paket-Manager -> NuGet-Pakete verwalten das Package Costura.Fody installieren. Jetzt, wenn du das Projekt kompilierst, sind alle DLLs in deiner Binary gebündelt.
Sicherheit
Ich glaube, den Punkt kann ich mir sparen. Ein Programm was nicht nur zur privaten Nutzung verwendet wird und wo das Risiko besteht, dass es aus Versehen durch ein Teammitglied versendet werden kann, sollte keine sensiblen Login Daten enthalten. Besonders in Metin2 sind viel zu viele Leute viel zu naiv und legen private Keys im selben Ordner ab wo das Programm liegt oder auf dem Desktop. Ich habe private Keys bereits auf Github Repositories gesehen und das passiert unfassbar oft.
Statt ein Google Login zu verwenden und Daten Roh in deinem Programm zu speichern, kannst du einfach einen node.js Server oder ähnliches mit einer simplen API aufsetzen die 30 - 50 Zeilen groß ist. Die übernimmt für dich die ganzen Datenbank-Connections und all den Kram.
Ansonsten..
..ist das ein sehr guter Anfang. Ich empfehle dir auf jeden Fall weiter zu machen und dir Best Practices anzueignen. Ich empfehle jedoch niemanden das Programm zu verwenden, unter keinen Umständen. Außer, du richtest für jeden Nutzer einen eigenen Server- / Datenbankaccount ein, nur für dieses Tool und nur mit den nötigsten Rechten. Das wäre aber dann der Job des Nutzers und so viel wollen Entwickler dem Nutzer in der Regel nicht zumuten.
Da du versucht hast das Programm wenigstens etwas aufzuhübschen, könnte WPF vielleicht was interessantes für dich sein. Ich würde .NET Framework mit WinForms gar nicht erst nutzen, wenn ich du wäre. WPF ist ein Traum, wenn man etwas für Design übrig hat. Wenn du dich an das MVVM Pattern hältst und aktiv durchziehst, kannst du innerhalb wenigen Monaten verdammt viel lernen, da man beim MVVM Pattern quasi gezwungen ist irgendeiner Art von Vorlage zu folgen. Ich habe in deinem Source Code gesehen, dass du dich bemüht(?) hast, Flickern durch die ganzen Control-Updates zu verhindern (durch die ControlExtension.cs). Über sowas müsstest du dir in WPF keine Gedanken mehr machen.
Trotzdem Respekt für die Bemühungen. Die Kernidee ist super und wenn mal jemand Zeit und Lust hat (vielleicht sogar du?), könnte daraus was sehr cooles werden.
