@matbo: Erster und IMHO sehr wichtiger Kritikpunkt: Es läuft nicht. Du versuchst im `start`-Modul ein `Menu`-Modul zu importieren dass es nicht gibt:
Code: Alles auswählen
$ python start.py
Traceback (most recent call last):
File "start.py", line 1, in <module>
import Menu
ImportError: No module named Menu
Namen in Python sind „case sensitive”, dass heisst Gross- und Kleinschreibung spielt eine Rolle.
Das `start`-Modul besteht nur aus diesem einen Import und sonst nichts. Damit ist es ziemlich überflüssig. Ausserdem bedeutet es, dass man `menu` nicht importieren kann, ohne dass das Programm abläuft. Module sollte man aber so schreiben, dass man sie ohne Effekte importieren kann. Denn nur dann kann man sie wiederverwenden, einzelne Funktionen oder Klassen (automatisiert) testen, damit „live” in einer Python-Shell herum spielen um sie zu erkunden oder Fehlern interaktiv auf die Spur kommen, und so weiter.
Bei den Kommentaren wird zu viel gebrauch von '#' gemacht. Eines am Anfang der Kommentarzeile reicht völlig aus. Wozu das anhübschen mit vielen '#' führen kann, sieht man an einigen Kommentaren wo der Text anscheinend nachträglich verändert wurde und jetzt diese Kommentarblockrahmen rechts nicht mehr einheitlich abschliessen.
Im `menu`-Modul ist in Zeile 16 eine Zeichenkette die keinen Effekt hat. Wenn das ein Kommentar sein soll, der an *der* Stelle im Quelltext stehen soll, dann bitte wirklich als Kommentar und nicht als Zeichenkette. Falls es der DocString für das Modul sein soll, dann steht er syntaktisch an der falschen Stelle und gehört vor die ``import``-Anweisungen oben im Modul. Vom Inhalt her scheint mir das aber weder das Modul zu beschreiben noch irgend etwas mit den Funktionen zu tun zu haben, die darunter definiert werden‽
Die Namensgebung und die Quelltextformatierung halten sich nicht an den
Style Guide for Python Code.
Die Namen sind auch teilweise ziemlich kryptisch. Wenn Du `menu_width` meinst, dann schreib das doch auch statt `mw` unter dem man sich nichts vorstellen kann. Diese Funktion aus dem `Texte`-Modul gibt nur eine Zahl zurück, also eigentlich hätte eine Konstante völlig ausgereicht, statt das man in allen möglichen Funktionen und Methoden `Texte.mw()` aufruft und den Wert dann lokal noch mal an einen Namen bindet. Man hätte das auch besser in einer Klasse gekapselt die Methoden hat um Texte zentriert und mit ”Verzierungen” wie Linien und so weiter auszugeben.
Die erste „Abkürzung” in `menuOptions()` verstehe ich nicht. Warum wird da die Funktion verlassen? So etwas gehört dokumentiert.
Einen Namen sollte man nur für den selben („duck”-)Typ verwenden und nicht wie das `special`-Argument für `False` *oder* für eine Liste (oder allgemein ein iterierbares Objekt). Dann ist der Quelltext leichter verständlich, weniger fehleranfällig, und kann auch vereinfacht werden. Wenn man dort statt `False` einfach eine leere Liste übergeben würde, kann man sich das ``if`` sparen und dieses Argument einfach *immer* als Liste/iterierbares Objekt behandeln. Der Name `special` sagt irgendwie nicht wirklich aus, dass es sich um optionale zusätzliche Kopfinformation für das Menü handelt. Da wäre `header_lines` zum Beispiel treffender, und da es optional ist, sollte man das Argument nach hinten in der Signatur verschieben und als Defaultwert zum Beispiel ein leeres Tupel angeben.
``for i in range(len(sequence)):`` um dann mit der Laufvariablen auf eine oder mehrere Sequenzen per Index zuzugreifen ist in Python ein „anti pattern”. Man kann *direkt* über die Elemente von Sequenzen iterieren, ohne einen Umweg über einen Index. Wenn man *zusätzlich* eine fortlaufende Zahl benötigt, gibt es die `enumerate()`-Funktion. Die man auch bei anderen Startwerten als 0 anfangen lassen kann. Soll „parallel” über mehrere Sequenzen iteriert werden, gibt es die `zip()`-Funktion.
Wenn man über „parallele” Listen itieriert, sollte man sich auch fragen warum das überhaupt nötig ist, denn wenn dabei zusammengehörende Daten in verschiedenen Listen stecken, hat man sehr wahrscheinlich etwas bei der Datenstruktur falsch gemacht.
Der Aufruf von `Texte.turnInfo()` hat in der `menuOptions()`-Funktion IMHO nichts zu suchen, denn dadurch ist das Menü nicht mehr allgemein verwendbar.
Auch in der `menuInput()` verstehe ich den Sinn der ersten beiden Zeilen nicht. Genau so wenig wie die `Texte.hitWords()`-Funktion. Die könnte glatt aus einer Anleitung für möglichst schlechten, verwirrenden Quelltext stammen. Der Name der Funktion ist mir unklar, und einen verständlichen Text mit einem kryptischen Funktionsaufruf mit einer magischen Zahl zu verschleiern ist echt schräg.
Du hast auch an mehreren Stellen ``while``-Schleifen bei denen Namen erst an Dummy-Werte gebunden werden, die erst innerhalb der Schleife an sinnvolle Werte gebunden werden. Das sind eigentlich Schleifen bei denen die Abbruchbedingung *in* der Schleife beziehungsweise oft an deren Ende geprüft werden. Dafür verwendet man in Python eine „Endlos-Schleife” und die ``break``-Anweisung. Beispiel:
Code: Alles auswählen
#
# anstatt:
#
spam = None
while spam != 'end':
spam = do_something()
#
# besser:
#
while True:
spam = do_something()
if spam == 'end':
break
Die `menulist` bei der `__init__` der `Menu`-Klasse ist viel zu kompliziert. Da sollten einzelne Argumente übergeben werden statt einer komplexen Datenstruktur. Wenn man so etwas übergeben können möchte, dann stellt man dafür üblicherweise eine Klassenmethode (`classmethod()`) zusätzlich zur Verfügung.
`execs` klingt nach Mehrzahl, es wird dort daran aber nur ein aufrufbares Objekt gebunden — das ist irreführend. Ausserdem gibt es dort auch wieder das Problem, dass entweder ein aufrufbares Objekt *oder* `False` gebunden wird, was an jeder Stelle wo man das Attribut verwenden möchte, einen Test erforderlich macht. Der übliche Wert wenn man „nichts” meint ist nicht `False` sondern `None` und wenn `None` übergeben wurde, sollte in der `__init__` dafür gesorgt werden, dass an das Attribut *in jedem Fall* ein aufrufbares Objekt gebunden wird, damit man beim Verwenden nichts mehr prüfen muss.
Die `update_*`-Methoden müssten eigentlich `set_*` heissen, wenn triviale Getter/Setter in Python nicht sowieso unüblich wären.
Die `make*`-Methoden „machen” nichts in dem Sinne in dem man im englischen das Wort „make” verwendet und auch wenn das deutsche „mache dieses oder jenes” gemeint wäre, dann ist das einfach nur ein allgemeines Füllwort welches man durch eine treffenderes ersetzen sollte, was besser beschreibt was diese Methoden leisten. Beispielsweise `print_options()` und `get_input()`. Da beide Methoden nur den Aufruf von Funktionen enthalten, stellt sich die Frage warum diese Funktionen nicht stattdessen an dieser Stelle stehen.
Diese ``# end-irgendwas``-Kommentare sind eigenartig. Wenn Du Python's Syntax ohne „druckbare” Blockenden nicht magst, dann verwende besser eine andere Programmiersprache. Das Ende sollte man auch ohne solche Kommentare erkennen können.
`MenuController` verwendet in der `__init__` Daten die auf „magische Weise” aus einem anderen Modul genommen werden, statt als Argument übergeben zu werden. Damit ist das `Texte`-Modul viel zu eng an das `menu`-Modul gekoppelt. Das war es bis zu diesem Zeitpunkt zwar auch schon, zum Beispiel durch die `mw()`-Funktion, aber hier ist ein Punkt erreicht der das `menu`-Modul nicht für andere Aufgaben wiederverwendbar macht.
Dabei wird auch wieder eine magische, nichtssagende Zahl verwendet, denn was soll die 2 in ``Texte.menusDict(2)`` aussagen? Was wäre hier die Bedeutung von 0, 1, 3, …?
Was bedeutet „Chall” in `ChallView`?
Das was danach mit den magischen Zahlen mit `self.titles` passiert ist gruselig. Da werden total unnötige Indirektionen über magische Zahlen/Indexe(?) erzeugt, statt eine vernünftige Datenstruktur zu verwenden, wo alle Informationen die zusammengehören auch zusammengefasst gespeichert werden.
Die auskommentierten Debug-Ausgaben kann man eleganter mit dem `logging`-Modul umsetzen. Dann braucht man sie nämlich nicht auskommentieren und kann sie trotzdem je nach Bedarf (de)aktivieren.
In `last_check()` werden Sachen gemacht die nicht objektorientiert sind. Statt in einer riesigen Methode zu testen welchen Menüpunkt man vor sich hat und dann entsprechend etwas zu tun, sollte man einfach eine Methode auf dem gewählten Menüpunkt-Objekt — ein Typ den es in diesem Programm noch nicht gibt — aufrufen, der dann die Aktion selbst durchführt oder aber eine ihm dafür übergebene Funktion oder Methode aufruft.
So, das war's erst einmal für dieses eine Modul. Ein kurzer Blick in das `Texte`-Modul lässt Böses erahnen was die Verwendung von globalen Daten(strukturen) angeht. Da ist sicher noch viel zu tun.
Die (Um)Frage nach der Programmierdauer kann ich nicht beantworten, weil ich es nicht zum Laufen bekomme und damit auch nicht so einfach sehen kann was das Programm eigentlich leistet. Wenn ich den Namen der Menu-Datei korrigiere kommt bei mir das hier:
Code: Alles auswählen
$ python3 start.py
Traceback (most recent call last):
File "start.py", line 1, in <module>
import Menu
File "/home/bj/tmp/affsaison01/Menu.py", line 1, in <module>
from AffSaison import game
File "/home/bj/tmp/affsaison01/AffSaison.py", line 867, in <module>
game = Game()
File "/home/bj/tmp/affsaison01/AffSaison.py", line 752, in __init__
self.init_saison()
File "/home/bj/tmp/affsaison01/AffSaison.py", line 787, in init_saison
liga.add_turnier(name, punkte, time, self)
File "/home/bj/tmp/affsaison01/AffSaison.py", line 509, in add_turnier
self.playersToTurnier(tName)
File "/home/bj/tmp/affsaison01/AffSaison.py", line 512, in playersToTurnier
self.turnierListe[tu].set_players(self.standings)
File "/home/bj/tmp/affsaison01/AffSaison.py", line 367, in set_players
self.players = new.copy()
AttributeError: 'list' object has no attribute 'copy'
