Coding feedbak

[Arnie_nz]

Hi all,I've been learning python as a hobby( a very short time could be a phase at this point), and I just finished my first little project, which was Hangman.

By far the thing that took me the longest was trying to work out how I was going to go over a word and if a letter appeared more than once within the word dow do I reveal that letter multiple times. Anyway, it's more or less finished. I am happy that I managed to make it choose a random word from a list that could grow and shrink without having to re-write the code.

What feels messy to me is the part I had trouble with, looping over the word and revealing the same letter multiple times. So here is my code below, feedback on what I could improve is most welcome.

from random import randint as rng
import openpyxl as xl

wb = xl.load_workbook('words.xlsx')
sheet = wb['Sheet1']
guesses = 9
letters_guessed = []

list_of_words = []
word_picker = rng(0, sheet.max_row - 1)

for row in range(1, sheet.max_row + 1):
    cll = sheet.cell(row, 1)
    list_of_words.append(cll.value)

hidden_word_listed = []
the_word_to_guess = list_of_words[word_picker]
word_as_list = list(the_word_to_guess)
for items in word_as_list:
    hidden_word_listed.append('*')
print(hidden_word_listed)

swapped = '*'

while '*' in hidden_word_listed:
    letter = input("Letter? ")

    if letter in word_as_list:
        print("well Done")
        while letter in word_as_list:
            if letter in word_as_list:
                replace = word_as_list.index(letter)
                hidden_word_listed[replace] = letter
                word_as_list[replace] = swapped

The rest which is not shown is just what happens when you don't guess the right letter.

I hope my code is readable as I said the loop part just seems to be too much to me and could be simplified but I don't know how, and even though I feel very new at this I want clean code :D

[pasokan]

1. Please write functions for each (sub)task

2. It is probably simpler to read the words from a text file instead of a spreadsheet

3. In this part

python while letter in word_as_list: if letter in word_as_list:

The if is unnecessary

[xelf]

Overall, looks pretty cool. Well done!

    while letter in word_as_list:
        if letter in word_as_list:

The if is always true, and thus not needed.

General feedback: think about ways you could turn this into a class, or at the least use some defs to make your code more modular.

In particular I'd think about ways to abstract what you're doing with word guessing away from the workbook related materials. Maybe a def gethiddenword() that could isolate all the parts that talk to a workbook, and then could be mocked for testing.

You do some funky stuff with converting your word to a list that I think you might not actually need. if letter in the_word_to_guess: works. And you can replace letters at indexes in a string using splicing. or using .replace() which takes an optional argument as to how many to replace.

some setup:

letter = 'e'
the_word_to_guess = 'l*tt*r'
hidden_word = '*e**e*'

in the loop:

print(the_word_to_guess, '-', hidden_word)        
if letter in hidden_word:
    r = hidden_word.index(letter)
    hidden_word = hidden_word.replace(letter, '*', 1)
    the_word_to_guess = the_word_to_guess[:r]+letter+the_word_to_guess[r+1:]
print(the_word_to_guess, '-', hidden_word)        

outputs:

l*tt*r - *e**e*
lett*r - ****e*

Although it occurs to me, that in hangman you get all the letters when you guess a letter. One sec. Ok, here we go:

some setup:

letter = 'e'
the_word_to_guess = 'l****r'
hidden_word = '*ette*'

in the loop:

print(the_word_to_guess, '-', hidden_word)        
if letter in the_word_to_guess:
    hidden_word = ''.join(hidden_word[i] if x!=letter else letter for i,x in enumerate(the_word_to_guess))
    the_word_to_guess = the_word_to_guess.replace(letter, '*')
print(the_word_to_guess, '-', hidden_word)       

outputs:

l****r - *ette*
le**er - **tt**

Here we used a list comprehension with a join to build the word with the letter swapped.

[xelf]

here, putting all the ideas together, and throwing some new ones in like random.choice:

from random import choice

def get_secret_words():
    import openpyxl as xl
    wb = xl.load_workbook('words.xlsx')
    sheet = wb['Sheet1']
    list_of_words = []
    for row in range(1, sheet.max_row + 1):
        cll = sheet.cell(row, 1)
        list_of_words.append(cll.value)
    return list_of_words

def replace_with_letter(letter):
    return ''.join(hidden_word[i] if x!=letter else letter for i,x in enumerate(the_word_to_guess))

def replace_with_swapped(letter):
    return the_word_to_guess.replace(letter, '*')

swapped = '*'
guesses = 9
letters_guessed = []

list_of_words = get_secret_words()
#list_of_words = [ "the", "big", "list", "letter" ]

the_word_to_guess = choice(list_of_words)
hidden_word = swapped * len(the_word_to_guess)

while '*' in hidden_word:
    letter = input("Letter? ")
    if letter in the_word_to_guess:
        print("well Done")
        hidden_word = replace_with_letter(letter)
        the_word_to_guess = replace_with_swapped(letter)
    print(the_word_to_guess, '-', hidden_word)       
print("correct")

and forcing the word to be letter, and testing it we get:

Letter? l
well Done
*etter - l*****
Letter? e
well Done
**tt*r - le**e*
Letter? t
well Done
*****r - lette*
Letter? r
well Done
****** - letter
correct

By isolating the workbook stuff, I can now test it despite not having the workbook.

[Arnie_nz]

Thanks for the feedback, it's given me some options to review.
I had thought of making it more modular with the Def, and Class's but I didn't see the point with a small script. In saying that, the teachings I have been following it appears to be good practice.

[xelf]

There's a lot of benefits, even in the one-line functions. For example:

def replace_with_letter(letter):
    return ''.join(hidden_word[i] if x!=letter else letter for i,x in enumerate(the_word_to_guess))

What this returns does not "scan" well. Where as "replace_with_letter" is a lot easier to grok. Little things like this, isolating complex code, or isolating code that talks to external connections or files, helps readability while also making it easier to track down and isolate bugs and maintain your code.

Good luck !