Hallo,
bei diesen beiden Foren sind wir uns hier selten sicher, was wo wirklich reinpasst
Zu Deinem Code kann man so einiges anmerken:
- Wozu diese Klassen? In Wirklichkeit "missbrauchst" Du diese als Modul. Sie haben keinen "inneren" Zustand und kapseln auch nichts. Das könnte man alles eleganter ohne Klassen lösen!
- Es gibt Multiline-Strings (für Zeile 11):
Wenn man dabei eine Einrückung zur besseren Lesbarkeit bewahren will, kann man mittels `dedent`-Modul die Einrückungstiefe eleminieren.
- Strings sollte man nicht per `+`-Operator zusammenbauen - auch wenn das natürlich geht. Benutze dafür z.B. die `"".format()`-Methode:
- Zeile 37: Öffnet man Dateien mit `with`, braucht man diese nicht explizit zu schließen. Das `f.close` kannst (und solltest!) Du Dir sparen.
- Ich weiß nicht genau, ob es eine elegante Möglichkeit gibt, einzelne Abschnitte in einem Python-Programm mit speziellen Rechten auszuführen, aber Deine Idee, `sudo` mit `os.system` zu koppeln klingt für mich wenig sinnvoll. Zum einen ist das sehr plattformspezifisch (Unter Windows wird das alles nicht klappen!), zum anderen wird es schwierig, eine sinnvolle Fehlerbehandlung durchzuführen. Bei Dir fehlt die ja auch komplett. Wenn Du die entsprechenden Funktionen aus den `os`- oder dem `shutil`-Modulen benutzt, wird das schon einfacher zu realisieren. Insofern würde ich dann ggf. für die komplette Ausführung des Programms Administratorrechte "verlangen", aber dafür die Funktionen aus der Standard-Lib benutzen.
- Anstelle von `os.system` solltest Du auch eher auf das `subprocess`-Modul setzen.
- Die Methode `troubleshoot` ist viel zu "komplex". Du erledigst dort zig verschiedene Dinge, die man auf den ersten Blick wunderbar in separate Funktionen auslagern könnte. Zudem erwarte ich doch, dass das "Dispatching" (also das Verzweigen, je nach Auswahl) außerhalb erledigt wird. Du hast doch extra eine `UserInterface`-Klasse implementiert; wieso wird das dann nicht auch darin erledigt? (Obwohl ich immer noch dafür plädiere, diese Klassen einzustampfen!)
- Dir fehlt es an einer Struktur für Deine Menüeinträge. Was passiert, wenn Du das Menü an zwei Stellen ändern willst und noch Einträge dazu kommen, die dann auch noch an verschiedenen Stellen eingefügt werden sollen? So etwas kann man einfacher lösen, indem man für ein Menü eine passende Datenstruktur benutzt und die Verzweigung dann "berechnet", anstatt sie hart in langen `if...elif`-Kaskaden zu kodieren. (Für diese Sache brauchen wir mal eine wiki-Seite
)
Beispiel:
Code: Alles auswählen
main_menu = (
("Install Minecraft", install),
("Uninstall Minecraft", uninstall),
("Troubleshooting", troubleshoot),
("Exit", lambda: sys.exit(0)),
)
for index, item in enumerate(main_menu, 1):
print "{} {}".format(index, item[0])
choice = input(...)
# here is the 'magic'; we call a function, without using its name.
main_menu[choice-1][1]()
Ich habe hier ein Tupel für jeden Menüeintrag benutzt. Als erster Eintrag steht ein String, der den Eintrag beschreibt, als zweites Element füge ich ein Funktionsobjekt hinzu, welches bei Auswahl aufgerufen werden soll. Das ist die "Magie", die für viele Anfänger neu / unbekannt ist. In Python ist alles ein Objekt und deswegen kann ich alles auch an Stellen verwenden, an denen ein Objekt stehen darf. Also auch in Listen, Dictionaries, Tupeln usw.
Meine Struktur kann ich nun einfach durchgehen und die Ausgabe erzeugen. Die Benutzereingabe habe ich nur mal angedeutet, aber da hast Du ja etwas brauchbares. Bei positiver Verifikation benutzt Du den Index (-1, da Indizies bei 0 anfangen, unsere Ausgabe aber bei 1 anfängt) und pickst den zweiten Eintrag heraus, also die aufzurufende Funktion. Diese rufst Du dann einfach auf `()`. Schon landest Du in den Funktionen, die dann eine bestimmte "Arbeit" durchführen.
Willst Du nun Einträge umsortieren, neue Einträge einfügen o.ä., brauchst Du nur die Menüdefinition anpassen. Ich hoffe Du siehst, dass das viel einfacher ist, als Deine Lösung.
Ok,. Du musst noch ein Submenü einbauen... da lasse ich Dich mal selber überlegen, wie man das realisieren könnte
- Zeile 131: Das einzelne `raise` soll genau was bringen?
- Zeile 132: Fange niemals *alle* Ausnahmen ab. Fange nur diejenigen ab, die Du wirklich behandeln willst. In dem Falle wohl einen `ValueError`.
- Zeilen 140f.:
Was soll das bewirken? Wozu der Name `troubleshoot`? Du kannst doch genauso gut gleich
schreiben! Du verlässt die Schleife doch eh erst durch ein `break` und nicht etwa dadurch, dass Du `troubleshoot` auf `False` setzt. Zudem bindest Du im Schleifenrumpf ständig neue Werte an `troubleshoot` - ist das wirklich beabsichtigt? Wäre ein Menüeintrag "0", so würdest Du unfreiwillig die Schleife verlassen
- Als letztes: Es fehlt ein wenig Doku und ggf. ein Lizenzhinweis. Zumindest einen Modul-Docstring mit Hinweis auf die Funktionsweise des Moduls könnte man erwarten.
So, das war ziemlich viel... ich hoffe Du kannst da einiges von gebrauchen.
Positiv finde ich übrigens, dass Du Dein Script in einem Pastebin ausgelagert hast
