3
\$\begingroup\$

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
}
asked May 31, 2023 at 22:25
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

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()
answered Jun 1, 2023 at 1:27
\$\endgroup\$
2
  • \$\begingroup\$ Not a criticism, but a question. Is it more pythonic to pass or to continue when ignoring errors in a while loop? \$\endgroup\$ Commented 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 the except:, you can probably pass and continue with the rest of the loop. \$\endgroup\$ Commented Jun 1, 2023 at 18:22
0
\$\begingroup\$

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.

answered Jun 22, 2023 at 14:02
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.