Feedback zu Hangman

[e.s.t.e.senor]

Hallo zusammen,
Ich beschäftige mich erst seit kurzem mit Python und möchte einer meiner ersten Python-Projekte hier vorstellen. Dabei habe ich mit Pygame und Tkinter gearbeitet, um das Spiel Hangman in Python umzusetzen (random wird auch verwendet). Das Programm an sich funktioniert ohne Fehlermeldungen, doch ich wäre sehr glücklich über Feedback, da bei meinen Codes als Anfänger wahrscheinlich viel Verbesserungspotenzial besteht. Wie könnte man z.B meinen Code "eleganter" gestalten oder vereinfachen? Welche Funktionen wären für das Programm noch praktisch? Ist die Struktur mit zwei Klassen sinnvoll? Welche evt. Schwachstellen müssen beseitigt werden? Ich bin dankbar für jegliche Art von Input und Tipps.
Hier ist der Code:

Code: Alles auswählen

# Imports
from tkinter import *
import random
import pygame as pg # Muss zusätzlich zu Python heruntergeladen werden

# Farben (nicht alle werden gebraucht)
Blue = (0, 0, 50) 
Lila = (255, 0, 255)
White = (255, 255, 255)
Red = (255, 0, 0)

# Das self macht Variabeln von ausserhalb der Klasse und der Methode innerhalb der Klasse abrufbar

class choseWord(): 

    def __init__(self) : # Wird beim Aufrufen der Klasse ausgeführt
        self.createWindow()

    def createWindow(self): 
        # Erstellung des Fensters und seiner Elemente
        Fenster = Tk()
        Fenster.title("Anfang")
        Fenster.config(height=300, width=600)
        Anzeige = Label(Fenster, text="Wie willst du spielen?")
        Anzeige.place(width=500, height=100, x=50, y=20)
        Eingabe = Entry(Fenster)
        Eingabe.place(width=500, height=50, x=50, y=220)
        # Erstellung von Buttons, welche mit commands verküpft werden (man braucht lamda, da zwei commands)
        Knopf1 = Button(Fenster, text="Eigenes Wort einreichen", command=lambda: [self.ownWord(Eingabe), self.furtherProcessing(Fenster)])
        Knopf2 = Button(Fenster, text="Zufallswort", command=lambda: [self.randomWord(), self.furtherProcessing(Fenster)])
        Knopf1.place(width=200, height=100, x=50, y=100)
        Knopf2.place(width=200, height=100, x=350, y=100)
        # Schleife
        Fenster.mainloop()   
        
    def ownWord(self, par1):
        self.Wort = par1.get() # Man bekommt, den Input des Fenster-Elements Eingabe
        
    def randomWord(self):
        # Aus Datei werden Wörter in die Liste Wörter reinkopiert, und aus dieser wird ein Wort zufällig ausgewählt
        Wörter = []
        Datei = open("Wörter_Liste.txt", "r") # Textdatei muss im selbem Ordner wie der Code vorhanden sein
        for Zeile in Datei:
            Wörter.append(Zeile)
        Datei.close()
        self.Wort = Wörter[random.randint(0, 99)]
        
    def furtherProcessing(self, par1):
        # Wort wird für den späteren Gebrauch modifiziert, Variabeln die für die Klasse Hangman() benötigt werden, werden erstellt
        self.Buchstaben = []
        self.Wort = self.Wort.rstrip("\n").upper()
        for Buchstabe in self.Wort:
            self.Buchstaben.append(Buchstabe)
        Länge = len(self.Wort)
        self.Wortstand = Länge * "_ "
        par1.destroy() # Nötig, da tkinter nicht mit pygame kompatibel
    

# Player-Klasse 
class Hangman (pg.sprite.Sprite) : 
    
    def __init__(self) : 
        while True: # Beim Aufrufen der Klasse, wird immer wieder erstmal choseWord() ausgeführt und dann self.play(), die sich der Methoden der Klasse Hangman() bedient
            # Try-except-Struktur braucht man um Fehlermeldung beim Schliessen des tkinter-Fensters zu vermeiden
            try:
                self.tkinterPart = choseWord()
                super().__init__()
                pg.init()
                self.play()
                pg.quit()
            except: # Bei Fehler wird pygame und die while-Schleife verlassen
                pg.quit()
                break
                
               
    def play(self):
        # Fenster wird erstellt, Liste Bilder wird gefüllt
        Fenster = pg.display.set_mode((1200, 600))
        Bilder = self.initGraphics()
        # Grundlegene Variabeln für das Programm werden für die Klasse erstellt
        Buchstaben = self.tkinterPart.Buchstaben # Kein self, da das nicht abrufbar sein soll
        self.Wortstand = self.tkinterPart.Wortstand
        self.Eingabe = ""
        self.beiVerlieren = "" # Wird nur geändert falls verloren (sonst kein Absatz möglich)
        self.Fehler = 0
        self.finished = False 
        self.running = True
        # While-Schleife: 1. Was macht der User und auf Input reagieren; 2. Prüfen ob gewonnen und verloren; 3. Oberfläche updaten
        while self.running:
            for event in pg.event.get():
                self.checkEvent(event, Buchstaben)
            self.wonOrLost(Fenster, Buchstaben)
            self.update(Fenster, Bilder)       

    def checkEvent(self, event, Buchstaben):
        if event.type == pg.QUIT:
            # Wenn rotes Kreuz gedrückt, pygame verlassen und while-Schleife von self.play() verlassen
            # Dadurch kann bei der while-Schleife in der Initialisierung der nächste Schritt erfolgen  
            self.running = False
        # Sei angemerkt: bei pygame gibt es keinen einfachen input()-Befehl wie bei tkinter
        if event.type == pg.KEYDOWN and self.finished == False: # Wenn finished auf True gestzt kann man nicht mehr den Wortstand verändern und keine zusätzlichen Fehler machen       
            # Wenn etwas "eingereicht" wird, wird die Eingabe in Grossbuchstaben konvertiert (das gesuchte Wort wurde ja auch in Grossbuchstaben konvertiert)
            if event.key == pg.K_RETURN: 
                self.Eingabe = self.Eingabe.upper()
                # Wenn Eingabe in der Liste Buchtaben ist, wird folgende Methode aufgerufen
                if self.Eingabe in Buchstaben:
                        self.changeWortstand(Buchstaben)
                # Wenn Eingabe nicht in der Liste Buchtaben ist, hat man ein Fehler mehr gemacht
                else:
                    self.Fehler += 1
                # Nach dem Einreichen, Eingabe wieder frei
                self.Eingabe = ""         
            elif event.key == pg.K_BACKSPACE: # Eingabe kann wieder gelöscht werden
                self.Eingabe = self.Eingabe[:-1]
            else: # Jedes Tastenereignis wird zur Eingabe angerechnet, wenn es sich nicht um Return oder Löschen handelt
                self.Eingabe += event.unicode
                
    def changeWortstand(self, Buchstaben):
            while self.Eingabe in Buchstaben: # Wenn der eingegebene Buchtabe mehrmals in den übbrig gebliebenen Buchtaben des Wortes vorkommt
                Pos = Buchstaben.index(self.Eingabe) # Wo befindet sich der eingegebene Buchtabe im Wort  
                Buchstaben[Pos] = False # Satt dem gefunden Buchstaben in der Liste Buchtaben, steht jetzt der Wert False
                self.Wortstand = self.Wortstand[:2*Pos] + self.Eingabe + self.Wortstand[2*Pos + 1:] # Der Wortstand (dieser wird geblitet) wird an der richtigen Stelle verändert
                
    def wonOrLost(self, Fenster, Buchstaben):                        
        if self.Fehler > 7: # Wenn zu viele Fehler: Auflösung, und self.finished wird auf True gesetzt
            self.Eingabe = "Das richtige Wort war: "
            self.beiVerlieren = self.tkinterPart.Wort
            self.finished = True    
        if all(element == False for element in Buchstaben) and self.finished == False: # Üperprüfen, ob in der Liste Buchtaben alles auf False gesetzt wurde: gewonnen!
            Komplimente = ["Du bist ein wahrer Meister!", "Knapp dem Strang entkommen!", "Yeah, Hinrichtung verschoben!", "Du hast es geschafft!", "Du hast es verdient!", "Ein Wunder ist geschehen!", "Glückwunsch!"]
            self.Eingabe = random.choice(Komplimente)
            self.finished = True
            
    def update (self, Fenster, Bilder):
        # Sprites in Fenster (neu) positionieren
        Fenster.fill(Blue) # Altes wird einfach überdeckt
        Fenster.blit(Bilder[self.Fehler], (20, 20)) # Je mehr Fehler, desto weiter in der Bilder-Liste
        Fenster.blit(self.showMessage(self.Wortstand, Lila), (20, 500))
        Fenster.blit(self.showMessage(self.Eingabe, Lila), (400, 200))
        Fenster.blit(self.showMessage(self.beiVerlieren, Lila), (400, 300)) 
        pg.display.update()
            
    def initGraphics(self):
        Bilder = []
        for Nr in range(1, 10):
            Bild = pg.image.load("/Applications/Python 3.11/Hangman/Hangman_Projekt/Hangman_Bilder/Hang" + str(Nr) + ".jpeg") # Bilddateien müssen vorhanden sein, Pfadweg ggf. anpassen
            Bild = pg.transform.scale(Bild, (350,450)) # Grösse der Bilder anngepasst
            Bilder.append(Bild)
        return Bilder # Ausgabe der Methode
        
    def showMessage(self, text, Farbe): # parameter text wird so verarbeitet, dass man ihn blitten kann
        Font = pg.font.SysFont("arial", 48)
        Text = Font.render(text, True, Farbe)
        return Text     

pyHangman = Hangman()

Vielen Dank im Voraus!

PS: .
In der choseWord-Klasse wird eine Datei "Wörter_Liste.txt" genutzt, da ich die ca 100 Wörter nicht ins Programm selber reinschreiben wollte. Da ich nicht herausfinden konnte wie man Dateien anfügt, ist diese Datei leider nicht verfügbar.
Ausserdem kann ich die 9 verwendeten Bilder nicht anhängen, die das Galgenmännchen progressiv zeigen. Der Dateipfad müsste eh individuell angepasst werden.

[Sirius3]

*-Importe benutzt man nicht, weil man nicht konotrollieren kann, welche Namen damit in den eigenen Namensraum geladen werden.
Eingerückt wird immer mit 4 Leerzeichen pro Ebene, nicht mal 4 und mal 8.
Konstanten schreibt man komplett gross, Klassen dagegen mit großem Anfangsbuchstaben. Alles andere klein.
Alle Attribute einer Klasse werden in __init__ angelegt.
__init__ ist dazu da eine Klasse zu initialisieren, da hat ein mainloop-Aufruf nichts darin verloren. Man setzt werde die Größe eines Fensters, noch plaziert man Elemente mit place. Beides verhindert, dass GUIs auf verschiedenen Systemen nutzbar sind.
Der Rückgabewert von Callbacks wird nicht benutzt, warum erzeugst Du also Listen?
Variabelnnamen sollten sprechend sein, was bedeutet par und was soll die 1?
Dateien öffnet man innerhalb eines with-Blocks. Man gibt immer das Testencoding an. Es gibt random.choice.
Auch in der Hangman-Klasse ist __init__ falsch. Noch viel mehr, da es sich ja um ein Sprite-Objekt handelt. Nackte excepts darf man nicht benutzen, weil es die Fehlersuche verhindert.
__init__ darf nur einmal aufgerufen werden.
Es ist etwas seltsam, welche Variablen du als Attribute ans Objekt bindest, und welche Du über Argumente an alle Funktionen übergibst.
Mit False vergleicht man nicht direkt, sondern benutzt `not`.
Buchstaben ist eine Liste, die entweder Strings oder False enthält. Eine Liste sollte aber immer nur Elemente eines Typs enthalten.
Strings verarbeitet man möglichst nicht per + und Indexzugriff.
Verschieden GUI-Rahmenwerke sollte man nicht mischen. Alles was Du tust läßt sich einfach mit tkinter umsetzen, pygame ist eigentlich unnötig.

Schreibe richtige Klassen, also eine __init__ die wirklich nur initialisiert.

Schreibe die Logik von Hangman unabhänig von der GUI. Welche Aktionen gibt es bei Hängman? Jede wird als eigene kurze Funktion implementiert.
Eine Funktion ist, aus einem Wort und den geratenen Buchstaben den Lückentext zu bilden:

Code: Alles auswählen

def generate_hidden_text(word, guessed_characters):
    return " ".join(c if c in guessed_characters else "_" for c in word.upper())

[__blackjack__]

Kommentare sollen dem Leser einen Mehrwert über den Code geben. Faustregel: Kommentare beschreiben nicht *was* der Code macht, denn das steht da bereits als Code, sondern warum er das macht. Sofern das nicht offensichtlich ist. Offensichtlich ist in aller Regel auch was in der Dokumentation von Python und den verwendeten Bibliotheken steht.

Statt zu kommentieren das nicht alle Farbkonstanten gebraucht werden, würde man eher keine unbenutzten Konstanten definieren.

Bei den Farben sticht `Lila` als Name hervor weil diese Farbe im Gegensatz zu den anderen deutsch statt englisch benannt ist. Man sollte generell eher bei einer natürlichen Sprache bei den Namen bleiben. Englisch bietet sich da an, weil der Rest der Sprache und Bibliotheken ebenfalls englische Namen verwenden.

Beim öffnen von Textdateien sollte man immer explizit die Kodierung angeben. Sonst kann es passieren das diese falsch ”geraten” wird.

Statt eine Schleife zu schreiben die alle Elemente eines iterierbaren Objekts an eine anfangs leere Liste anzuhängen, kann man auch einfach `list()` aufrufen:

Code: Alles auswählen

items = []
for item in some_iterable:
    items.append(item)

# =>

items = list(some_iterable)

Das ``random.randint(0, 99)`` geht fest davon aus, dass in der Datei genau 100 Worte sind. Das sollte man besser von der tatsächlichen Anzahl abhängig machen. Und es gibt dafür auch schon eine fertige Funktion: `random.choice()`.

`furtherProcessing()` ist als Name recht generisch. Falls man Probleme hat einen *passenden* Namen für etwas zu finden, kann das auch oft ein Hinweis darauf sein, dass man selbst das Problem oder die Lösung noch nicht ganz verstanden hat, oder das man in einer Funktion oder Methode oder auch in einer Klasse Sachen zusamman packt die so gar nicht zusammen gehören, sondern sinnvollerweise anders aufgeteilt werden sollten.

`Buchstaben` und `Wortstand` haben als Attribute eigentlich nichts auf dem Dialog zur Auswahl eines Wortes zu suchen. Das gehört bereits zum Spiel. Wobei `Wortstand` redundant ist — das kann man während des Spiels ja jederzeit aus dem Wort und der Eingabe berechnen.

Das `Hangman` von `Sprite` erbt macht keinen Sinn. Diese Eigenschaft wird nirgends im Programm verwendet. Man kann diese Vererbung einfach entfernen ohne das sich am Programmablauf etwas ändert.

Pygame ist nicht darauf ausgelegt das nach `pygame.quit()` noch etwas mit Pygame gemacht wird. Das kann funktionieren, tut es aber nicht auf allen Systemen.

`initGraphics()` ist keine Methode, sollte deshalb auch nicht als solche in einer Klasse definiert werden. Ausserdem macht es keinen Sinn die Bilder mehr als einmal zu laden. Der Aufruf dieser Funktion gehört also vor die Schleife in der `play()` wiederholt aufgerufen wird.

`Bilder` ist nicht wie kommentiert die Ausgabe von `initGraphics()`, sondern der Rückgabewert.

`event.type` kann nur einen Wert haben, das sind also keine zwei ``if`` sondern ein ``if`` und ein ``elif`` für den zweiten Test.

Die Eingaben werden IMHO falsch verarbeitet. Denn man kann mehrere Zeichen eingeben, aber mehr als ein Zeichen ist immer ein Fehler. Das macht keinen Sinn. Denn dann kann man sich das mit der Eingabetaste sparen und einfach bei jedem getippten Buchstaben sofort testen ob der im Wort enthalten ist.

Das `self.Eingabe` auch für Ausgaben benutzt wird ist verwirrend.

`showMessage()` ist eine Funktion und keine Methode und der Name ist falsch, denn diese Funktion zeigt nichts an sondern rendert einen Text als `Surface`. Zum Anzeigen muss man das erst noch auf den Bildschirm blitten.

Man muss auch nicht jedes Zwischenergebnis an einen Namen binden.

Ungetesteter Zwischenstand:

Code: Alles auswählen

#!/usr/bin/env python3
import random
import tkinter as tk

import pygame as pg

BLUE = (0, 0, 50)
PURPLE = (255, 0, 255)


class ChoseWordDialog(tk.Tk):
    def __init__(self):
        tk.Tk.__init__(self)
        self.title("Anfang")

        self.wort = None

        tk.Label(self, text="Wie willst du spielen?").pack()

        frame = tk.Frame(self)
        tk.Button(
            frame, text="Eigenes Wort einreichen", command=self.use_own_word
        ).pack(side=tk.LEFT)
        tk.Button(
            frame, text="Zufallswort", command=self.use_random_word
        ).pack(side=tk.LEFT)
        frame.pack()

        self.eingabe = tk.Entry(self)
        self.eingabe.pack(expand=True, fill=tk.X)

    def use_own_word(self):
        self.set_result_and_quit(self.eingabe.get())

    def use_random_word(self):
        with open("Wörter_Liste.txt", "r", encoding="utf-8") as file:
            self.set_result_and_quit(random.choice(list(file)).strip())

    def set_result_and_quit(self, wort):
        self.wort = wort.upper()
        self.quit()


def load_images():
    return [
        pg.transform.scale(
            pg.image.load(
                f"/Applications/Python 3.11/Hangman/Hangman_Projekt/Hangman_Bilder/Hang{i}.jpeg"
            ),
            (350, 450),
        )
        for i in range(1, 10)
    ]


def render_text(text, colour):
    return pg.font.SysFont("arial", 48).render(text, True, colour)


class Hangman:
    def __init__(self):
        pg.init()
        self.fenster = pg.display.set_mode((1200, 600))
        self.bilder = load_images()
        self.ausgabetext = ""
        self.wort_ausgabe = ""
        self.running = False

        self.finished = False
        self.fehleranzahl = None
        self.wort = None
        self.geratene_buchstaben = None

    def process_event(self, event):
        if event.type == pg.QUIT:
            self.running = False
        elif event.type == pg.KEYDOWN and not self.finished:
            eingabe = event.unicode.upper()
            if eingabe not in self.wort:
                self.fehleranzahl += 1
            else:
                self.geratene_buchstaben.add(eingabe)

    def check_for_end(self):
        if self.fehleranzahl > 7:
            self.ausgabetext = "Das richtige Wort war: "
            self.wort_ausgabe = self.wort
            return True

        if (
            all(
                buchstabe in self.geratene_buchstaben
                for buchstabe in self.wort
            )
            and not self.finished
        ):
            self.ausgabetext = random.choice(
                [
                    "Du bist ein wahrer Meister!",
                    "Knapp dem Strang entkommen!",
                    "Yeah, Hinrichtung verschoben!",
                    "Du hast es geschafft!",
                    "Du hast es verdient!",
                    "Ein Wunder ist geschehen!",
                    "Glückwunsch!",
                ]
            )
            return True
        
        return False

    def update(self):
        self.fenster.fill(BLUE)
        self.fenster.blit(self.bilder[self.fehleranzahl], (20, 20))
        self.fenster.blit(
            render_text(
                " ".join(
                    buchstabe if buchstabe in self.geratene_buchstaben else "_"
                    for buchstabe in self.wort
                ),
                PURPLE,
            ),
            (20, 500),
        )
        self.fenster.blit(render_text(self.ausgabetext, PURPLE), (400, 200))
        self.fenster.blit(render_text(self.wort_ausgabe, PURPLE), (400, 300))
        pg.display.update()

    def play(self, wort):
        self.ausgabetext = ""
        self.wort_ausgabe = ""
        self.wort = wort
        self.geratene_buchstaben = set()
        self.fehleranzahl = 0
        self.finished = False
        self.running = True
        while self.running:
            for event in pg.event.get():
                self.process_event(event)
            self.finished = self.check_for_end()
            self.update()

    def run(self):
        try:
            while True:
                choose_word_dialog = ChoseWordDialog()
                choose_word_dialog.mainloop()
                if not choose_word_dialog.wort:
                    break

                self.play(choose_word_dialog.wort)
        finally:
            pg.quit()


def main():
    Hangman().run()


if __name__ == "__main__":
    main()

`Hangman` hat zu viele Attribute, aber da ist ja auch noch die Spiellogik und die GUI vermischt in der Klasse.

[e.s.t.e.senor]

Hallo Sirius3, hallo __blackjack__,
Danke für die Hinweise und dass ihr euch dafür Zeit genommen habt, mir Feedback zu geben. Ich habe gemerkt, dass ich ein sehr falsches Verständnis von Klassen habe und ich werde mich auf jeden Fall noch mal in Ruhe hinsetzen und mich dazu belesen.
Mein Code ist sehr unübersichtlich und unstrukturiert und ich werde ebenfalls versuchen, ihn mit euren Tipps zu verbessern, ganz zu Schweigen von euren anderen Kritikpunkten, mit denen ich mich noch beschäftigen will. Ich möchte mich ebenfalls für die konkrete Verbesserung von dir __blackjack__ bedanken. Ihr habt mich auf jeden Fall gut weitergebracht und ich habe diese Woche auf jeden Fall viel Arbeit damit.
PS: Darf ich fragen wie ihr euch das Wissen zu objektorientierten Programmieren angeeignet habt? Habt ihr euch ein Buch zugelegt oder mit Online-Tutorials gelernt? Oder sonstiges?

[__blackjack__]

OOP lernen zieht sich bei mir über einen längeren Zeitraum und ich kann da nicht *eine* Quelle nennen. Das fing mit Turbo Pascal 5.5 in der Schule an, wo ich die entsprechende Syntax gelernt, aber den Sinn noch nicht verstanden hatte. Dann kam irgendwann Java in der Uni mit entsprechenden Büchern zum lernen von Java und damit auch dessen Geschmacksrichtung von OOP. Der Rest zum Verständnis dann aus vielen unterschiedlichen Quellen. Unter anderen auch verschiedene Programmiersprachen. Also zum Beispiel Smalltalks Geschmacksrichtung von OOP, oder auch Io das nur Objekte aber keine Klassen kennt → Prototyp-OOP. Um besser zu verstehen was es bedeutet das OOP keine Spracheigenschaft ist, hilft auch sich OOP mal in Sprachen anzuschauen die keine spezielle Unterstützung dafür haben. Also zum Beispiel Bibliotheken wie Gtk & Co in C.

[snafu]

OOP hat wie so vieles beim Programmieren den syntaktischen und den semantischen Aspekt. Soll heißen: man kann Klassen so schreiben, dass Python sie anstandslos ausführt. Das bedeutet aber nicht, dass die Klasse in ihrem Aufbau und dem ihr zugewiesenen Aufgabenbereich immer sinnvoll ist. Das ist dann der semantische Teil, der auf uns als Menschen abzielt und von der Programmiersprache nicht durch Fehlermeldungen übernommen werden kann.

[e.s.t.e.senor]

Ok, interessant. Danke für die Ratschläge!

[/me]

Ein weiterer Hinweis: Es gibt den offiziellen Style Guide for Python Code. Ich würde dringend empfehlen, dass man sich als Entwickler daran orientiert. Einige relevante Punkte daraus wurden hier übrigens bereits genannt.