@Astorek: So ganz grob: Namen sind oft zu kurz und Funktionen oft zu lang.
Im Gegensatz zu QBasic reicht es in Python nicht eine Funktion oder Methode einfach nur "hinzuschreiben", man muss sie auch tatsächlich *aufrufen*. Denn ein aufrufbares Objekt zu referenzieren und es aufzurufen sind in Python zwei voneinander trennbare Schritte, da Funktionen und Methoden auch Objekte sind, und man sie zum Beispiel an andere Namen binden oder als Argumente übergeben kann. Also ``spam = eggs`` ist etwas anderes als ``spam = eggs()``. Im ersten Fall wird das Objekt, das an den Namen `eggs` gebunden ist, auch an den Namen `spam` gebunden, und im zweiten Fall wird das Objekt `eggs` aufgerufen und das *Ergebnis* an den Namen `spam` gebunden. Sollte es keine Zuweisung geben, sondern nur die rechte Seite des Ausdrucks, dann ist der erste Fall einfach nur eine Referenzierung von `eggs` ohne einen Effekt. So etwas hast Du zum Beispiel in `dumbmenu()` mit der Zeile `pygame.font.init`. Das macht nichts aus, weil man diese Funktion normalerweise sowieso nicht selbst aufrufen muss, wenn man davor schon mal `pygame.init()` aufgerufen hat.
`dumbmenu()` ist so eine Funktion die zu lang ist. Das mit den Nummern und Buchstaben ist ein wenig undurchsichtig. Wenn man das erste mal `char` sieht, wird der Wert erhöht, man hat aber noch gar nicht die erste Bindung gesehen.
Den Namen `i` in einer Schleife für etwas anderes als eine Zählvariable zu verwenden ist keine gute Idee. Das verwirrt gerade erfahrene Programmierer ungemein, denn `i`, `j`, und `k` sind in sehr vielen Sprachen traditionelle Namen für ganzahlige Zählvariablen in Schleifen. Da `cursorpos` "manuell" parallel zu den Menüpunkten hochgezählt wird, kann man das mittels `enumerate()` auch in die Schleife integrieren. Oder man könnte die Positionen als Generator schreiben. Man könnte den Teil zum Beispiel so formulieren (`count()` und `izip()` aus `itertools`):
Code: Alles auswählen
labels = string.digits[1:] + string.ascii_uppercase
positions = (size // distance * i for i in count())
for position, label, text in izip(positions, labels, menu):
rendered_text = myfont.render('%s. %s' % (label, text),
True,
fgcolor)
textrect = rendered_text.get_rect()
textrect = textrect.move(x_pos, position + y_pos)
screen.blit(rendered_text, textrect)
pygame.display.update(textrect)
Vergleiche auf `True` und `False` sind überflüssig wenn das, was man damit vergleicht sowieso schon ein Wahrheitswert ist.
Auf `exitMenu` kann man wohl auch ganz verzichten, wenn man stattdessen an den entsprechenden Stellen einfach ein ``return`` verwendet.
Der "hässliche lange Teil" kann mit einem Dictionary verkürzt werden, dass `pygame.K_*`-Konstanten auf die entspechenden Werte abbildet. Mehrere Anweisungen mit Semikolon getrennt sind in Python nicht so gerne gesehen. Das ist in der Regel "Platz" sparen auf Kosten der Lesbarkeit. Gleiches gilt für Code direkt nach ``:`` statt den eingerückt in die nächste Zeile zu schreiben.
Die "wrap around"-Logik für den Menücursor kann man mit Modulo mit weniger Tests erreichen. Einfach bei den entsprechenden Tastendrücken `cursorpos` anpassen und am Ende mit ``cursorpos %= len(menu)`` dafür sorgen, dass der Wert im erlaubten Bereich landet.
Bei der `loadLevel`-Klasse solltest Du Dich nochmal mit Objektorientierung beschäftigen. Die ist grauenvoll. Objekte sollten nach dem erstellen benutzbar sein. Alle Attribute des Objekts sollten in der `__init__()` gebunden werden -- zumindest an einen Platzhalter, damit man als Leser sieht aus was solche Objekte eigentlich bestehen.
Der Name `pos` ist in dem Modul ziemlich verwirrend, weil ich das im Kopf irgendwie immer in `position` expandiere, aber anscheinend zumindest nicht immer Positionen damit gemeint sind!?
Die vielen doppelten Unterstriche wurden ja schon erwähnt.
Warum werden `__returnvalue` und `__yf`, in `getPos()` an das Objekt gebunden? Was bedeutet der Index 2 in dieser Methode? Solche magischen Zahlen machen Programme undurchsichtig.
Die ganzen `set*`-Methoden, die einfach nur Attribute auf die übergebenen Werte setzen sind unpythonisch. Die kann man auch direkt setzen. Triviale "getter" wie `getArray()` sind auch nicht nötig.
In `fillArray()` werden wieder *alle* Namen an das Objekt gebunden. Methoden zu verwenden bedeutet doch nicht, dass man keine lokalen Namen mehr verwenden darf. Die Methode ist auch zu lang und zu kompliziert. Ich hab's jetzt nicht genau durchblickt, aber ich habe auch den Eindruck Du hast es irgendwie mit "anlegen von Arrays" und die dann mittels Indexzugriffen mit Werten zu füllen. An der Stelle baut man die Listen in Python in der Regel dynamisch auf.
Der Name von `_startme.py` ist ungünstig weil widersprüchlich, denn ein führender Unterstrich bedeutet per Konvention in Python "Implementierungsdetail, bitte nicht direkt verwenden".
Pfade plattformunabhängig zusammensetzen geht mit `os.path.join()`. Das kommt auch damit klar, wenn eine Plattform mehr als einen Pfadtrenner unterstützt und die in den Teilen gemischt vorkommen.
Die Werte bei `lev.setVals()` sehen mir eher nach einem Einsatz für ein Dictionary aus. Und die Zahlen, die den Indizes entsprechen sehen mir irgendwie redundant aus!? Und es wären auch wieder so "magische" Zahlen, bei denen man gar nicht weiss, was sie bedeuten, wenn man ihnen ohne weitere Erklärung begegnet. Die ganzen `set*()`-Aufrufe hätte man sich auch sparen können, wenn die Klasse eine `__init__()`-Methode hätte, der man die Werte hätte übergeben können.
Dass die `main()` *viel* zu umfangreich ist, wurde ja schon gesagt.
Das Menü selbst und einige Menüpunkte bringen Textzeilen auf den Bildschirm und da wiederholt sich ein guter Teil Quelltext, den man in eine Funktion auslagern könnte.
Das die "Nicht-Programme" eine Meldung ausgeben, wenn man sie als solche starten will, ist zwar IMHO überflüssig, aber nicht so schlimm. Das sich das Programm aber aktiv dagegen wehrt als Modul importiert zu werden ist Unsinn. Das kann Dokumentationsgeneratoren und Testprogramme behindern.