Iyfjky

Question

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.')

score 5 · Accepted Answer · 2018-08-25 21:37:44Z

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()

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(). — Aug 25 at 20:21
@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 — Aug 25 at 20:36
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). — Aug 25 at 20:45
@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? — Aug 25 at 20:52
@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 — Aug 25 at 21:33

score 2 · Answer 2 · 2018-08-26 01:00:40Z

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()

Nice. Is while not guessed >= correct_letters: equivalent to while guessed < correct_letters:? — Aug 26 at 12:28
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. — Aug 26 at 14:49
I might go for something easier for other readers to comprehend (including my future self :p) - maybe while not correct_letters.issubset(guessed):? — Aug 26 at 15:00
I might actually go for the really explicit and easy to understand while any(letter not in guessed for letter in correct_letters):. — Aug 26 at 15:08
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. — Aug 26 at 16:13

score 5 · Accepted Answer · 2018-08-25 21:37:44Z

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()

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(). — Aug 25 at 20:21
@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 — Aug 25 at 20:36
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). — Aug 25 at 20:45
@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? — Aug 25 at 20:52
@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 — Aug 25 at 21:33

score 2 · Answer 4 · 2018-08-26 01:00:40Z

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()

Nice. Is while not guessed >= correct_letters: equivalent to while guessed < correct_letters:? — Aug 26 at 12:28
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. — Aug 26 at 14:49
I might go for something easier for other readers to comprehend (including my future self :p) - maybe while not correct_letters.issubset(guessed):? — Aug 26 at 15:00
I might actually go for the really explicit and easy to understand while any(letter not in guessed for letter in correct_letters):. — Aug 26 at 15:08
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. — Aug 26 at 16:13

Search This Blog

Iyfjky

Very basic Hangman game in Python

2 Answers
2

Your Answer

Post as a guest

2 Answers
2

2 Answers
2

Post as a guest

Comments

Post a Comment

Popular posts from this blog

Long meetings (6-7 hours a day): Being â€œbabysatâ€ by supervisor

What does second last employer means? [closed]

One-line joke

Category

Random preview

Very basic Hangman game in Python

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Post as a guest

2 Answers 2

2 Answers 2

Sign up or log in

Post as a guest

Post as a guest

Sign up or log in

Post as a guest

Sign up or log in

Post as a guest

Sign up or log in

Post as a guest

Comments

Post a Comment

Popular posts from this blog

Long meetings (6-7 hours a day): Being â€œbabysatâ€ by supervisor

What does second last employer means? [closed]

One-line joke

2 Answers
2

2 Answers
2

2 Answers
2

Long meetings (6-7 hours a day): Being â€œbabysatâ€ by supervisor