0
\$\begingroup\$

can anyone give me advice on how i can make my code more compact and efficient, and if you can, an implementation using OOP? Since, i just started learning Object Oriented Programming and it looks real hard.

def Hangman(): # for MENU
    print("H A N G M A N")
    while True:
        choice = input('Type "play" to play the game, "exit" to quit: ')
        if choice == "play":
            hint("option")
            play_game()
        if choice == "exit":
            break


def play_game(): # FOr game mechanics
    import random
    my_list = ['python', 'java', 'kotlin', 'javascript']
    # WORD CHOICES
    global lives, chosen_word, changed_word, changed_list, hidden_word, hidden_list
    lives = 8
    winner = False
    chosen_word = random.choice(my_list)
    changed_word = chosen_word[:]
    hidden_word = (len(chosen_word))*"-"
    hidden_list = list((len(chosen_word))*"-")
    changed_list = [char for char in changed_word]

    while lives != 0:
        guess = "-"
        print('\n', hidden_word, sep="") # gives the word every guess

        while True:
            guess = input("Input a letter: ")
            if len(guess) != 1:
                print("You should print a single letter\n")
                continue
            if guess != guess.lower() or not guess.isalpha():
                print("It is not an ASCII lowercase letter\n")
                continue
            break

        if guess not in changed_word: 
            if guess in chosen_word:
                print('You already typed this letter')
                continue

            else:
                print("No such letter in the word")
                lives -= 1
                hint("check_lives")

        else:
            letter_found_count = chosen_word.count(guess)

            for _ in range(letter_found_count):
                indexfound = changed_word.find(guess)
                hidden_list[indexfound] = guess
                changed_list[indexfound] = "-"
                hidden_word = "".join(hidden_list)
                changed_word = "".join(changed_list)

        if hidden_word == chosen_word:
            print("\n%s\nYou guessed the word!\nYou survived!" % hidden_word)
            winner = True
            break

    if not winner:
        print("You are hanged!")
    if hint_count > 1:
        print(f"Though you used {str(hint_count)} hints")
    print()


def hint(setting):
    if setting == "game":
        global chosen_word, changed_word, changed_list, hidden_word, hidden_list
        for _ in range(len(changed_list)):
            guess = changed_list[_]
            if guess != "-":
                letter_found_count = chosen_word.count(guess)
                for _ in range(letter_found_count):
                    indexfound = changed_word.find(guess)
                    hidden_list[indexfound] = guess
                    changed_list[indexfound] = "-"
                    hidden_word = "".join(hidden_list)
                    changed_word = "".join(changed_list)

                return changed_word, hidden_word, changed_list, hidden_list

    if setting == "option":
        global hint_flag, hint_count
        hint_count = 0
        while True:
            choice = input("With Hints? 'Yes' or 'No': ")
            if choice.lower() == "yes":
                hint_flag = True
                break
            elif choice.lower() == "no":
                hint_flag = False
                break

    if setting == "check_lives":
        if lives != 0:
            if lives < 5: # HINT CONDITION
                if hint_flag:
                    while True:
                        choice = input("For hint, answer: 'yes' or 'no': ")
                        if choice.lower() == "yes":
                            changed_word, hidden_word, changed_list, hidden_list = hint("game")
                            hint_count += 1
                            break
                        elif choice.lower() == "no":
                            break 

Hangman()
```
\$\endgroup\$
  • 1
    \$\begingroup\$ Asking us to change your code to use OOP is off-topic here, as your code does not work as you intended. Getting the correct output isn't the only requirement for things to be classed as 'working as intended'. Your code doesn't expose any classes and so does not follow OOP in any way. \$\endgroup\$ – Peilonrayz Apr 11 at 23:17
  • \$\begingroup\$ Related \$\endgroup\$ – Mast Apr 12 at 14:00
4
\$\begingroup\$

I can make some general suggestions. Having code completely rewritten is offtopic here though.


In play_game, you import random from within the function. PEP8 notes though that:

Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.

It's conceivable that you may want to have an import conditionally happen inside of a function if the import is expensive, and would only rarely be used. That certainly isn't the case here though.


changed_list = [char for char in changed_word]

This just creates a copy of changed_word in a verbose way. You would be better off doing what you did before:

changed_list = changed_word[:]

You use far too many global variables. For clarity, those variables making up the state of the game should be explicitly passed to each function that needs them. You could use a dataclass to hold the data, and then pass the state object around.

Having everything as global with complicate testing, and makes your program harder to understand.


You have a loop that's written in a confusing way:

while True:
    guess = input("Input a letter: ")
    if len(guess) != 1:
        print("You should print a single letter\n")
        continue
    if guess != guess.lower() or not guess.isalpha():
        print("It is not an ASCII lowercase letter\n")
        continue
    break

You continue in two branches, then break at the bottom. You might as well just use an elif and else to set it up so you only break if both checks were false:

while True:
    guess = input("Input a letter: ")
    if len(guess) != 1:
        print("You should print a single letter\n")
    elif guess != guess.lower() or not guess.isalpha():
        print("It is not an ASCII lowercase letter\n")
    else:
        break

"\n%s\nYou guessed the word!\nYou survived!" % hidden_word

In more modern Python, this can be written more cleanly using f-strings like you use later:

f"\n{hidden_word}\nYou guessed the word!\nYou survived!"

if setting == "check_lives":
    if lives != 0:
        if lives < 5: # HINT CONDITION
            if hint_flag:

You don't have an else case for any of those checks, so you might as well just and them:

if setting == "check_lives" and 0 < lives < 5 and hint_flag:

That will cut down on a lot of nesting.


list((len(chosen_word))*"-")

Would make a lot more sense as just:

len("chosen_word") * ["-"]

for _ in range(len(changed_list)):
    guess = changed_list[_]

Don't call something _ unless you don't use it. If you use it, give it a good, descriptive name. Here though, you should just iterate the word directly:

for letter in changed_list:
    guess = letter  # Which mostly gets rid of the need for guess
| improve this answer | |
\$\endgroup\$

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