Skip to content

Simplify the code #1

@GasperPaul

Description

@GasperPaul

Even if I forget that this is not up to the specification provided in the problem, there's still quite a few issues, mainly unnecessary complex code. For example:

python_hangman/Main.py

Lines 9 to 18 in 3f93eb0

def read_tries ():
try:
i = int(input ("How many attempts do you want to have: "))
except ValueError:
print ("Enter number from 1 to 100")
i = read_tries ()
if (i<1) | (i>100):
print ("Enter number from 1 to 100")
i = read_tries ()
return i

This can be as simple as:

def read_tries ():
    i = None
    while not i or not i.isdigit() or 0 >= int(i) or int(i) > 100:
        i = input ("How many attempts do you want to have: ")
    return int(i)

Also, trie is a special data structure, so the name is somewhat misleading. Better call it something like read_attempts.

python_hangman/Main.py

Lines 20 to 28 in 3f93eb0

def read_cont ():
ans = input ()
if ans == "+":
return True
elif ans == "-":
return False
else:
print ("Enter + if you want to restart or - if you'd like exit")
return(read_cont())

def read_cont ():
    ans = None
    while ans not in "+-": 
        ans = input("Enter + if you want to restart or - if you'd like exit")
    return ans == "+"

This is:

  1. Simplier in logic.
  2. More readable.
  3. Doesn't use recursion. Even though it's a tail recursion Python doesn't optimise it. In your version if you fail to enter enough times the program will crash. Each call will also take up memory.

In Python 3.8 this can be rewritten even shorter using :=.

python_hangman/Main.py

Lines 30 to 36 in 3f93eb0

def read_letter ():
letter = input ("Enter your letter: ")
if (len(letter) == 1) & (((ord(letter[0])>64) & (ord(letter[0])<91)) | ((ord(letter[0])>96) & (ord(letter[0])<123))):
return letter.lower()
else:
print ("Enter one letter of latin alphabet")
return(read_letter())

This is hideous.

  1. Do not use magic numbers in code. What are that 64 or 91. Yeah, I know that they are letters. But it doesn't follow from the code itself, which is bad. At least use ord on literals, like ord('a'), instead. This also makes this code encoding specific.
  2. Bitwise operators on bool expressions convert them to ints first. Why do this when you have and and or?

Previous version with isalpha was better. Python 3.8 have isascii, but even without it you can write:

def read_letter ():
    letter = input("Enter your letter: ")
    while not len(letter) == 1 or letter not in string.ascii_letters:
        letter = input("Enter one letter of latin alphabet")
    return letter.lower()

python_hangman/Main.py

Lines 48 to 60 in 3f93eb0

def solve_form(solve_old, letter_pos, word):
solve_new = ""
j = 0
for i in letter_pos:
while j != i:
solve_new += solve_old[j]
j += 1
solve_new += word[j]
j += 1
while j < (len(word)):
solve_new += solve_old[j]
j += 1
return solve_new

This is literally four lines of code:

def solve_form(solve_old, letter_pos, word):
    for i in letter_pos:
        solve_old = solve_old[:i] + word[i] + solve_old[i+1:]
    return solve_old

It's not even better, because it's shorter. It's better because the intent is clear. I read that piece of code trying to wrap my head around your use of indices for 5 minutes.

solve = str("*" * len(word))

str is redundant.

python_hangman/Main.py

Lines 68 to 93 in 3f93eb0

res = 1
while (res == 1):
if (old_letters != []):
print ("You've entered: " + str(old_letters))
print ()
letter = read_letter()
if (letter not in old_letters):
old_letters.append(letter)
letter_pos = letter_check (word, letter)
if letter_pos != []:
solve = solve_form(solve, letter_pos, word)
print ("You guessed! " + solve)
else:
if (tries == 1):
res = 0
else:
tries -= 1
print ("You guessed wrong. " + str(tries) + " attempts remain")
if (solve.find("*") == -1):
res = 2
else:
print ("You have already entered this letter")
if res == 0:
print ("You lost. The word was " + word)
if res == 2:
print ("Congratulations! You won!")

This can be simplified:

finished = False
while not finished:
    if old_letters != []:
        print(f"You've entered: {str(old_letters)}\n")
    letter = read_letter()
    if letter in old_letters:
        print("You have already entered this letter")
    else:
        old_letters.append(letter)
        letter_pos = letter_check(word, letter)
        if letter_pos != []:
            solve = solve_form(solve, letter_pos, word)
            print(f"You guessed! {solve}")
            if solve.find("*") == -1:
                print("Congratulations! You won!")
                finished = True
        else:
            tries -= 1
            if tries == 0:
                print(f"You lost. The word was {word}")
                finished = True
            else:
                print(f"You guessed wrong. {tries} attempts remain")

Note, that you don't need result as int, you can use more natural bool for that. The endgame status can be inlined. If you dislike this fo stylistic reasons, use enumerations or named constants instead of magic numbers.

python_hangman/Main.py

Lines 100 to 101 in 3f93eb0

print ("Would you like to restart? (+/-)")
cycle_flag = read_cont()

This is inconsistent with your other usages of input functions. You should either put input message prompt inside the read_cont, or move all prompts from their respective functions. I would suggest first option.

In a more general error/issues category: using tabulation as an indent is frowned upon. Use 4 spaces, they're rendered better in most editors.

Instead of function main you can use

if __name__ = '__main__':
    # code

construction, as it is more pythonic.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions