Csdrhrt

I have recently finished a course on Python 3 and I really learned a lot, but every time I write a script, it is messy and I often feel like there must be shorter, more efficient ways to write and structure my scripts.

So my question is: How could I rewrite the below script to be shorter and more efficient, and what advice would you guys give me to improve my coding skills in general (maybe a way of thinking about the program that will make it easier to define the structure of what my script should look like...)

I am really enthused about the possibilities that coding bring and I would really like to improve, so don't hesitate to be harsh with your comments ;)

Here below is one of the scripts I wrote. This one is a hangman game: (The word list for the game are being picked from a .txt file that lives in the same directory as my script)

import string

import random





class Main:



    def __init__(self):

        self.place_holder = 

        self.wrong_guesses = 10

        self.guesses = 

        with open('./words.txt', mode='r') as f:

            self.word = random.choice(f.readlines())

            self.word = self.word.strip()

            print(self.word, end='')

            return self.place_holders()



    def place_holders(self):

        # creates the placeholders for the word

        for char in self.word:

            if char != " ":

                self.place_holder.append(' _ ')

            elif char == " ":

                self.place_holder.append('   ')

        print("".join(self.place_holder))  # prints initial placeholder

        return self.is_letter()



    @staticmethod

    def guess():

        # Prompts the user for a guess

        while True:

            guessed_letter = input(str('Give a guess: ')).lower()

            if guessed_letter not in string.ascii_lowercase or len(guessed_letter) > 1:

                print(

                    'You have inputted more than one character or the character entered was not recognized.nPlease try again.')

            else:

                break

        return guessed_letter



    def is_letter(self):

        # finds out if the letter belongs to the word, and if so, places it on the right placeholder

        guessed_letter = self.guess()

        if guessed_letter in self.word and guessed_letter not in self.guesses:

            for i in range(len(self.word)):

                if guessed_letter == self.word[i]:

                    self.place_holder[i] = f' {self.word[i]} '

        elif guessed_letter in self.guesses:

            print('You already said that..n')

        else:

            self.wrong_guesses -= 1

            print(f'Sorry, missed.nYou have {self.wrong_guesses} guesses left.n')



        self.guesses.append(guessed_letter)

        print("".join(self.place_holder))  # prints the updated placeholder

        return self.is_over()



    def is_over(self):

        # Checks if the players has guessed the full word or if the player ran out of guesses

        if ' _ ' in self.place_holder and self.wrong_guesses > 0:

            self.is_letter()

        elif ' _ ' in self.place_holder and self.wrong_guesses == 0:

            print(f'Sorry, Game Over!nThe word to guess was: {self.word}')

            self.play_again()

        else:

            self.play_again()



    @staticmethod

    def play_again():

        # Prompts the player if he wants to play again or not

        if input('Do you want to play again? (y/n): ').lower().startswith('y'):

            Main()

        else:

            print('Fair enough.. Thanks for playing!')

            quit()





Main()

Your code structure is one of infinite recursion. Main() calls placeholder(), which calls is_letter() which calls is_over() which calls is_letter() (recursive), or play_again() which calls Main() (recursive)! Eventually, you will end up with a Stack Overflow if you play long enough!

What you want is a loop. Two loops, actually. Your program should be structured to operate something like:

play_again = True

while play_again:

    # initialize game state

    while not solved and guesses_left:

        # get guess

        # update game state

    play_again = ask_play_again()

No recursive calls are necessary.

You read all lines of the file to choose a random puzzle. When you play again, you again read all lines in the file. Perhaps you could read and store all the puzzles, and each time a game is played, you just select one randomly from the list.

Your placeholders are complicated, 3 character entities. They could just be single characters, and spaces could be added during printing.

self.placeholder = [ "_" if ch != " " else " "  for ch in self.word ]

To print:

print("".join(f" {ch} " for ch in self.placeholder))

Or simply:

print("", "  ".join(self.placeholder))

You should allow for punctuation in the puzzles. Like spaces, these should be directly shown, not converted to underscores.

 _  _  _  _  _  ’  _     _  _  _  _

Don't unconditionally call Main() from global scope - if someone else imports your file, you want to leave it up to them what should be executed. This is why you should use the if __name__ == '__main__' pattern that we see so often elsewhere.

self.guesses shouldn't be a list. Since you need fast lookup, even though it won't make a noticeable difference, you should be using a set.