4
\$\begingroup\$

I'd like feedback please as to improvements which could be made to make this more 'pythonic' in its approach/structure. Any constructive feedback welcome. The below is a game which takes a single random word from a list of 3000 and has the player guess letters (whilst being limited to a certain number of attempts).

#!/usr/bin/env python3


import random
import linecache
import sys

sourcefile = 'wordlist'  # list of words


def playagain():
    response = input('Do you want to play again? ')
    response = response.lower()
    if response == 'yes' or response == 'y':
        main()
    else:
        print('OK...')
        sys.exit(0)


def find_word():
    random_line = random.randint(1, 3000)  # random line number
    chosen_word = linecache.getline(sourcefile, random_line)
    str_chosen_word = chosen_word.strip('\n')
    return str_chosen_word.lower()


def getIndexPositions(list_chosen_word, char):
    indexPosList = []
    indexPos = 0
    while True:
        try:
            # Search for item in list from indexPos to the end of list
            indexPos = list_chosen_word.index(char, indexPos)
            # Add the index position in list
            indexPosList.append(indexPos)
            indexPos += 1
        except ValueError as e:
            break

    return indexPosList


def main():
    guessed = []  # create list to track guessed letters
    list_masked_word = []  # create list to show progress of guessing
    str_chosen_word = find_word()  # get random word
    list_chosen_word = list(str_chosen_word)  # create list from word
    length_list_chosen_word = len(list_chosen_word)  # length of word
    attempts = length_list_chosen_word + 3  # number of attempts
    list_masked_word = ['\u25A0'] * length_list_chosen_word  # masking
    game(attempts, guessed, list_chosen_word, list_masked_word)


def game(attempts, guessed, list_chosen_word, list_masked_word):
    print()
    print('Welcome to the word game')
    while (attempts != 0):
        print()
        print('You have', attempts, 'attempts to guess the word correctly')
        print('So far you have found: ', ''.join(list_masked_word))
        attempts -= 1  # reduce the number of attempts available
        guess = str(input('Enter a letter: ')).lower()
        if len(guess) == 1:
            if guess in guessed:
                print('Ooops - You have already tried that letter')
            else:
                guessed.append(guess)  # keeps track of letters guessed
                indexPosList = getIndexPositions(list_chosen_word, guess)
                for index in indexPosList:
                    list_masked_word[index] = guess
                if list_chosen_word == list_masked_word:
                    print('The word was: ',''.join(list_chosen_word))
                    print('Well Done - you guessed the word')
                    playagain()

        else:
            print()
            print('Enter only one letter!')
    print()
    print('You are out of guesses')
    playagain()


if __name__ == "__main__":
    main()
\$\endgroup\$
2
  • 1
    \$\begingroup\$ What is this program? What is it supposed to do and why does it exist? Please add context; thanks. \$\endgroup\$
    – Reinderien
    Apr 25 '20 at 15:12
  • \$\begingroup\$ It's a word game. It takes a random word. Masks it. The player has a certain number of attempts to guess the correct word. \$\endgroup\$
    – yeleek
    Apr 25 '20 at 20:58
2
\$\begingroup\$

Global constants

sourcefile = 'wordlist'  # list of words

should be

SOURCE_FILE = 'wordlist'  # list of words

Looping instead of recursion

if response == 'yes' or response == 'y':
    main()

For a small number of replays this will have no noticeable ill effect, but if an obsessive user replays enough, they will eventually blow the stack. Replace this with a loop at the upper level in main.

Function names

def getIndexPositions

should be

def get_index_positions

and similar snake_case for your local variables.

State passing

Why are guessed and list_masked_word passed into game? They seem like they should just be initialized within game without being parameters.

String interpolation

'You have', attempts, 'attempts to guess the word correctly'

is more easily expressed as

f'You have {attempts} attempts to guess the word correctly'

Loop syntax

This:

while (attempts != 0):

does not need parens. Also, rather than manually decrementing it, you can do

for attempt in range(attempts, 0, -1):

Typo

Ooops -> Oops

Early-continue

I find that this:

    if len(guess) == 1:
        # lots of code...
    else:
        print()
        print('Enter only one letter!')

is more easily legible as

    if len(guess) != 1:
        print()
        print('Enter only one letter!')
        continue

    # lots of code...
\$\endgroup\$
1
  • \$\begingroup\$ Thats great - thank you. \$\endgroup\$
    – yeleek
    May 7 '20 at 14:51

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.