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

Мы с дочерью пытаемся выучить 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()      

3 ответа
3

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

  • Последовательность имеет значение для читателей, поэтому следует либо 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]

  • Обратите внимание, что о вашем если только ты не [strive] для очень жесткой границы между частной и общественной собственностью, в Python такого нет.

    – Райндериен

  • 1

    Что касается вашего комментария о words будучи list, Я бы поспорил tuple лучше подходит для этой цели, поскольку используется фиксированное количество возможностей, которые не будут обновляться во время выполнения

    – м-алорда

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


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

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 дважды)

    Добавить комментарий

    Ваш адрес email не будет опубликован. Обязательные поля помечены *