I'm a hobbist trying to become a professional software developer. I do have a lot interest in Data Science in general and my goal is to master Python before diging deeper into specific data science topics.

I created this game when I started studying object oriented programming. Thank you all in advance.

import os
import getpass

class Hangman:
    winner = False
    word = ''
    mask = ''
    mistakes = []

    def __init__(self, word):
        self.word = word
        self.mask = '_ ' * len(word)

    def make_move(self, letter):
        letter_match = self.word.find(letter)
        if letter_match >= 0:
            for i, w in enumerate(self.word):
                if w == letter:
                    new_mask = list(self.mask)
                    new_mask[i * 2] = letter
                    self.mask = ''.join(new_mask)


    def check_winner(self):
        if self.mask.find('_') < 0:
            print('You won!')
            self.winner = True

        return self.winner

    def check_looser(self):
        if len(self.mistakes) > 4:
            print('You loose! The word was %s' % self.word)

    def show_board(self):

    def play(self):
        while not self.winner:

            letter_input = input('Choose a letter: ').upper()

secret_input = getpass.getpass('Secret Word: ').upper()

game = Hangman(secret_input)
  • 1
    \$\begingroup\$ You should elaborate a bit more about your code, what decisions you made and why, to get better reviews. \$\endgroup\$ – πάντα ῥεῖ Nov 13 '19 at 17:32

It might be slightly cleaner and more performant to keep self.mask as a list, and just join it before printing:

# Basically what you had before, without the space
# Just "list multiplication" instead of "string multiplication"
self.mask = ['_'] * len(word)

. . .

if w == letter:
    mask[i] = letter

. . .

if '_' not in self.mask:
    print('You won!')

. . .

print(' '.join(self.mask))

Now there's less converting back and forth between strings and lists. This also saves you from needing to do i * 2. The spaces can just be added in using join before being printed, so you don't need to worry about them.

print(self.mistakes) currently prints out a normal list representation to the end user without any context. I'd probably prefix a message to that, and format it a bit before printing:

print(f"Already tried: {', '.join(self.mistakes)}")
# Already tried: R, Q

I'd probably get rid of self.winner and change how you're checking for winning. Notice what you're using it for: you assign it in check_winner, then return that same value that you just assigned, but never use the return value. I'd have check_winner just return the value, move the call to check_winner to after the call to make_move, and use the return value:

def check_winner(self):
    if '_' not in self.mask:
        print('You won!')
        return True

    return False

. . .

def play(self):
    won = False  # Just track this locally instead

    while not won:

        letter_input = input('Choose a letter: ').upper()

        won = self.check_winner()  # And assign it here

I'd also probably change check_loser (As a heads up, it's spelled "loser". "Looser" means that one thing is "less tight" than something else). I'd maybe have it return a bool saying whether or not the player lost, and just return from that function instead. As I noted in a previous review (at the bottom), quit can cause problems when testing code. Just returning from the main loop to allow control to pass back to the REPL is much cleaner.

|improve this answer|||||
  • \$\begingroup\$ Impressive! Thank you so much \$\endgroup\$ – Croves Nov 14 '19 at 16:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.