Bool'scher Test funktioniert nicht wie gewünscht

[CasualCoding.org]

Hallo Forum,

ich arbeite gerade an einem kleinen Hangman-Spiel für die Konsole. Ich habe eine Klasse für das Ratewort, die bisher folgendermaßen aussieht:

Code: Alles auswählen

class Puzzle:
    def __init__(self, new_word):
        self.word = new_word
        self.letters = len(self.word)
        self.solution = list(self.word)
        self.current = ['*'] * self.letters

    def evaluate_guess(self, guess):
        return (guess in self.solution)

Im Hauptprogramm erstelle ich ein Ratewort durch:

Code: Alles auswählen

# word_pool ist eine Liste mit allen Ratewörtern (in Großbuchstaben), rd ist ein zufälliger Index
puzzle = Puzzle(word_pool[rd])

Die Buchstabeneingabe des Nutzers realisiere ich über folgende Funktion (eigenständige Funktion, gehört nicht zur Klasse Puzzle):

Code: Alles auswählen

def get_user_guess():
    print()
    guess = input('Buchstabe: ')
    return guess.upper()

In der Hauptspielschleife habe ich nun folgenden Code:

Code: Alles auswählen

guess = get_user_guess()
valid_guess = puzzle.evaluate_guess(guess)
# Folgende Ausgabe nur zu Testzwecken:
print(valid_guess)

Und hier gibt mir die Testausgabe für jeden Buchstaben immer False zurück. Wahrscheinlich habe ich einfach nur einen ganz blöden Denkfehler, aber ich sehe ihn gerade nicht. Ein kleiner Tipp?

[Hyperion]

Ein kleiner Tipp?

Ja, man sollte *nie* so viel Code schreiben, ohne die Einzelteile zwischenduch abgetestet zu haben

Lass Dir doch mal in der ``evaluate_guess`` die Werte ausgeben - vermutlich gibt es da einen Wert, der nicht Deinen Erwartungen entspricht.

Nebenbei:
- ``letters`` ist kein guter Name für die *Länge* eines Wortes. ``word_count`` o.ä. wäre da besser. Fragt sich auch: Wotzu? Du kannst die Länge ja leicht mittels ``len`` ermitteln...

- Anstelle eines zufälligen Index, kannst Du auch ``random.choice`` nutzen - analog zum Iterieren über Schleifen drückt das viel besser aus, wie Du von der Liste von Wörtern ein zufälliges auswählst.

Edit: Vielleicht ist das als Inspiration nützlich für Dich

[BlackJack]

@SolitaryMan: Lass Dir doch einfach mal `puzzle.solution` ausgeben. Vielleicht enthält das ja nicht das was Du denkst‽

Die `Puzzle`-Objekte haben zu viele Attribute. `self.letters` ist schlecht benannt weil es weder Buchstaben noch Briefe enthält sondern eine Anzahl. Aber das liesse sich auch jederzeit schnell aus `self.word` berechnen. `self.solution` ist redundant weil es genau das selbe enthält wie `self.word`. Man könnte den Test auch problemlos auf `self.word` machen. `current` ist im Grunde auch überflüssig wenn man sich ein `set()` mit richtig geratenen Buchstaben halten würde, könnte man die Darstellung des aktuellen Standes daraus einfach berechnen.

[CasualCoding.org]

Erstmal danke für eure Tipps und Hinweise.

Der Tipp, mir einfach mal self.solution ausgeben zu lassen, hat mein Problem auch gelöst - auf die Idee hätte ich kommen können, sorry. Ich habe tatsächlich vergessen, die Lösungsworte beim Erstellen von word_pool in Großbuchstaben umzuwandeln. Bei einer vorhergehenden rudimentären Fassung war das nicht nötig, weil ich da mit einem festen, hardcodierten Lösungswort experimentiert habe. Beim Umsetzen meines jetzigen Konzeptes habe ich diese Umwandlung schlicht vergessen.

Ich könnte zwar die Lösungsworte auch direkt in Großbuchstaben in der Datei hinterlegen, aber langfristig soll der Nutzer auch die Möglichkeit haben, selbst Lösungsworte einzugeben und/oder abzuspeichern, deshalb die Umwandlung in Großbuchstaben erst beim Erstellen des jeweiligen Lösungswortes.

Eure anderen Anstöße waren viel Holz für den Anfang, ich überdenke das in den nächsten Tagen mal. Nur zu der Variablen "self.letters" (die ich umbenennen werde) schon eine Stellungnahme: Ich möchte die gern als Attribut drin haben, weil ich einige Erweiterungen des Spiels im Hinterkopf habe, für die ich die Länge brauche. Und da ist es mir zu unhandlich, jedes Mal mit len() neu zu berechnen.

[BlackJack]

@SolitaryMan: `len()` aufzurufen ist nicht wirklich aufwändig. Ausserdem könnte man es in ein berechnetes Attribut (`property()`) kapseln, dann hat man einerseits ein Attribut wie jetzt, andererseits kann man sicher sein, dass die beiden Werte nie inkonsistent werden können.

[EyDu]

Ich möchte die gern als Attribut drin haben, weil ich einige Erweiterungen des Spiels im Hinterkopf habe, für die ich die Länge brauche. Und da ist es mir zu unhandlich, jedes Mal mit len() neu zu berechnen.

Dann mach daraus einfach ein Property welches dir die Länge liefert:

Code: Alles auswählen

>>> class Spam(object):
...     def __init__(self, word):
...         self.word = word
...     @property
...     def length(self):
...         return len(self.word)
... 
>>> s = Spam("test")
>>> s.length
4
>>> s.word = "FooBarBaz"
>>> s.length
9

Es ist generell eine schlechte Idee Information doppelt irgendwo zu speichern, da dann auch immer an zwei Stellen aktualisiert werden muss. Wenn du ein neues Wort setzt, dann musst du auch immer die Länge mit berechnen und darfst dies nirgends vergesen. Früher oder später wirst du es aber mal übersehen und dir wunderschön schwer zu findene Fehler einfangen.

Davon abgesehen ist "letters" noch immer ein sehr schlechter Name, da man darin Buchstaben erwartet und nicht die Länge eines Worts.

[BlackJack]

@SolitaryMan: Ich hätte die Klasse wohl so geschrieben:

Code: Alles auswählen

#!/usr/bin/env python

class Word(object):
    def __init__(self, word):
        self.word = word
        self.distinct_letters = set(self)
        self.guessed_letters = set()

    def __iter__(self):
        return iter(self.word)

    def __str__(self):
        return ''.join(c if c in self.guessed_letters else '*' for c in self)

    def is_solved(self):
        return self.distinct_letters == self.guessed_letters

    def guess(self, character):
        result = (
            character in self.distinct_letters
            and character not in self.guessed_letters
        )
        if result:
            self.guessed_letters.add(character)
        return result


def main():
    word = Word('PYTHON')
    errors = 0
    max_errors = 5
    while not word.is_solved():
        print word
        if not word.guess(raw_input('Buchstabe: ').upper()):
            errors += 1
            print 'Fehler: {0} von ({1})'.format(errors, max_errors)
            if errors == max_errors:
                print 'Verloren!'
                break
        print
    else:
        print 'Gewonnen!'
    print word


if __name__ == '__main__':
    main()

[CasualCoding.org]

Danke danke danke für die vielen guten Anregungen! Insbesondere bei deinem Snippet, BlackJack, werde ich mich kräftig bedienen - falls erlaubt!?

[CasualCoding.org]

Mit einjähriger Verspätung (weil ich das Projekt wieder ausgegraben habe und daran weiterarbeiten will) noch eine Verständnisfrage an BlackJack:

Wieso kannst du in den Zeilen 6 und 13 deines Beispielcodes einfach nur auf 'self' Bezug nehmen, statt auf 'self.word'? 'self' ist doch das komplette Objekt, oder nicht? Woher wissen die set()-Methode und die comprehension, dass du die Buchstaben aus 'self.word' meinst?

[EyDu]

Weil Word eine __iter__-Methode hat. Diese wird aufgerufen, wenn der set-Aufruf und der Generatorausdruck einen Iterator brauchen. Die __iter__-Methode liefert dann den Iterator für ``self.word``.

[Hyperion]

@EyDu: Psst, die Frage war doch an BlackJack gerichtet!

[EyDu]

Oh. Hmm. Das ist jetzt blöd gelaufen. Dann ignorieren am besten jetzt alle meinen Beitrag und wir warten auf die richtige Antwort von BlackJack.

[BlackJack]

Na jut, ich verweise dann mal auf die Antwort von EyDu.

[CasualCoding.org]

Ah, verstehe! Danke euch. Da heißt es wohl 'Nachsitzen' in Sachen Iteratoren (ich gebe zu, ich habe das entsprechende Kapitel damals in meinem Lehrbuch nur oberflächlich überflogen).