12
\$\begingroup\$

Just completed a /r/dailyprogrammer challenge. Challenge #238 to be precise.

A Github repository with the wordbank.txt is available here.

import random

difficulty = {1: [4, 5, 6], 2: [7, 8],
              3: [9, 10, 11], 4: [12, 13],
              5: [14, 15]
              }
#Keys are the various word lengths for each difficulty (1-5).
acceptedWord = []
#Start a blank list, used in getWords()

def getWords():
    while True:
        try:
            level = int(input("What difficulty would you like? (1,2,3,4,5) "))
            break
        except (ValueError, KeyError):
            print("That is not a number")
    #Take a numerical input from the user, attempt to parse it, if it fails, carry on the loop.    
    num = random.choice(difficulty[level])
    #num (used to store word lengths) is a random choice from the available lengths for the selected difficulty.    
    with open('wordbank.txt', 'r') as file:
        #Open the file containing word-bank
        for x in map(str.upper, map(str.strip, file.readlines())):
            #loop over the stripped down words
            if len(x) == num:
                acceptedWord.append(x)
                 #If length of word is equal to random choice from suitable lengths, add it to a list.

    #Random index from total number of items in list.
    answers  = random.sample(acceptedWord, num)
    #picks a selection of the available words. Amount of words = word length.
    trueWord = answers[random.randint(0, len(answers)-1)]
    #trueWord = answer. Random item of the shortlisted avaliable words
    print('\n'.join(answers))
    #Prints hints.
    game(trueWord)


def game(trueWord):
    for x in range(0,5):
        #The user has 5 guesses
        print("Guesses Left: ", 5-x)
        userInput = input("Guess: ").upper()
        #All guesses/inputs are parsed to uppercase.
        userList = list(userInput)
        #List of the letters the user inputed
        trueList = list(trueWord)
        #List of letter of 
        if userInput == trueWord:
            print("Well done, you guessed correctly")
            getWords()
            #If the user enters the correct word, quit the program
        correctGuess = 0
        for item in list(userList):
            #for each letter the user inputed
            if item == trueList[userList.index(item)]:
                #if the letter is in the same position as the answer
                correctGuess += 1
                #increment the correct amount of letters by 1
        print(correctGuess, "out of ", len(trueList), "correct.")
    print("Bad luck! The answer was: ", trueWord)
    getWords()

getWords()
\$\endgroup\$
10
\$\begingroup\$

Some obvious changes that I'd make:

  • rename all the variables / methods such that they follow the snake_case convention
  • remove all the obvious comments and rather add a docstrings into each function instead
  • add only one space before and after any operator (except if we're passing some arguments into a function / class)
  • after , you should have a space
  • the acceptedWord list should be defined within the getWords() function (the same goes for the difficulty dictionary). (or better yet, remove its declaration completely and use list comprehension instead)
  • you should add if __name__ == '__main__'
  • user_list is already a list so you don't have to specify the exact same thing here: for item in list(user_list).
  • use string formatting when you print something: this: print(correct_guess, "out of ", len(true_list), "correct.") will become this: print("{} out of {}".format(correct_guess, len(true_list)))
  • when you use with to open a file in read mode, you can omit r as that's passed by default.

Reviewed code:

import random


def get_words():
    """Docstring goes here"""
    difficulty = {1: [4, 5, 6], 2: [7, 8], 3: [9, 10, 11], 4: [12, 13], 5: [14, 15]}
    while True:
        try:
            level = int(input("What difficulty would you like? (1,2,3,4,5) "))
            break
        except (ValueError, KeyError):
            print("That is not a number")

    num = random.choice(difficulty[level])

    with open('wordbank.txt') as file:
        accepted_word = [x for x in map(str.upper, map(str.strip, file.readlines())) if len(x) == num]

    answers = random.sample(accepted_word, num)
    true_word = answers[random.randint(0, len(answers) - 1)]
    print('\n'.join(answers))
    game(true_word)


def game(true_word):
    """Docstring goes here"""
    for x in range(0, 5):
        print("Guesses Left: {}".format(5 - x))
        user_input = input("Guess: ").upper()

        user_list, true_list = list(user_input),  list(true_word)
        if user_input == true_word:
            print("Well done, you guessed correctly")
            get_words()
        correct_guess = 0
        for item in user_list:
            if item == true_list[user_list.index(item)]:
                correct_guess += 1
        print("{} out of {}".format(correct_guess, len(true_list)))
    print("Bad luck! The answer was: {}".format(true_word))
    get_words()

if __name__ == '__main__':
    get_words()

More, I'd split the get_words() methods into two methods: get_user_level() and get_words():

def get_user_level():
    """Docstring goes here"""
    difficulty = {1: [4, 5, 6], 2: [7, 8], 3: [9, 10, 11], 4: [12, 13], 5: [14, 15]}
    while True:
        try:
            level = int(input("What difficulty would you like? (1,2,3,4,5) "))
            break
        except (ValueError, KeyError):
            print("That is not a number")

    return random.choice(difficulty[level])


def get_words():
    """Docstring goes here"""
    num = get_user_level()

    with open('wordbank.txt') as file:
        accepted_word = [x for x in map(str.upper, map(str.strip, file.readlines())) if len(x) == num]

    answers = random.sample(accepted_word, num)
    true_word = answers[random.randint(0, len(answers) - 1)]
    print('\n'.join(answers))
    game(true_word)
\$\endgroup\$
  • \$\begingroup\$ Just in very little addition: when making the if __name__ == '__main__' create a main-method and iterative call get_words. It might get messy after a few billion games when game calls get_words and vice versa, it's to much recursion imo \$\endgroup\$ – SchreiberLex Feb 28 '17 at 13:54
8
\$\begingroup\$
acceptedWord = []
#Start a blank list, used in getWords()

...

with open('wordbank.txt', 'r') as file:
    #Open the file containing word-bank
    for x in map(str.upper, map(str.strip, file.readlines())):
        #loop over the stripped down words
        if len(x) == num:
            acceptedWord.append(x)
             #If length of word is equal to random choice from suitable lengths, add it to a list.

This is far from efficient as you are reading the whole file at once, then stripping each line and capitalizing it before applying filtering. Instead, you may want to use the fact that file objects are iterable to retrieve lines one at a time, decide wether or not to keep it and then capitalize it:

def words_of_length(required_length, filename='wordbank.txt'):
    with open(filename) as words_file:
        for line in words_file:
            line = line.strip()
            if len(line) == required_length:
                yield line.upper()

Usage being accepted_words = list(words_of_length(num)).

You can still use some form of functional programming if you prefer:

def words_of_length(required_length, filename='wordbank.txt'):
    def right_size(word):
        return len(word) == required_length
    with open(filename) as words_file:
        return map(str.upper, filter(right_size, map(str.strip, words_file)))
\$\endgroup\$
7
\$\begingroup\$

Fundamental misunderstanding

The most significant problem with this code is a misunderstanding of functions. Functions are not goto labels. There is no reason for getWords and game to be mutually recursive. This leads to two problems:

  • Your functions are inflexible and not reusable.
  • If you play several rounds, the call stack gets deeper and deeper. Eventually, after many many rounds, the program can crash from stack overflow.

Each function should have a single purpose, and it should be documented in a docstring. If a function is named "getWords", then I expect it to return a list of words, and do nothing more than that. It shouldn't pick a word and proceed to play an entire game based on it.

A simple way to reorganize your code to use functions correctly might look like this:

def get_word():
    """
    Ask the user for the difficulty level, and return a randomly
    selected word from the dictionary.
    """
    # Do stuff …, then
    return word

def game(true_word):
    """
    Play a game of Mastermind, where the user needs to guess the
    given word.  Return True if the user guesses correctly within
    5 turns.
    """
    for turn in range(5):
        …
        if user_input == true_word:
            print("Well done, you guessed correctly")
            return True
        …
        print(hint)
    print("Bad luck! The answer was: " + true_word)
    return False

while True:
    game(get_word)

But we can do even better by breaking down those two functions into even smaller functions.

General remarks

  • Commenting nearly every line is mildly obnoxious. For example:

    #num (used to store word lengths) is a random choice from the available lengths for the selected difficulty.
    

    That is just an indication that num is a vaguely named variable. A better choice would be word_length — and then you can eliminate the comment.

    with open('wordbank.txt', 'r') as file:
        #Open the file containing word-bank
    

    Python is quite readable. You don't have to tell me what the code obviously stated.

  • Please follow the PEP 8 official style guide, in particular the naming conventions. Variables should be named like true_word or user_input unless you have a good reason to deviate.

  • Be suspicious of global variables, and avoid using them at all. Your acceptedWord list keeps growing, but never gets cleared.

Specific suggestions

The difficulty dictionary could be better formatted for readability. Its purpose is not immediately obvious, so I'd add a comment.

import random

# Possible word lengths for each difficulty level
DIFFICULTY = {
    1: [4, 5, 6],
    2: [7, 8],
    3: [9, 10, 11],
    4: [12, 13],
    5: [14, 15],
}

getWords() should be turned into a function that reads and returns a data structure representing the entire file, so that you don't have to re-read the file every time before starting a game. Asking for the difficulty level and selecting words are separate tasks that deserve to be in their own functions.

def read_dictionary(filename='wordbank.txt'):
    """
    Read the file, containing one word per line.  Return a dictionary, keyed
    by word length.
    """
    dictionary = {}
    for lengths in DIFFICULTY.values():
        for length in lengths:
            dictionary[length] = []
    with open(filename) as f:
        for line in f:
            word = line.strip().upper()
            dictionary.get(len(word), []).append(word)
    return dictionary

def ask_level():
    """
    Ask the user for to select a valid difficulty level.
    """
    allowable_levels = [str(level) for level in sorted(DIFFICULTY.keys())]
    prompt = "What difficulty would you like? ({}) ".format(
        ','.join(allowable_levels)
    )
    while True:
        choice = input(prompt)
        if choice in allowable_levels:
            return int(choice)
        print("Invalid choice")

def select_words(level, dictionary):
    """
    Pick a sample of words from the dictionary with an appropriate length
    for the difficulty level.  The number of words chosen is equal to the
    length of the chosen word.
    """
    length = random.choice(DIFFICULTY[level])
    return random.sample(dictionary[length], length)

game() could be better named. Hard-coded arbitrary values are often better written as default parameters (turns=5 in this case). There is no point in converting strings to lists. The for loop can be more elegantly written as a single expression.

def mastermind_game(words, turns=5):
    """
    Play a game of Worded Mastermind, where the user has to guess which of the
    words is correct within the specified number of turns.  Return the number
    of remaining guesses (0 if the user lost, non-zero if the user won).
    """
    print("Possible answers:")
    print('\n'.join(words))
    word = random.choice(words)
    for remaining_guesses in range(turns, 0, -1):
        print("Guesses left: {}".format(remaining_guesses))
        guess = input("Guess: ").upper()
        if guess == word:
            print("Well done, you guessed correctly")
            return remaining_guesses
        correct_letter_count = sum(g == w for g, w in zip(guess, word))
        print("{} out of {} correct.".format(correct_letter_count, len(word)))
    print("Bad luck! The answer was: {}".format(word))
    return 0

The main code should obviously give a high-level view of what the program does. In this case, it reads the dictionary once, then loops forever playing Mastermind games. It's customary to write if __name__ == '__main__' so that the code can be safely imported into other Python programs without immediately executing it.

def main(wordlist_filename='wordbank.txt'):
    dictionary = read_dictionary(wordlist_filename)
    while True:
        level = ask_level()
        words = select_words(level, dictionary)
        mastermind_game(words)

if __name__ == '__main__':
    main()
\$\endgroup\$

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.