5
\$\begingroup\$

Can the following code be simplified/ improved? (NOTE: I am yet to learn OOP/ def functions). I have not used OOP and def functions on purpose, as I am yet to learn this aspect of Python. I have listed some things I would like to improve on below.

  1. Could/ Should I implement for loops?
  2. Is the structure of the code of a good standard?
  3. How can I simplify the code (using less loops/ less code in general)?
# Python 3.4.3 // MacOS (Latest)

import random

WORD = ('apple', 'oracle', 'amazon', 'microsoft')
word = random.choice(WORD)
correct = word
clue = word[0] + word[(len(word)-1):(len(word))]
letter_guess = ''
word_guess = ''
store_letter = ''
count = 0
limit = 5

print('Welcome to "Guess the Word Game!"')
print('You have 5 attempts at guessing letters in a word')
print('Let\'s begin!')
print('\n')

while count < limit:
    letter_guess = input('Guess a letter: ')

    if letter_guess in word:
        print('yes!')
        store_letter += letter_guess
        count += 1

    if letter_guess not in word:
        print('no!')
        count += 1

    if count == 2:
        print('\n')
        clue_request = input('Would you like a clue?')
        if clue_request == 'y':
            print('\n')
            print('CLUE: The first and last letter of the word is: ', clue)
        if clue_request == 'n':
            print('You\'re very brave!')

print('\n')
print('Now its time to guess. You have guessed',len(store_letter),'letters correctly.')
print('These letters are: ', store_letter)

word_guess = input('Guess the whole word: ')
while word_guess:
    if word_guess.lower() == correct:
        print('Congrats!')
        break

    elif word_guess.lower() != correct:
        print('Unlucky! The answer was,', word)
        break

print('\n')
input('Press Enter to leave the program')
\$\endgroup\$
5
\$\begingroup\$

In this case it's totally fine to not define any functions or classes. It does not always make sense to use those things just because you can.

In this case for loops do indeed make sense.

The structure is pretty straight forward, but you should revise what if, else and elif do.

correct = word

Not quite sure what you are doing here by having two variables with the same content.


clue = word[0] + word[(len(word)-1):(len(word))]

You can use negative indexes to count from the end of the sequence:

That would make this line:

clue = word[0] + word[-1]

letter_guess = ''
word_guess = ''
store_letter = ''
count = 0
limit = 5

you could use more descriptive names like: guess_limit, guess_count etc. Some of these variables don't need to be set here yet, e.g. letter_guess.


while count < limit:

Here you would use a for loop (for guess_count in range(limit)) like you suspected.


    letter_guess = input('Guess a letter: ')

I recommend you add some checking here to see if the letter is actually valid.


    if letter_guess in word:
        ...

    if letter_guess not in word:

If it was in the word previously, it still is. No need to re-check, you can just use else


            print('\n')

you can just use an empty print() to print a new line


        if clue_request == 'y':
            ...

        if clue_request == 'n':
            ...

what happens here if clue_request is neither 'y' nor 'n'?


while word_guess:

what is the purpose of this while loop? All cases hit break


    if word_guess.lower() == correct:
        ...

    elif word_guess.lower() != correct:
        ...

You can just use else here again

Full program:

"""
Word Guess Game

by Coal_
"""
import random

WORDS = ('apple', 'oracle', 'amazon', 'microsoft')
correct_word = random.choice(WORDS)
clue = correct_word[0] + correct_word[-1]

store_letter = ''
limit = 5

print('Welcome to "Guess the Word Game!"')
print('You have 5 attempts at guessing letters in a word')
print('Let\'s begin!')
print()

for guess_count in range(limit):
    while True:
        letter_guess = input('Guess a letter: ')

        if len(letter_guess) == 1:
            break
        else:
            print("Oops! Guess a letter!")

    if letter_guess in correct_word:
        print('yes!')
        store_letter += letter_guess
    else:
        print('no!')

    if guess_count == 2:
        print()
        clue_request = input('Would you like a clue?')
        if clue_request.lower().startswith('y'):
            print()
            print('CLUE: The first and last letter of the word is: ', clue)
        else:
            print('You\'re very brave!')

print()
print('Now its time to guess. You have guessed', len(store_letter), 'letters correctly.')
print('These letters are: ', store_letter)

word_guess = input('Guess the whole word: ')
if word_guess.lower() == correct_word:
    print('Congrats!')
else:
    print('Unlucky! The answer was,', correct_word)

print()
input('Press Enter to leave the program')
\$\endgroup\$
4
\$\begingroup\$
  • Instead of importing all of random, you could import only random.choice (from random import choice)

  • Instead of using escape sequences to print single quotes ' and double quotes ", you should, for the sake of readability, do this:

    print("Something in 'quotation' marks") print('Something in "quotation" marks') # Note the use of single and double quotes to denote strings-

  • Conditional statements can be improved, instead of if letter_guess in word: and then if letter_guess not in word:, use an if-elif (-else) setup. See Rewritten.

  • I am really tired, but I'm sure there's better ways to implement game logic here-

Rewritten

I've rewritten your code to be easier to follow and read. I've avoided using functions (but seriously, learn functions, they're helpful).

from random import choice

# Removed WORD variable since you don't really need it
word = choice(('apple', 'oracle',
                     'amazon', 'microsoft'))

# Removed correct variable since it points to word and is therefore not
# needed

clue = word[0] + word[::-1][0]    # Simplified slices
# Removed redundant letter_guess = '', since 
# this is overwritten in the while loop
word_guess = ''
store_letter = ''
count = 0
limit = 5

print('Welcome to "Guess the Word Game!"')
print('You have 5 attempts at guessing letters in a word')
print("Let's begin!")
print('\n')    # print('\n') prints a newline,
                  # not a blank line, maybe use
                  # print()

while count < limit:
    letter_guess = input('Guess a letter: ')
    count += 1
    # Moved count += 1 here so count doesn't have
    # to be incremented twice

    if letter_guess in word:
        print('yes!')
        store_letter += letter_guess

    else:
    # if letter_guess not in word:
        print('no!')

    if count == 2:
        print('\n')
        clue_request = input('Would you like a clue? [y / n] ')
        # Added "[y / n]" to make it clear the 
        # user can only use those responses
        if clue_request == 'y':
            print('\n')
            print('CLUE: The first and last letter of the word is: ', clue)
        elif clue_request == 'n':
            # Changed to elif
            print("You're very brave!")

print('\n')
print('Now its time to guess. You have guessed', len(store_letter), 'letters 
correctly.')
print('These letters are: ', store_letter)

word_guess = input('Guess the whole word: ')

# Removed useless while loop (both if and else 
# statements break out of the loop)

if word_guess.lower() == word:
    print('Congrats!')
else:
    # You don't have to write out a whole
    # elif condition, just use else-
    print('Unlucky! The answer was,', word)
    break

print('\n')
input('Press Enter to leave the program ')
# This last input may be redundant if your only
# option is to leave
\$\endgroup\$
1
\$\begingroup\$

There are 2 things that you must care about right away:

  • You do not find OOP sexy for the moment, but at least you should adopt a functional approach to design the solution of your program. I mean, you need to refactor your code in terms of functions because your solution, as well as the 2 other ones suggested to you in previous answers, follows an archaeological way of coding (no offense)
  • You have a long list of settings. It is a bad practice to mix settings with the properly useful instructions. You should avoid that in the name of separation of concerns principle by either designing a class which only concern is to implement the settings or dedicate a module to do that job (this is very common in the professional area)

From the first and second bullets above, the answer to your second and main question Is the structure of the code of a good standard? is simply no.

Regarding the naming conventions, I guess you improved comparing to your first posts. Continue on.

You have too much ifs. That is bad as I informed you previously because, among other things, it leads to deeply nested code (problem still visible in the suggested answers)

Unlike what @Coal suggested you, it is usually better to import modules the way you do. There are many reason for that: for example, if later you need more submodules of random you will need to type them one by one, adding unnecessary lines of code to your program. Sometimes (like with tkinter and PyQt wildcards), it is even a "must" that you import the modules the way you did.

\$\endgroup\$
  • 1
    \$\begingroup\$ I disagree with the point you make about importing modules -especially in this situation where only random.choice() is needed, it makes no sense to import all of random (yes, if you need randrange, randint, choice, shuffle, SystemRandom, sample, (...), it would make sense to import the whole module)- \$\endgroup\$ – Daniel Jun 12 '17 at 8:37
  • 1
    \$\begingroup\$ Moderator note: The above comment by @Coal_ has been flagged three times for being "wrong". Wrong comments should not be deleted, they should be replied to. Unfortunately moderators are not able to decline comment flags with a specific message. \$\endgroup\$ – Simon Forsberg Jun 13 '17 at 10:09
  • 3
    \$\begingroup\$ The thing is, it isn't necessarily a bad thing to use from random import choice. A Stackoverflow answer on this reads, and I quote: "The difference between import module and from module import foo is mainly subjective. Pick the one you like best and be consistent in your use of it. ..."- That being said, I much prefer using from x import y over import x, simply because it's easier to reference y thereafter. \$\endgroup\$ – Daniel Jun 13 '17 at 14:29
  • 1
    \$\begingroup\$ Misinformation is downvoted, not flagged to death - here and on SO. That said the line between preference and flatout "wrong" is rather blurry, at least in this case. If you disagree with an answer, comment, downvote and move on. If InfoSec deletes answers that are wrong, they're doing it wrong. The system is made so that good answers get upvotes, and bad/wrong ones get downvotes. Badly downvoted answers may end up removed by community members, but never by a moderator - mod-deleted answers can't be edited/fixed and undeleted. \$\endgroup\$ – Mathieu Guindon Jun 14 '17 at 0:33
  • 2
    \$\begingroup\$ @BillalBEGUERADJ I'd just prefer that you reply to the original comment and explain why you think it's wrong. Please, share the link to the docs and educate the original commenter. \$\endgroup\$ – Simon Forsberg Jun 14 '17 at 13:47

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.