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
}
\$\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()
\$\endgroup\$
3
  • \$\begingroup\$ Not a criticism, but a question. Is it more pythonic to pass or to continue when ignoring errors in a while loop? \$\endgroup\$
    – C.Nivs
    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\$ Jun 1, 2023 at 18:22
  • \$\begingroup\$ Thank you for the advice. \$\endgroup\$
    – Beginner
    Jun 2, 2023 at 18:54
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.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.