9
\$\begingroup\$

I am currently learning Python by working my way through Automate the Boring Stuff with Python. This project is from Chapter 8 which is focused on validating input by using the PyInputPlus module. Whenever I finish, I like to look the problem up and compare to other's solutions to see what I could have done better. However, I can only find a couple of links to this problem. Here is the problem:

Sandwich Maker

Write a program that asks users for their sandwich preferences. The program should use PyInputPlus to ensure that they enter valid input, such as:

  • Using inputMenu() for a bread type: wheat, white, or sourdough.
  • Using inputMenu() for a protein type: chicken, turkey, ham, or tofu.
  • Using inputYesNo() to ask if they want cheese. If so, using inputMenu() to ask for a cheese type: cheddar, Swiss, or mozzarella.
  • Using inputYesNo() to ask if they want mayo, mustard, lettuce, or tomato.
  • Using inputInt() to ask how many sandwiches they want. Make sure this number is 1 or more.

Come up with prices for each of these options, and have your program display a total cost after the user enters their selection.

That being said, I'd like to hear what I can do better and how I can improve this code:

import pyinputplus as pyip

# store a dictionary of ingredients and their respective prices
optionPrices = {'white' : 2.00,
                'wheat' : 2.50,
                'sour dough' : 3.00,
                'chicken' : 2.50,
                'turkey' : 2.25,
                'ham' : 1.75,
                'tofu' : 4.00,
                'cheddar' : 1.00,
                'swiss' : 1.25,
                'mozzarella' : 2.00,
                'mayo' : 0.25,
                'mustard' : 0.25,
                'lettuce' : 0.30,
                'tomato' : 0.50
                }

customerOrder = [] # a list to store the current order
extras = ['mayo', 'mustard', 'lettuce', 'tomato']
sandwichTotal = 0.0

# ask the user for bread choice and append to order list
breadChoice = pyip.inputMenu(['white', 'wheat', 'sour dough'], 'Please choose your bread:\n', lettered=True)
customerOrder.append(breadChoice)

# ask the user for protein choice and append to order list
proteinChoice = pyip.inputMenu(['chicken', 'turkey', 'ham', 'tofu'], 'Please choose your protein:\n', lettered=True)
customerOrder.append(proteinChoice)

# ask if the user wants cheese, and if so, record cheese choice
cheeseResponse = pyip.inputYesNo('Would you like cheese?\n')
if cheeseResponse == 'yes':
    cheeseChoice = pyip.inputMenu(['cheddar', 'swiss', 'mozzarella'], 'Please choose your cheese:\n', lettered=True)
    customerOrder.append(cheeseChoice)
else:
    cheeseChoice = ''

# loop through 'extras' and ask if customer wants each one. If so, append it the order
choice = ''
for i in extras:
    choice = pyip.inputYesNo('Would you like ' + i +'?\n')
    if choice == 'yes':
        customerOrder.append(i)
    else:
        choice = ''

# get the number of sandwiches from the customer
numSandwiches = pyip.inputInt('How many sandwiches would you like?\n', min=1)

print('\nYour order: ')
# check if the item exists in the options, and get the price for each sandwich
for item in customerOrder:
    if item in optionPrices.keys():
        sandwichTotal += optionPrices.get(item)
        print('\t' + item + ' - $' + str(optionPrices.get(item)))

print('Total for your sandwich: $' + str('{:0.2f}'.format(sandwichTotal))) # per sandwhich total
print('Total for your order: (' + str(numSandwiches) + ' sandwiches @ $' + 
      str('{:0.2f}'.format(sandwichTotal)) + ' each): ')
print('$' + str('{:0.2f}'.format(sandwichTotal * numSandwiches))) # give the total price of sandwiches

I know that there are likely a lot of things I could do better to optimize or make the code easier to read, but there are some things I just haven't learned yet. I realize this probably isn't the most efficient way to solve this problem and that is why I'd like to get some feedback.

\$\endgroup\$
0
15
\$\begingroup\$

PEP8

The "PEP 8 -- Style Guide for Python Code" has many recommendations on how programs should be written, for maximum understandability between programmers.

Variable Names

The one that you violate the most relates to variable naming. PEP8 recommends snake_case for all variables. So customerOrder should be customer_order, sandwichTotal should be sandwich_total, etc.

Constants

Constants, things which never change, should be named using UPPERCASE or UPPERCASE_WITH_UNDERSCORES. Therefore, optionPrices should be OPTION_PRICES.

White Space

PEP-8 recommends no space between a diction key and the :.

Encapsulate Data into Containers

optionPrices contains breads, proteins, cheeses, and extras. This seems like it is stuffing too much -- well, stuffings -- into one container.

I might write:

BREAD_PRICE = {'white':2.00, 'wheat': 2.50, 'sour dough': 3.00}
PROTEIN_PRICE = {'chicken': 2.50, 'turkey': 2.25, 'ham': 1.75, 'tofu': 4.00}
CHEESE_PRICE = {'cheddar': 1.00, 'swiss' : 1.25, 'mozzarella' : 2.00}
EXTRA_PRICE = {'mayo': 0.25, 'mustard': 0.25, 'lettuce': 0.30, 'tomato': 0.50}

With the bread, protein, and cheese options, you could now write a common function:

def get_sandwich_choice(category, options):
    prompt = "Please choose your " + category + ":\n"
    choices = list(options.keys())
    choice = pyip.inputMenu(choices, prompt, lettered=True)
    return choice

bread_choice = get_sandwich_choice('bread', BREAD_PRICE)
protein_choice = get_sandwich_choice('protein', PROTEIN_PRICE)
...

The price containers are all really the same kind of things, just in groups. So one might instead ...

PRICE = {'bread': {'white':2.00, 'wheat': 2.50, 'sour dough': 3.00},
         'protein': {'chicken': 2.50, 'turkey': 2.25, 'ham': 1.75, 'tofu': 4.00},
         'cheese': {'cheddar': 1.00, 'swiss' : 1.25, 'mozzarella' : 2.00},
         'extras': {'mayo': 0.25, 'mustard': 0.25, 'lettuce': 0.30, 'tomato': 0.50},
         }

and the choices could be fetched with:

bread_choice = get_sandwich_choice('bread', PRICE['bread'])
protein_choice = get_sandwich_choice('protein', PRICE['protein'])
...

You could expand this further by adding more data. bread is choice, protein is a choice, cheese is an optional choice, extras are optionals. Based on the category type, you could call the appropriate user input functions. The prompt for each category could be customized, and so on. To add this extra data, I'd look into @dataclass.

Are all things in customerOrder the same? The bread is kind of unique; you need two slices for the sandwich. The extras might eventually have options like "light on the mayo" or "heavy on the mustard", where as you can't ask for extra bread. So it might make more sense keeping them separate. bread_choice, protein_choice, cheese_choice, and a list of extras. This also helps our cost, since the prices are in different containers.

def sandwich_cost(bread, protein, cheese, extras):
    cost = PRICE['bread'][bread] + PRICE['protein'][protein]
    if cheese:
        cost += PRICE['cheese'][cheese]
    for extra in extras:
        cost += PRICE['extra'][extra]

sandwich_total = sandwich_cost(bread_choice, protein_choice, cheese_choice, extras)

Again, a @dataclass may be useful in construction a sandwich object.

By having a separate function for computing the sandwich cost, you make your code testable. For example, you could check:

assert sandwich_cost('white', 'tofu', 'cheddar', ['tomato']) == 7.50
assert sandwich_cost('white', 'tofu', '', []) == 6.00

Look into unittest for better ways of writing test code.

Main Guard

It is a good habit to move your code into functions, and call it from inside a "main guard" at the end of the code. Eg)

import pyinputplus as pyip

... helper functions here ...

def main():
    ... code here ...

if __name__ == '__main__':
    main()

Summary

Lots of ways to improve this code. I've touched on a few. Experiment, and once you've got something working, post a new followup question for further advice.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much for taking the time to break this down and help me. \$\endgroup\$
    – tylerc515
    Jan 27 at 15:03
2
\$\begingroup\$

Unecessary else-statements

In

if cheeseResponse == 'yes':
    cheeseChoice = pyip.inputMenu(['cheddar', 'swiss', 'mozzarella'], 'Please choose your cheese:\n', lettered=True)
    customerOrder.append(cheeseChoice)
else:
    cheeseChoice = ''

the final else-statement is not necessary. cheeseChoice is only used when cheeseResponse is yes. Therefore there is no need to assign a value to it if cheeseResponse is no.

Since cheeseChoice is only used once for appending the choice to the order list you could drop the variable altogether and inline it into the append. However if the explicit declaration of cheeseChoice helps you read and understand the code then it is fine to leave it in there. In the end there is a good argument to be made that readability is the most important property of code (besides working correctly of course).

If a line of code gets too long you can also wrap it to the next line to make it more readable. I used an explicit line break (\) here. More info can be found in this question.

if cheeseResponse == 'yes':
    customerOrder.append(pyip.inputMenu(['cheddar', 'swiss', 'mozzarella'], \
            'Please choose your cheese:\n', lettered=True))

Correspondingly in

choice = ''
for i in extras:
    choice = pyip.inputYesNo('Would you like ' + i +'?\n')
    if choice == 'yes':
        customerOrder.append(i)
    else:
        choice = ''

the else-statement is not necessary. However so is the initial declaration of choice (since it is then reassigne within the for-loop. In the for loop you could use a more verbose variable instead of i which is usually used when incrementing an int-counter in something like for i in range(10):. Personally I prefer for extra in extras: here.

for extra in extras:
    choice = pyip.inputYesNo('Would you like ' + extra +'?\n')
    if choice == 'yes':
        customerOrder.append(extra)

or inlined:

for extra in extras:
    if pyip.inputYesNo('Would you like ' + extra +'?\n') == 'yes':
        customerOrder.append(extra)

Good luck in your efforts.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the additional insights. I am currently rewriting using these tips. \$\endgroup\$
    – tylerc515
    Jan 28 at 20:56

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.