@AndreaC: Es wurde nach Kritik gefragt.
Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst. Insbesondere das mischen von Hauptprogramm und Funktionsdefinitionen ist unübersichtlich.
Funktionen bekommen alles was sie ausser Konstanten benötigen als Argument(e) übergeben. Wenn die einfach so ”magisch” in der Funktion auf Variablen zugreifen, weiss man nicht welche Funktion mit welchen Variablen arbeitet solange man nicht alle Funktionen bis ins Detail kennt.
Beim öffnen von Textdateien sollte man immer explizit die Kodierung angeben.
`list_book` ist ein komischer Name. Das ist keine Funktion die Bücher listet. Und falls das `list` für den Datentyp stehen sollte: Grunddatentypen haben in Namen nichts verloren. Den Datentyp ändert man im Laufe der Programmentwicklung gerne mal in etwas spezialisierteres, und dann sind entweder Namen falsch und irreführend, oder man muss schauen welche Namen man alle ändern muss. Hier würde sich beispielsweise eine Menge (`set`) statt einer Liste eignen, denn Worte können in dem Buch ja mehrfach vorkommen. Im Moment hat das mit der Liste einfluss auf die Wahrscheinlichkeit mit der ein Wort ausgewählt wird. Soll das so?
Das mit dem `n` ist unnötig umständlich geschrieben. `n` wird zu früh definiert, und eigentlich kann man sich initialisieren und manuell hochzählen ganz sparen wenn man die `enumerate()`-Funktion verwendet:
Code: Alles auswählen
for n, line in enumerate(lines, 1):
if 36 <= n <= 3549:
...
Wenn man keine Zeilen nach Zeile 3549 mehr verarbeiten möchte, wäre es sinnvoll auch tatsächlich mit der Schleife aufzuhören (``break``). Und die ersten 36 Zeilen könnte man in einer zusätzlichen Schleife vor dieser überlesen. Oder man verwendet gleich `itertools.islice()` und spart sich dieses `n` und eigene Tests ganz.
Ein `strip()` ist vor einem `split()` ohne Argumente nicht notwending:
Code: Alles auswählen
In [194]: " ham spam \n".split()
Out[194]: ['ham', 'spam']
Statt später solange Worte zu ziehen bis die Länge innerhalb bestimmter Grenzen liegt, wäre es besser die Worte ausserhalb dieser Grenzen gleich auszufiltern. Die spätere ``while``-Schleife ist nämlich nicht dagegen abgesichert zu einer Endlosschleife zu werden wenn keine passenden Worte vorhanden sind.
Wiederholtes ``+=`` mit Zeichenketten in einer Schleife ist ineffizient und sollte nicht gemacht werden. Man sammelt die Teilzeichenketten besser in einer Liste oder schreibt einen Generatorausdruck und setzt das Ergebnis mit der `join()`-Methode auf Zeichenketten zusammen.
Warum wird `list_book` nach der Wahl eines Wortes wieder an eine leere Liste gebunden? So komische Sachen zur ”Speicherverwaltung” (?) sollte man nicht machen. Das bisschen Speicher sollte eigentlich nicht auffallen, und ansonsten sollte man einfach sauber(er) programmieren statt solche Hacks einzubauen. Wenn das ein lokaler Name in einer Funktion zum einlesen der Daten wäre, dann würde der Name automatisch am Ende der Funktion verschwinden und Speicher freigegeben der freigegeben werden kann.
Wenn man `randrange()` statt `randint()` verwendet, braucht man nicht 1 von der Länge abziehen. Wenn man `random.choice()` verwendet, braucht man sich mit der Länge überhaupt nicht zu beschäftigen und es wird lesbarer.
Das `lower()` nach der Zufallswahl aus einer Liste mit Worten in Kleinbuchstaben kann man sich sparen.
`actual`, `correct`, und `good` sind als Namen nicht gut. Tatsächlich, korrekt, und gut *was*? Und warum tatsächlich?
Die Schleife um `correct` zu füllen kann man sich sparen wenn man `set()` gleich mit dem Wort aufruft.
Es sollte doch nicht nur die Länge von `good` und `correct` gleich sein, sondern auch der *Inhalt* der beiden Mengen.
`check` ist keine Funktion und prüft nichts, sollte also wohl eher `letter` oder `character` heissen.
`exit()` gibt's eigentlich gar nicht wenn man das nicht aus `sys` importiert. Aber die Funktion sollte man sowieso nicht einfach in anderen Funktionen ausser der Hauptfunktion verwenden um sich um einen sauberen Programmablauf zu drücken. Statt `game_over()` innerhalb der Schleife aufzurufen, sollte man die einfach abbrechen und das Programm bis zum Ende der Hauptfunktion laufen lassen.
Zwischenstand (ungetestet):
Code: Alles auswählen
#!/usr/bin/env python3
import random
from itertools import islice
ALPHABET = "abcdefghijklmnopqrstuvwxyzäöüß"
def filter_word(text):
return "".join(character for character in text if character in ALPHABET)
def display(secret_word, tried_letters, tries, points):
result = " ".join(
letter if letter in tried_letters else "_" for letter in secret_word
)
print("\n" + result, 10 * " ", "Versuche:", tries, "Punkte:", points)
def game_over(secret_word, secret_word_letters, correctly_guessed_letters):
if secret_word_letters == correctly_guessed_letters:
print(
"\nDas Wort >>>",
secret_word,
"<<< ist richtig, Du hast gewonnen!!!",
)
else:
print("\nDu hast verloren, geraten werden sollte:", secret_word)
def main():
with open("tomsawyers-deu.txt", encoding="utf-8") as lines:
book_words = []
for line in islice(lines, 35, 3550):
words = (filter_word(word.lower()) for word in line.split())
book_words.extend(word for word in words if 4 <= len(word) <= 12)
secret_word = random.choice(book_words)
points = 0
tries = 0
secret_word_letters = set(secret_word)
tried_letters = set()
correctly_guessed_letters = set()
while correctly_guessed_letters != secret_word_letters:
display(secret_word, tried_letters, tries, points)
letter = input("\nGib einen Buchstaben ein: ").lower()
if not letter:
break
#
# double input, but not empty word
#
if letter in tried_letters:
if correctly_guessed_letters:
points -= 5
#
# first time right letter
#
elif letter in secret_word_letters:
points += 10
correctly_guessed_letters.add(letter)
#
# wrong, but not empty word
#
elif correctly_guessed_letters:
points -= 3
if points < 0:
break
tries += 1
tried_letters.add(letter)
game_over(secret_word, secret_word_letters, correctly_guessed_letters)
if __name__ == "__main__":
main()
