4
\$\begingroup\$

I'm trying to learn Python, so I made this project. I think there are problems with the code, but I don't know how to improve it.

import os
import random
clear = lambda: os.system('cls')
def game():
    def printOutCorrectGuess():
        print('Correct! There is/are ', lettersInWord, 'of this letter in this word.')
    def printOutStars():
        print('Currently revealed of the word: ', ''.join(stars))
    def printOutVictory():
        print('You won!')
        print('The word was ', theWord)
        print('Press enter to quit...')
    def printOutWords():
        print('You guessed these letters : ', words)
    def printOutLifes():
        print('You have ', lives, ' more lives.')
    print("Let's start guessing!")
    correctLetters = 0
    lives = 10
    words = []
    stars = list('*' * len(theWord))
    while True:
        guess = str(input('Please give me a letter: '))
        if len(guess) > 1 or len(guess) < 1:
            clear()
            print('I need 1 and letter, thanks.')
            printOutStars()
        elif guess in words:
            clear()
            print('You already wrote this letter.')
            printOutStars()
        elif guess in theWord:
            theStart = 0
            theEnd = 1
            lettersInWord = 0
            letterPosition = []
            for i in range(len(theWord)):
                tempWords = theWord.find(guess, theStart, theEnd)
                theStart = theStart + 1
                theEnd = theEnd + 1
                if tempWords >= 0:
                    lettersInWord = lettersInWord + 1
                    letterPosition.append(i + 1)
                    stars[i] = guess
            correctLetters = correctLetters + lettersInWord
            if correctLetters == len(theWord):
                clear()
                printOutVictory()
                if input() == '':
                    break
                else:
                    break
            clear()
            printOutCorrectGuess()
            printOutStars()
        else:
            lives = lives - 1
            if lives == 0:
                clear()
                print('You lost.')
                print('The word was: ', theWord)
                print('Press enter to quit the game')
                if input() == '':
                    break
                else:
                    break
            clear()
            printOutLifes()
            printOutStars()
        if guess not in words and len(guess) == 1:
            words.append(guess)
        printOutWords()
def welcome():
    print('Now we will play the classic Hangman game, but for this time without drawing it.')
    print('1 or 2 player game mode? Write 1 or 2 please.')
welcome()
while True:
    try:
        gameMode = int(input('INPUT: '))
    except ValueError:
        print('Write 1 or 2 please.')
        continue
    if gameMode == 1:
        words = ['letters', 'sloppy', 'bedroom', 'jazzy', 'discovery', 'wistful', 'unadvised', 'help', 'line', 'shake', 'mend', 'time', 'attempt', 'dare', 'straw', 'destroy', 'health', 'shiny']
        theWord = random.choice(words)
        clear()
        game()
        break
    elif gameMode == 2:
        theWord = str(input('Please write here the word: '))
        clear()
        game()
        break
    elif gameMode < 1 or gameMode > 2:
        print('Write 1 or 2 please.')
\$\endgroup\$
5
\$\begingroup\$

Brand new to programming myself, but here's what I've got for you.

This is just different formatting to not call print so many times:

def printOutVictory():
    print('You won!')
    print('The word was ', theWord)
    print('Press enter to quit...')

Alternate format:

# snake_case over camelCase in python
def print_out_victory(): 
    message = "You won!\nThe word was " + theWord
    message += "\nPress enter to quit..."
    print(message)

You could clean up your main body by creating game_mode_1 and game_mode_2 outside of main and calling them for your if and elif and then just reduce your last elif to an else statement:

def game_mode_1():
    words = ['letters', 'sloppy', 'bedroom', 'jazzy', 'discovery', 'wistful', 'unadvised', 'help', 'line', 'shake', 'mend', 'time', 'attempt', 'dare', 'straw', 'destroy', 'health', 'shiny']
    theWord = random.choice(words)
    clear()
    game()
    break

def game_mode_2():
    theWord = str(input('Please write here the word: '))
    clear()
    game()
    break

You would then have:

#using game_mode in place of gameMode, snake_cases again
def main():
    welcome()
    while true:
        try:
            game_mode = int(input('INPUT: '))
        except ValueError:
            print('Write 1 or 2 please.')
            continue

        if game_mode == 1:
            game_mode_1()

        elif game_mode == 2:
            game_mode_2()

        else:
            print("Write 1 or 2 please.")

Furthermore you could reduce main by creating user_selection

def user_selection():
    while true:
        try:
            game_mode = int(input('INPUT: '))
        except ValueError:
            print('Write 1 or 2 please.')
            continue

        if game_mode == 1:
            game_mode_1()

        elif game_mode == 2:
            game_mode_2()

        else:
            print("Write 1 or 2 please.")

And then your main would look like this

def main():
    welcome()
    user_selection()

Then run main adding if __name__ == '__main__': guard, this will prevent main from running if you import this script into another script

if __name__ == '__main__':
    main()
\$\endgroup\$
  • \$\begingroup\$ I think I'd return the function to be called from user_selection like game_function = game_mode_1 ... return game_function then call that from main like run_game = user_selection() ... run_game(). \$\endgroup\$ – Jonathan Allan Aug 25 '18 at 20:21
  • \$\begingroup\$ @JonathanAllan so you mean you would create a function that returns the game mode the user selects then apply that to a function that executes the selected game mode, if i understand properly , then main would contain a function to request the game mode and then a function that runs that selected mode, hmm I think i got it can update my answer \$\endgroup\$ – vash_the_stampede Aug 25 '18 at 20:36
  • \$\begingroup\$ I'd return the game_mode_* function from the mode selection function rather than call it inside the mode selection function. (Or I might do what you have and call it run_game instead of user_selection because that is what it does). \$\endgroup\$ – Jonathan Allan Aug 25 '18 at 20:45
  • \$\begingroup\$ @JonathanAllan Yep we're on the same page here, if we took that route then we would have to add code to handle a non 1 or 2 value in a loop inside the mode selection since currently its set to be handled by else after the game mode is set. Correct? \$\endgroup\$ – vash_the_stampede Aug 25 '18 at 20:52
  • 1
    \$\begingroup\$ @JonathanAllan Yes I seen how you handled that, and agree that would work well! Unfortunately the edit was rejected by others before me, I'm still just learning how to program and use this site, so I don't know how that works \$\endgroup\$ – vash_the_stampede Aug 25 '18 at 21:33
2
\$\begingroup\$

A couple of high-level comments:

  • You have many places where you can pull things out of the conditionals to avoid repeating yourself (DRY principle)
  • I would look to pass values into functions vs. using global variables.
  • You can use set()s to simplify a lot of the checking
  • The print functions can be inlined after the refactoring because they only occur in one place
  • Some of the variable names are misleading and it is recommended by pep8 to use lower_case instead of camelCase for variable names

Updated code:

import os
import random

You might want to keep the dictionary in a file and read it in vs. hard coding it:

dictionary = ['letters', 'sloppy', 'bedroom', 'jazzy', 'discovery', 'wistful', 
              'unadvised', 'help', 'line', 'shake', 'mend', 'time', 'attempt',
              'dare', 'straw', 'destroy', 'health', 'shiny']

Don't recommend using lambda if you are just going to assign it to a value, this is equivalent to the def:

def clear():
    os.system('cls')  # Not portable

After refactoring you can eliminate all the inner print functions because the prints only happen in one location:

def game(the_word):
    print("Let's start guessing!")
    lives = 10

Using sets allow you to simplify many of the checks.
Instead of while True:, this condition checks that not all of the correct_letters are in guessed:

    correct_letters = set(the_word)
    guessed = set()

    while not guessed >= correct_letters:
        guess = input('Please give me a single letter: ')
        clear()

        if len(guess) != 1:   # Simplify check
            print('I need 1 letter, thanks.')
        elif guess in guessed:
            print('You already choose this letter.')
        else:
            guessed.add(guess)
            if guess in correct_letters:
                count = the_word.count(guess)
                verb = 'is' if count == 1 else 'are'
                print('Correct! There {} {} of this letter in this word.'.format(verb, count))
            else:
                lives -= 1
                if lives == 0:
                    print('You lost.')
                    break

Refactored out of each of the conditional expressions:

            print('You have {} more lives.'.format(lives))
            print('You guessed these letters :', guessed)

Just calculate the '***' on demand using a simple generator expression:

            print('Currently revealed of the word: ', ''.join(c if c in guessed else '*' for c in the_word))

The else: clause of a loop is only executed if the loop complete (i.e. no break) - which will only be true if the game is won:

    else:
        print('You Won.')

    print('The word was: ', the_word)

def welcome():
    print('Now we will play the classic Hangman game, but for this time without drawing it.')
    print('It can be played in 1 or 2 player game mode')

def main():
    run = True
    welcome()

    while run:
        while True:
            try:
                game_mode = int(input('Game Mode (1/2): '))
                if game_mode in [1, 2]:
                    break    # Break out if the correct input
            except ValueError:
                pass

        if game_mode == 1:
            the_word = random.choice(dictionary)
        else:
            the_word = str(input('Please write here the word: '))

Refactored out of the conditionals:

        clear()
        game(the_word)

Best to ask about another game outside of the actual game:

        run = input('Another game (Y/N): ') in 'yY'

This is the normal way to run a script, it allows other ways to invoke game() in the future:

if __name__ == '__main__':
    main()

Putting it all together:

import os
import random

dictionary = ['letters', 'sloppy', 'bedroom', 'jazzy', 'discovery', 'wistful',
              'unadvised', 'help', 'line', 'shake', 'mend', 'time', 'attempt',
              'dare', 'straw', 'destroy', 'health', 'shiny']

def clear():
    os.system('cls')

def game(the_word):
    print("Let's start guessing!")

    lives = 10
    correct_letters = set(the_word)
    guessed = set()

    while not guessed >= correct_letters:
        guess = input('Please give me a single letter: ')
        clear()
        if len(guess) != 1:
            print('I need 1 letter, thanks.')
        elif guess in guessed:
            print('You already wrote this letter.')
        else:
            guessed.add(guess)
            if guess in correct_letters:
                count = the_word.count(guess)
                verb = 'is' if count == 1 else 'are'
                print('Correct! There {} {} of this letter in this word.'.format(verb, count))
            else:
                lives -= 1
                if lives == 0:
                    print('You lost.')
                    break

            print('You have {} more lives.'.format(lives))
            print('You guessed these letters:', guessed)
            print('Currently revealed of the word:', ''.join(c if c in guessed else '*' for c in the_word))
    else:
        print('You won!')

    print('The word was ', the_word)

def welcome():
    print('Now we will play the classic Hangman game, but for this time without drawing it.')
    print('It can be played in 1 or 2 player game mode')

def main():
    welcome()
    run = True
    while run:
        while True:
            try:
                game_mode = int(input('Enter 1 or 2: '))
                if game_mode in [1, 2]:
                    break
            except ValueError:
                pass

        if game_mode == 1:
            the_word = random.choice(dictionary)
        else:
            the_word = str(input('Please write here the word: '))

        clear()
        game(the_word)
        run = input('Another game (Y/N): ') in 'yY'

if __name__ == '__main__':
    main()
\$\endgroup\$
  • \$\begingroup\$ Nice. Is while not guessed >= correct_letters: equivalent to while guessed < correct_letters:? \$\endgroup\$ – Jonathan Allan Aug 26 '18 at 12:28
  • \$\begingroup\$ Unfortunately not, e.g. {'a'} < {'b'} is False whereas not {'a'} >= {'b'} is True. If guessed only contained valid letters then you could but it includes invalid guesses. \$\endgroup\$ – AChampion Aug 26 '18 at 14:49
  • \$\begingroup\$ I might go for something easier for other readers to comprehend (including my future self :p) - maybe while not correct_letters.issubset(guessed):? \$\endgroup\$ – Jonathan Allan Aug 26 '18 at 15:00
  • \$\begingroup\$ I might actually go for the really explicit and easy to understand while any(letter not in guessed for letter in correct_letters):. \$\endgroup\$ – Jonathan Allan Aug 26 '18 at 15:08
  • \$\begingroup\$ Personally, I wouldn't use any - a) it is less efficient, b) set algebra is well defined. Note: I'm ambivalent to using the operator or method either works, e.g. issubset() is equivalent to <= and you could also use not guessed.issuperset(correct_letters) which I feels reads better (guessed is the changing variable). But thank you for the comments. \$\endgroup\$ – AChampion Aug 26 '18 at 16:13

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.