This was simply practice to touch on somethings I have learned. I have also tried to take into account Pep 8 formatting practices to my best ability and applicable advice from my previous posts. It was difficult to keep the indents to 2 levels in. Constructive criticism much appreciated.
main.py
from time import sleep
import sys
from resources import MENU, resources
ACTIONS = ['espresso', 'latte', 'cappuccino', 'report', 'off', 'e', 'l', 'c', 'r', 'o']
FORMAT = '.2f'
TIME = 1.5
def perform_action(choice):
""" Main loop for paying for and receiving a coffee type"""
if choice in ['espresso', 'e']:
choice = 'espresso'
payment = paying(choice)
prepare(choice, payment)
elif choice in ['latte', 'l']:
choice = 'latte'
payment = paying(choice)
prepare(choice, payment)
elif choice in ['cappuccino', 'c']:
choice = 'cappuccino'
payment = paying(choice)
prepare(choice, payment)
elif choice in ['report', 'r']:
for item in resources:
print(f'{item.title()}: {resources[item] if item != "money" else f"${format(resources[item], FORMAT)}"}\n')
else:
sys.exit()
def paying(drink):
""" Checks for payment and its viability and returns how much you paid"""
cost = MENU[drink]['cost']
quarter, dime, nickel, penny = 'None', 'None', 'None', 'None'
while not quarter.isdigit():
quarter = (input('How many quarters? '))
while not dime.isdigit():
dime = (input('How many dimes? '))
while not nickel.isdigit():
nickel = (input('How many nickels? '))
while not penny.isdigit():
penny = (input('How many pennies? '))
quarter, dime, nickel, penny = int(quarter) * .25, int(dime) * .10, int(nickel) * .05, int(penny) * .01
pay = (quarter + dime + nickel + penny)
if pay < cost:
print(f'Not enough payment!, refunding ${pay}...\n')
sleep(TIME)
return [False,pay]
elif pay > cost:
change = pay - cost
pay = pay - change
print(f'Returning change of ${format(change, FORMAT)}...\n')
sleep(TIME)
print(f'Payment Successful!, ordering {drink}...\n')
sleep(TIME)
resources['money'] += pay
return [True, pay]
def prepare(drink, pay):
"""Calculates and removes resources from coffee machine if drink resources don't exceed it."""
prepared = pay[0]
if prepared:
for i in resources:
if i == 'money':
continue
if drink == 'espresso' and i == 'milk':
continue
if resources[i] < MENU[drink]['ingredients'][i]:
prepared = False
print(f'Not enough {i}.')
if prepared:
resources[i] -= MENU[drink]['ingredients'][i]
if prepared:
print(f'Enjoy your {drink}!\n')
sleep(TIME)
else:
print(f'Refunding {pay[1]}....')
resources['money'] -= pay[1]
running = True
while running:
action = None
while action not in ACTIONS:
action = input(
"\nCoffee:\n\t⚪Espresso[E]: 1ドル.50\n\t⚪Latte[L]: 2ドル.50\n\t⚪Cappuccino[C]: "
"3ドル.00\n\nCommands:\n\t⚪Report[R]\n\t⚪Off[O]\n\n"
"What would you like? "
).lower()
print('\n')
perform_action(action)
resources.py
MENU = {
"espresso": {
"ingredients": {
"water": 50,
"coffee": 18,
},
"cost": 1.5,
},
"latte": {
"ingredients": {
"water": 200,
"milk": 150,
"coffee": 24,
},
"cost": 2.5,
},
"cappuccino": {
"ingredients": {
"water": 250,
"milk": 100,
"coffee": 24,
},
"cost": 3.0,
}
}
resources = {
"water": 300,
"milk": 200,
"coffee": 100,
"money": 0
}
2 Answers 2
It's a decent start for a beginner.
ACTIONS
isn't very well-represented as a list of strings. Consider instead a dictionary of strings to action functions. The action functions can be pre-bound to arguments (like the drink product) by using partial
.
There is repeated code in perform_action
. You can reduce this repetition by maintaining a simple database of products and their prices.
Similarly, there is repeated code in paying
(which should be called pay
, the associated imperative-tense verb). Consider looping over a sequence of coin definitions.
Don't sleep
. This is a UI anti-feature, and I generally consider it to be lying to your user (seeming like the program is doing something worth waiting for when it isn't).
FORMAT
isn't all that useful as a hard-coded constant. In the simple case I would just write .2f
where it's used; but the more portable thing to do is use the currency formatting support from the locale module.
sys.exit()
is not a good thing to call on the inside of perform_action
(or, really, anywhere in this program). You should do a graceful unwind of the stack with return
instead of exiting.
Your variables quarter
, etc. start off as strings and end up as floats. Don't change the type of a variable midway through its use.
Add PEP484 type hints to your function signatures.
Move your main loop starting at running = True
into a main()
function so that those variables are not global, and so that e.g. a unit tester can selectively import some functions while avoiding the main entry point. For this reason, also add an if name == '__main__'
guard.
resources.py
is better off as a JSON file than as code.
if drink == 'espresso' and i == 'milk'
should not be hard-coded. Just loop over the resources.
Suggested
Some mid-level stuff here, but nothing that you can't learn.
resources.json
[
{
"name": "espresso",
"cost": 1.50,
"ingredients": {
"water": 50,
"coffee": 18
}
},
{
"name": "latte",
"cost": 2.50,
"ingredients": {
"water": 200,
"milk": 150,
"coffee": 24
}
},
{
"name": "cappuccino",
"cost": 3.00,
"ingredients": {
"water": 250,
"milk": 100,
"coffee": 24
}
}
]
main.py
import json
from functools import partial
from locale import currency, setlocale, LC_ALL
from typing import Callable, Iterator, Any, NamedTuple
resources = {
"water": 300,
"milk": 200,
"coffee": 100,
"money": 0
}
class Denomination(NamedTuple):
plural: str
cents: int
def input_tender(self) -> int:
count = int(input(f'How many {self.plural}? '))
if count < 0:
raise ValueError('Cannot tender a negative coin count')
return count * self.cents
DENOMINATIONS = (
Denomination('quarters', 25),
Denomination('dimes', 10),
Denomination('nickels', 5),
Denomination('pennies', 1),
)
def pay(drink: dict[str, Any]) -> float:
"""Checks for payment and its viability and returns how much you paid"""
cost: float = drink['cost']
tender_cents = 0
for coin in DENOMINATIONS:
while True:
try:
tender_cents += coin.input_tender()
break
except ValueError:
pass
tender = tender_cents*0.01
if tender < cost:
print(f'Not enough payment! Refunding {currency(tender)}...')
return 0
if tender > cost:
change = tender - cost
print(f'Returning change of {currency(change)}...')
print(f'Payment Successful! Ordering {drink["name"]}...')
return cost
def prepare(drink: dict[str, Any]) -> bool:
"""Calculates and removes resources from coffee machine if drink resources don't exceed it."""
new_resources = dict(resources)
for resource_name, needed in drink['ingredients'].items():
new = resources[resource_name] - needed
if new < 0:
print(f'Not enough {resource_name}.')
return False
new_resources[resource_name] = new
resources.update(new_resources)
return True
def purchase(drink: dict[str, Any]) -> bool:
"""End-to-end purchase of one drink"""
payment = pay(drink)
if payment > 0:
if prepare(drink):
print(f'Enjoy your {drink["name"]}!')
resources['money'] += payment
else:
print(f'Refunding {currency(payment)}...')
return True
def make_menu_description(
drinks: list[dict[str, Any]], commands: dict[str, Callable],
) -> Iterator[str]:
"""Iterator of strings (one line each) representing the menu"""
yield 'Coffee:'
for drink in drinks:
yield (
f'\t⚪{drink["name"].title()}'
f'[{drink["name"][0].upper()}]: '
f'{currency(drink["cost"])}'
)
yield ''
yield 'Commands:'
for name in commands.keys():
yield (
f'\t⚪{name.title()}'
f'[{name[0].upper()}]'
)
yield ''
def make_menu_choices(
drinks: list[dict], commands: dict[str, Callable],
) -> Iterator[tuple[
str, # menu key
Callable[[], bool] # menu action callable
]]:
"""Make the menu choice key-value iterator; all values are no-argument callables that return
True if the program should continue"""
for drink in drinks:
bound_purchase = partial(purchase, drink=drink)
yield drink['name'][0].upper(), bound_purchase
for name, command in commands.items():
yield name[0].upper(), command
def report() -> bool:
for item, amount in resources.items():
if item == 'money':
desc = currency(amount)
else:
desc = amount
print(f'{item.title()}: {desc}')
return True
def off() -> bool:
# signal main loop to quit
return False
def main() -> None:
setlocale(LC_ALL, '')
with open('resources.json') as f:
drinks = json.load(f)
commands = {'report': report, 'off': off}
menu_desc = '\n'.join(make_menu_description(drinks, commands))
choices = dict(make_menu_choices(drinks, commands))
while True:
print(menu_desc)
choice_str = input('What would you like? ')
perform_action = choices.get(choice_str[:1].upper())
if perform_action is not None and not perform_action():
break
print()
if __name__ == '__main__':
main()
-
\$\begingroup\$ Not a criticism, but a question. Is it more pythonic to
pass
or tocontinue
when ignoring errors in awhile
loop? \$\endgroup\$C.Nivs– C.Nivs06/01/2023 14:50:21Commented Jun 1, 2023 at 14:50 -
1\$\begingroup\$ It depends. If the rest of the loop depends on the information that would have been returned/calculated when the exception was raised, you probably need to
continue
since you never got the data. If the rest of the loop doesn't depend on that data and you don't need to do any cleanup operations in theexcept:
, you can probablypass
and continue with the rest of the loop. \$\endgroup\$Elias Benevedes– Elias Benevedes06/01/2023 18:22:35Commented Jun 1, 2023 at 18:22
Some questions
- What happens with the resources if water and milk are available, but coffee is not?
- What do you have to change
- if the price for Espresso changes?
- if Espresso has to come with milk?
- to introduce another drink?
def prepare(drink, pay):
You loop the wrong list thus checking if putting money into your coffee. Instead loop the ingredients of your drink.
for i, amount in MENU[drink]['ingredients']:
resources[i] -= amount
prepared
is a bad name for paid
. then you reuse the name for the
buggy logic "can be prepared" and "prepared".
Instead do not enter the function if there is insufficient payment.
For the bug either loop twice, first checking, then preparing,
or work on a copy of resources
which you discard in case of an error.
Prepare shall not care about refunding, it shall return success only. Refunding shall be done by the caller.
def paying(drink):
... is hard to test as it is unnecessarily coupled to the menu. It should care about money only.
Its signature should be paying(cost)
. As it claims to refund it shall not return pay
but 0
.
Thus the return tuple is not required, the caller shall test for 0.
hard-coded strings for drinks in the code
If you tried to answer the questions in the introduction you already know, that this a bad idea. You have hard-coded prices and hard-coded drink names. Think of selling a configurable vending machine. All prices, drinks etc. have to come from configuration only, which is resources.py in your case.
Also hard-coded strings as comparison target are error prone. At least use variables, typos there will result in exceptions.