ООП Палач Игра

Мы с дочерью пытаемся выучить Python и ООП. Мы написали игру с палачом и были бы очень признательны за отзывы. Мы идем в правильном направлении? Как мы могли это улучшить? Что-нибудь, что заставляет программиста Python пойти «ой»? Что-нибудь, что заставит съежиться эксперта по ООП? У нас возникают самые разные вопросы. Например, должны ли мы создавать экземпляры классов, когда есть только один экземпляр? Не стесняйтесь быть жестоким! Мы разместили это как ответ здесь:
https://replit.com/@scripta/Hangman#main.py

Изменил функцию вызова на класс Game согласно предложению ASM.

import random

class Word:

    def __init__(self):
        words = ("foxglove", "captain", "oxygen", "microwave", "rhubarb")
        self._word = random.choice(words)
        self.letters_in_word=set(self._word)

    @property
    def word(self):
        return self._word

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        progress_string= ""
        for char in self.word:
            if guesses.guessed(char):
                progress_string= progress_string+" "+char+" "
            else:
                progress_string= progress_string +" _ "
        return(progress_string)

    def guessed(self, guesses):
        letters_guessed = self.letters_in_word.intersection(guesses.guesses_made)
        if letters_guessed == self.letters_in_word:
            return True
        else:
            return False


class Guesses:

    def __init__(self):
        # guesses holds all guesses made (wrong or right)
        self.guesses_made = set()

    def guessed(self,char):
        if char in self.guesses_made:
            return True
        else:
            return False

    def record(self,guess,word):
        # All valid guesses (wrong or right) are added to the guesses set
        self.guesses_made.add(guess)
        if not guess in word.letters_in_word:
            Gallows.record_bad_guess()

    def made(self):
        # to sort the set of guesses made we need to cast it to a list
        guesses_list= list(self.guesses_made)
        guesses_list.sort()
        guesses_string=""
        #comma separate the guesses
        for char in guesses_list:
            guesses_string= guesses_string+char+","
        return guesses_string

class Gallows:

    _bad_guesses=0

    gallows_images = ("      n      n        n         n        n__________",
                      "      n |    n |      n |       n |      n_|________",
                      " _____n |    n |      n |       n |      n_|________",
                      " _____n |/   n |      n |       n |      n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |      n |       n |      n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |   O  n |       n |      n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |   O  n |   |   n |      n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"   n |      n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"\ n |      n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"\ n |  /   n_|________",
                      " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"\ n |  / \n_|________",)

    @classmethod
    def record_bad_guess(cls):
        cls._bad_guesses +=1

    @classmethod
    def hanged(cls):
        if cls._bad_guesses >= len(cls.gallows_images)-1:
            return True
        else:
            return False

    @classmethod
    def draw(cls):
        return cls.gallows_images[cls._bad_guesses]

class Game:

    def get_letter(self, guesses):
        while True:
            char= input("nPlease enter your guess: ")
            if len(char)<1:
                print ("You didn't choose a letter!")
            elif len(char)!=1:
                print ("One letter at a time!")
            elif not char.isalpha():
                print ("Alphabetic characters only!")
            elif guesses.guessed (char):
                print ("You already guessed that letter")
            else:
                break       
        return (char)

    def display_progress(self, target, gallows, guesses):
        print("n",target.progress(guesses)) 
        print(gallows.draw())
        print("nUsed: n",guesses.made())

    def play(self):
        target= Word()
        guesses= Guesses()
        gallows= Gallows()
        while True:
            guess= self.get_letter(guesses)
            guesses.record(guess,target)
            self.display_progress(target, gallows, guesses)
            if target.guessed(guesses):
                print("nYou win, well done")
                break
            if gallows.hanged():
                print ("nI win. The word was:",target.word)
                break

if __name__ == "__main__":
    game= Game()
    game.play()      

На первый взгляд довольно приятно. В основном у меня есть несколько более мелких проблем:

• Последовательность имеет значение для читателей, поэтому следует либо x = y, x= y
  или x =y (или любой вариант действительно). С использованием
  автоформат это, конечно, проще, чем делать это вручную, но в противном случае я бы предпочел первый вариант.
• Для words: если у вас есть последовательность, используйте list, а не tuple. Он понятнее и лучше подходит для цели, например, если вы добавляете новые элементы в список слов, он работает только с list, потому что
  tupleнеизменны.
• Отрицание in можно сделать с not in, который читается более естественно.
• Шаблон if x == y: return True else: return False не использует тот факт, что x == y уже возвращает логическое значение — его можно просто заменить на более короткое return x == y.
• У некоторых участников есть подчеркивание, у некоторых — нет — если вы не боретесь за очень жесткую границу между частными и общедоступными свойствами, я бы выбрал одно и придерживался его, если вы выберете подчеркивания, просто предоставьте
  @property аксессуары к ним при необходимости.
• Каждое изображение, вероятно, будет легче читать, если использовать многострочные строки ("""these ones""").
• Если вы хотите разобраться в понимании списка, madeспособ форматирования может быть немного проще с помощью strс join метод, например ",".join(self.guessed_made).
• sorted может быть проще, чем list с последующим sort, например sorted(self.guesses_made) делает то, что вы думаете.

Следующее, что я увидел структурно, это то, что Guesses использует Gallows
класс, а не объект, а в play_hangman.py он конструируется как объект и также используется как объект. Я бы выбрал один и придерживался его. Поскольку он предназначен быть более ООП, я бы отказался от методов класса и сделал бы его реальным объектом, который кажется более чистым и, по крайней мере, теоретически, в любом случае позволит лучше тестировать.

Wrt. общий Game class: конечно, если вы хотите сделать это в целях обучения, но на самом деле в Python, по крайней мере, тоже нормально иметь автономные функции. Конечно, вы можете делать такие вещи, как издевательство над всеми своими зависимостями, но даже просто имея play_game брать
target, guesses и gallows поскольку параметры все равно позволят вам это сделать. Изменить: И я не буду следить за этим редактированием, так как я уже печатал это раньше.

play_hangman.py:

import hangman


def get_letter(guesses):
    while True:
        char = input("nPlease enter your guess: ")
        if len(char) < 1:
            print("You didn't choose a letter!")
        elif len(char) != 1:
            print("One letter at a time!")
        elif not char.isalpha():
            print("Alphabetic characters only!")
        elif guesses.guessed(char):
            print("You already guessed that letter")
        else:
            break
    return char


def play_game():
    target = hangman.Word()
    guesses = hangman.Guesses()
    gallows = hangman.Gallows()
    while True:
        guess = get_letter(guesses)
        guesses.record(gallows, guess, target)
        print("n", target.progress(guesses))
        print(gallows.draw())
        print("nUsed: n", guesses.made())
        if target.guessed(guesses):
            print("nYou win, well done")
            break
        if gallows.hanged():
            print("nI win. The word was:", target.word)
            break


if __name__ == "__main__":
    play_game()

hangman.py:

import random


class Word:
    def __init__(self):
        words = ["foxglove", "captain", "oxygen", "microwave", "rhubarb"]
        self.word = random.choice(words)
        self.letters_in_word = set(self.word)

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        return "  ".join(char if guesses.guessed(char) else "_" for char in self.word)

    def guessed(self, guesses):
        letters_guessed = self.letters_in_word & guesses.guesses_made
        return letters_guessed == self.letters_in_word


class Guesses:
    def __init__(self):
        # guesses holds all guesses made (wrong or right)
        self.guesses_made = set()

    def guessed(self, char):
        return char in self.guesses_made

    def record(self, gallows, guess, word):
        # All valid guesses (wrong or right) are added to the guesses set
        self.guesses_made.add(guess)
        if guess not in word.letters_in_word:
            gallows.record_bad_guess()

    def made(self):
        return ",".join(sorted(self.guesses_made))


class Gallows:
    gallows_images = (
        "      n      n        n         n        n__________",
        "      n |    n |      n |       n |      n_|________",
        " _____n |    n |      n |       n |      n_|________",
        " _____n |/   n |      n |       n |      n_|________",
        " _____n "https://codereview.stackexchange.com/"n |      n |       n |      n_|________",
        " _____n "https://codereview.stackexchange.com/"n |   O  n |       n |      n_|________",
        " _____n "https://codereview.stackexchange.com/"n |   O  n |   |   n |      n_|________",
        " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"   n |      n_|________",
        " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"\ n |      n_|________",
        " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"\ n |  /   n_|________",
        " _____n "https://codereview.stackexchange.com/"n |   O  n "https://codereview.stackexchange.com/"\ n |  / \n_|________",
    )

    def __init__(self):
        self._bad_guesses = 0

    def record_bad_guess(self):
        self._bad_guesses += 1

    def hanged(self):
        return self._bad_guesses >= len(self.gallows_images) - 1

    def draw(self):
        return self.gallows_images[self._bad_guesses]

(Некоторые из этих вопросов уже были затронуты в уже опубликованном ответе, но я уже наполовину написал это, когда он появился.)

Это похоже на забавную программу, и в ней много интересного! Это ни в коем случае не полный обзор, но вот несколько небольших советов по улучшению вашего кода:

1. Переместите words переменная вне Word.__init__ метод.

Эта переменная — произвольная константа, которая не изменяется во время выполнения программы. Вам следует попытаться свести к минимуму степень, в которой такие постоянные значения погребены в середине вашего кода, чтобы было очевидно, где они находятся, и чтобы их было легко изменить, если вы обнаружите, что редактируете свой код в будущем. Также рекомендуется давать им имена ALL_CAPS, что является соглашением для постоянных значений, которые никогда не меняются. (Ваш Gallows.gallows_images атрибут следует переименовать как Gallows.GALLOWS_IMAGES по той же причине.)

Постоянные значения должны находиться либо в верхней части модуля в глобальном пространстве имен, либо в верхней части класса в пространстве имен классов. В этом случае я бы поставил его в начале класса:

import random

class Word:
    WORDS = ("foxglove", "captain", "oxygen", "microwave", "rhubarb")

    def __init__(self):
        self._word = random.choice(self.WORDS)
        self.letters_in_word = set(self._word)

2. Соблюдение Руководство по стилю PEP8 сделает ваш код более читабельным для других людей.

В частности, в вашем коде есть много мест, где у вас нет пробелов вокруг операторов, таких как + и =, который мне лично довольно трудно читать.

3. Использование F-строк — это более питонический способ объединения строк.

В вашей Word.progress метод, вы начинаете с пустой строки и добавляете к ней значения итеративно, используя + оператор. Вы можете сделать это более читаемым, используя += оператор для добавления символов в вашу строку и с помощью F-строк, чтобы уменьшить другие варианты использования + оператор:

class Word:
    # <--- snip --->

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        progress_string = ""
        for char in self.word:
            if guesses.guessed(char):
                progress_string += f" {char} "
            else:
                progress_string += " _ "
        return(progress_string)

Вы даже можете использовать тернарный оператор Python, чтобы еще больше сократить это:

class Word:
    # <--- snip --->

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        progress_string = ""
        for char in self.word:
            progress_string += f" {char} " if guesses.guessed(char) else " _ "
        return(progress_string)

Или используйте str.join метод и выражение генератора, чтобы сделать его однострочным!

class Word:
    # <--- snip --->

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        return "".join((f" {char} " if guesses.guessed(char) else " _ ") for char in self.word)

4. Ваши методы, которые возвращают bool значения можно упростить

Ваш Word.guessed можно упростить так:

class Word:
    # <--- snip --->

    def guessed(self, guesses):
        return bool(self.letters_in_word.difference(guesses.guesses_made))

Ваш Guesses.guessed можно упростить так:

class Guesses:
    # <--- snip --->

    def guessed(self, char):
        return char in self.guesses_made

И ваш Gallows.hanged также можно сократить до одной строки:

class Gallows:
    # <-- snip -->

    @classmethod
    def hanged(cls):
        return cls._bad_guesses >= (len(cls.gallows_images) - 1)

Мне немного непонятно, почему ты Gallows._bad_guesses Кстати, атрибут отмечен как частный. Я думаю, было бы хорошо, если бы это было публично Gallows.bad_guesses атрибут. Также немного запаха кода — иметь класс, в котором каждый метод является classmethod — для более полного рефакторинга вашего Gallows класс, я бы посмотрел на ответ @ferada.

Я также согласен с @ferada, что в вашем Game class, поскольку ни одна из его функций вообще не отслеживает внутреннее состояние. Если ваш код чисто процедурный, более питонично иметь ваши функции в глобальном пространстве имен, а не создавать объекты ради объектов (это сильно отличается от других языков, таких как Java).

Добро пожаловать в Code Review!

Если вы только начали учиться, это кажется прилично хорошим. Вот несколько комментариев в произвольном порядке:

Изменить: после публикации ответа я видел, что не все пункты ниже уже были рассмотрены в других ответах, написанных, когда я писал свой. Тем не менее, я оставлю это здесь, так как это может быть полезно

• В общем, читайте на PEP-8. Это общее руководство по стилю, которым широко пользуется сообщество. Например, вы не используете постоянный интервал:

# No slace on the left of the assignment
progress_string= progress_string+" "+char+" "
# Space on the left
self.guesses_made = set()

• Что касается дизайна ООП, Gallows плохо спроектирован. Он используется как король пространства имен, поскольку все методы являются методами классов, у него есть свойства класса, но он также инсталлирован. Это должен быть просто класс со своими свойствами и методами.

• Использование bools:

if condition:
  return True
else:
  return False

Может быть изменено на:

retun condition

Например, guessed метод в Guesses

• В Guesses#made метод можно упростить:

sorted_guesses = list(self.guesses_made).sort()
return ",".joint(sorted_guesses)

• Guesses — это просто оболочка для набора, поэтому, вероятно, это должен быть не класс, а просто свойство в игровом объекте. В Guesses#made метод может быть методом Game метод. В Guesses#record метод не должен зависеть от Gallows (на самом деле это не имеет смысла), так что это просто set#add.

• Для меня имена методов должны иметь значение сами по себе. Например, made ничего мне не говорит, если я не прочитаю это как Guesses#made. Так что его, вероятно, следует переименовать во что-то вроде made_guesses.

• word, guesses и gallows должно быть Gameсвойства, созданные при инициализации. Они также могут быть созданы с помощью метода, называемого init или reset который был вызван при инициализации. Это позволило бы перезапустить игру более явным образом (в настоящее время вы можете просто вызвать play дважды)