This program asks users to rank an item and validates the rank

Question

# get rank which takes the users rank as an argument and determines
# if it is valid input

def getRank(x):
    # make a list of the acceptable choices
    choices = ['l','m','h']
    # convert the users rank to lowercase
    x = x.lower()
    # create a while True loop that only runs if x is not in the choices
    while True:
        if x not in choices:
            # remind the user of the valid choices
            print('Only enter a valid choices')
            print(choices[0:3])
            x = input('Please enter a rank: ')
            x = x.lower()
            # add an if statement to continue the loop if the users new input is still incorrect
            if x not in choices:
                continue
        # if everything is all good break from the loop
        else:
            break
    return x

def main():
    print('Please enter a ranking for the items')
    print('l for low,m for medium,h for high')
    print('')
    print('Please enter a rank for milk')
    # prompt the user to enter their rate
    userRate = input('Please enter a rank: ')
    # validate their input
    validRank = getRank(userRate)
    # assign the users rating to validRank as it will only hold a valid choice
    # continue doing this for each item
    milkRate = validRank
    print('Please enter a rank for cheese')
    userRate = input('Please enter a rank: ')
    validRank = getRank(userRate)
    cheeseRank = validRank
    print('Please enter a rank for eggs')
    userRate = input('Please enter a rank: ')
    validRank = getRank(userRate)
    eggRank = validRank
    print('')
    #output the users ratings for the items
    print('Your overall ratings are:')
    print('Milk:',milkRate)
    print('Cheese:',cheeseRank)
    print('Eggs:',eggRank)

main()

Hi everyone! I'm a first year student and I would really love some tips to better my logic here. The above program does exactly what I want it to do: Display an item to the user, ask the user for their rank, validate that their rank is one of 3 choices case insensitive. If its invalid it requests reinput until it is. While this code works, I know there are oppurtunities for me to write better logic here.

I am open to all critiques here! Would just love feedback from the community

Answer 1

For printing multiple lines, instead of multiple print statements, you can use triple quotes.

"""Please enter a ranking for the items
l for low, m for medium, h for high
"""

This is equivalent to the first three lines in main()

Also, merging the input prompt and print statement above for all the 3 items might simplify the code.

For example for milk,

userRate = input('Please enter a rank for milk: ')
milkRate = getRank(userRate)

Instead of

print('Please enter a rank for milk')
userRate = input('Please enter a rank: ')
validRank = getRank(userRate)
milkRate = validRank

You may also use dictionaries to store the data of items and print them out (if possible), so that repetitive code parts (boilerplate) can be replaced with loops.

The suggestions are simple ones but might simplify your code.

Answer 2

Your getRank function could use a little flexibility. If the user inputs low, their input is rejected as incorrect. To solve this you can take the first character of the string passed. Since you index the string at 0, input like l will still work.

Your validation loop could also use some work. Since you're only checking if the users choice is in the valid input list, make the while loop represent that!

def getRank(choice: str) -> str:
    choices = ['l', 'm', 'h']
    choice = choice.lower()[0]
    while choice not in choices:
        print(f'Only enter a valid choice ({choices})')
        choice = input('Please enter a rank: ').lower()[0]
    return choice

Answer 3

I find it a bit weird that getRank, which is perfectly able (even expected) to ask the user for input, also expects to receive an input as an argument. You could just as well have getRank take no parameters and make it ask for the initial input just like it asks when it receives an invalid input.

getRank's structure is a bit awkward. The loop seems like it could just be while x not in choices: rather than a while True: containing nothing but an if / else: break. The continue in there also seems redundant - you're right at the end of the loop, it's going to continue either way.

Why does getRank print choices[0:3] rather than just printing choices? If choices contains 3 or fewer items, it doesn't actually change anything, but if it some day ended up containing more you'd effectively end up hiding options for, as far as I can tell, no particular reason. Sure, your program only needs 3 options right now, but maybe that'll change some day, or maybe you'll want to reuse the code for something else sometime.

Speaking of reuse, if someone (maybe future you) wanted to use your getRank function in the future, they'd likely want to import this file. When they do so, the entire file gets run, including the call to main at the bottom, which they probably don't want. You can avoid that by wrapping the main() call in an if __name__ == '__main__': test - there's really no drawback to doing that, so it's a good habit to pick up.

Your main is just doing the same thing with minor changes multiple times. Usually repetetive code is harder to read, write and maintain, and a lot of the time you can make it less repetetive by looking to the underlying structure and using loops or functions to make sure you only write each thing once. For example, in this case you could do something like

def main():
    ... # Intro messages go here

    items = ['Milk', 'Cheese', 'Eggs']
    ranks = {}

    # Ask for a rank for each item
    for item in items:
        ranks[item] = get_rank()

    for item in items:
        print(item + ':', ranks[item])

Side note, in Python it's customary to spell the names of functions and variables like_this rather than likeThis. It doesn't really matter, what you do so long as you're consistent, but since almost all existing code uses that style you'll probably want to adopt it for your own code as well - mixing styles generally doesn't look very nice.