3
\$\begingroup\$

Updated code here on repl.it I have changed the code so the get-input logic is in the class, as separate methods. This means the loop to input multiple orders is much more concise than before.

The full code:

"""Onehunga Pizzas phone orders"""

import re
import sys

# CONFIG #
# maximum number of pizzas in one order
MAX_PIZZAS = 5
# delivery charge (in $)
DELIVERY_CHARGE = 3.00
# list of dictionaries (pizzas) with name and price
PIZZAS_AVAILABLE = (
    {"name": "Hawaiian",             "price": 8.5},
    {"name": "Meat Lovers",          "price": 8.5},
    {"name": "Pepperoni",            "price": 8.5},
    {"name": "Ham & Cheese",         "price": 8.5},
    {"name": "Classic Cheese",       "price": 8.5},
    {"name": "Veg Hot 'n' Spicy",    "price": 8.5},
    {"name": "Beef & Onion",         "price": 8.5},
    {"name": "Seafood Deluxe",       "price": 13.5},
    {"name": "Summer Shrimp",        "price": 13.5},
    {"name": "BBQ Bacon & Mushroom", "price": 13.5},
    {"name": "BBQ Hawaiian",         "price": 13.5},
    {"name": "Italiano",             "price": 13.5},
)
# END CONFIG #


def get_input(regex, input_message=None, error_message=None):
    """Gets valid input, validated using regular expressions."""
    # loops until input is valid ("break" is called)
    while True:
        # input to validate, input prompt is as specified
        user_input = input(str(input_message))
        user_input = user_input.lower().strip()
        # check if the user wants to quit or cancel the order
        if user_input == "qq" or user_input == "quit":
            sys.exit()
        elif user_input == "cc" or user_input == "cancel":
            return "CANCEL"

        # check if the input matches the regex provided
        if re.match(regex, user_input, re.IGNORECASE):
            break

        # if it doesn't match, and an error message has been specified
        if error_message:
            print(str(error_message))

    return user_input


def print_line(line):
    """Prints line starting with | and ending with |. """
    print("| {:54} |".format(line))


def print_order(order):
    """Prints order details."""
    print_line("Name: "+ order.name)
    print_line("Order type: "+ ("Pickup" if order.pickup else "Delivery"))
    if not order.pickup:
        print_line("Delivery address: "+ order.address)
        print_line("Customer phone number: "+ order.phone)
    print_line("")
    print_line("Order summary:{:15}Price each:{:5}Subtotal:".format("", ""))
    for pizza in order.pizzas:
        print_line("{:5}x {:22}{:5}${:5.2f}{:8}${:>5.2f}".format(
            pizza["amount"], pizza["name"], "",
            pizza["price"], "", pizza["price"]*pizza["amount"]))
    if not order.pickup:
        print_line("{:4}Delivery charge{:29}${:>5.2f}"
                .format("", "", DELIVERY_CHARGE))

    print_line("{:48}------".format(""))
    print_line("{:40} Total: ${:.2f}".format("", order.cost))


class Order():
    def __init__(self):
        self.name = ""
        self.pickup = False
        self.address = None
        self.phone = None
        self.pizzas = []
        self.cost = 0

    def get_pickup(self):
        user_input = get_input(
            r"$|(?:P|D)",
            "Pickup or delivery? [Pickup]:",
            "Please enter a 'p' (pickup) or a 'd' (delivery)")
        if user_input == "CANCEL":
            return "CANCEL"
        self.pickup = user_input.lower().startswith("p") or not user_input

    def get_name(self):
        user_input = get_input(
            r"[A-Z]+$",
            "Enter customer name:",
            "Name must only contain letters")
        if user_input == "CANCEL":
            return "CANCEL"
        self.name = user_input[:48]

    def get_address(self):
        user_input = get_input(
            r"[ -/\w]+$",
            "Delivery address:",
            "Address must only contain alphanumeric characters")
        if user_input == "CANCEL":
            return "CANCEL"
        self.address = user_input[:36]

    def get_phone(self):
        user_input = get_input(
            r"\d+$",
            "Phone number:",
            "Phone number must only contain numbers")
        if user_input == "CANCEL":
            return "CANCEL"
        self.phone = user_input[:11]

    def get_pizzas(self):
        while True:
            user_input = get_input(
                r"\d$",
                "Number of pizzas to order:",
                "Must be a digit, 5 or less")
            if 0 < int(user_input) <= MAX_PIZZAS:
                number_pizzas = int(user_input)
                break
            else:
                print("Must be a digit, 5 or less (but more than 0)")

        print("\nWhat pizzas would you like to order?")
        for i, pizza in enumerate(PIZZAS_AVAILABLE):
            # each pizza's number is its index (i) + 1,
            # so the first pizza is 1
            print("{}: {}".format(str(i+1).zfill(2), pizza["name"]))

        print("\nEnter your selection number for each pizza you want to buy")
        for i in range(number_pizzas):
            while True:
                string = "Pizza #{} of {}:".format(i+1, number_pizzas)
                user_input = get_input(
                    r"\d\d?$",
                    string,
                    "Pizza selection number must "
                    "correspond to those listed above")
                try:
                    if int(user_input) == 0:
                        raise IndexError
                    # selects the pizza based on user_input
                    to_add = PIZZAS_AVAILABLE[int(user_input)-1]

                    # if the pizza has already been ordered,
                    # increment the amount ordered
                    for ordered in self.pizzas:
                        if to_add["name"] == ordered["name"]:
                            ordered["amount"] += 1
                            break
                    # else add the pizza to the order list
                    else:
                        to_add["amount"] = 1
                        order.pizzas.append(to_add)

                    # if there has been no error,
                    # input is valid, break from the while loop
                    break

                except IndexError:
                    print("Pizza selection number must "
                        "correspond to those listed above")

    def get_cost(self):
        cost = sum(
            pizza["price"]*pizza["amount"]
            for pizza in self.pizzas)
        if not self.pickup:
            cost += DELIVERY_CHARGE
        self.cost = cost

    def get_details(self):
        if self.get_pickup() == "CANCEL":
            return "CANCEL"
        if self.get_name() == "CANCEL":
            return "CANCEL"
        if not self.pickup:
            if self.get_address() == "CANCEL":
                return "CANCEL"
            if self.get_phone() == "CANCEL":
                return "CANCEL"
        if self.get_pizzas() == "CANCEL":
            return "CANCEL"
        if self.get_cost() == "CANCEL":
            return "CANCEL"


#if __name__ == "__main__":
if __name__ == "builtins": # for repl.it
    print(
    "== Onehunga Pizzas ==\n"
    "==  Order Manager  ==\n"
    "Enter 'CC' to cancel order, or 'QQ' to exit program at any time.\n"
    "The first letter of a word is usually only required as input.\n"
    "A word [enclosed] in brackets is the default option.\n")

    # list to hold all completed orders
    orders = []

    # sorts pizza list by price, then alphabetically
    PIZZAS_AVAILABLE = sorted(
        PIZZAS_AVAILABLE,
        key=lambda k: (k["price"], k["name"]))

    while True:
        order = Order()
        if not order.get_details() == "CANCEL":
            print("\nOrder saved. Order was:")
            print_order(order)
            orders.append(order)
        else:
            print("\nOrder cancelled.")

        user_input = get_input(
            r"$|(?:Y|N|O).*",
            "Would you like to enter another order or "
                "view all previous orders? [Yes]/No/Orders:",
            "Only yes/no or \"orders\" responses allowed")
        if user_input.lower().startswith("n"):
            sys.exit()
        elif user_input.lower().startswith("o"):
            for i, order in enumerate(orders):
                if i == 0:
                    print("-"*23 +" ALL ORDERS "+ "-"*23)
                else:
                    print("|"+ "-"*56 +"|")
                print_order(order)
                if i == len(orders) - 1:
                    print("-" * 58)


I have made a phone order pizza ordering program for my Digital Tech assignment. I would appreciate feedback on the code quality (increasing efficiency, whilst maintaining readability.)

The full code is here on repl.it, and is also pasted below.


In particular, I'm wondering how to correctly write the following (according to PEP8). I do not have access to Python 3.6 so cannot use f"string {var}". The options I current have are:

a)

print("| \t{}x {:<22}\t${:<6.2f}\t\t${:>5.2f}".format(pizza['amount'], pizza['name'], pizza['price'], pizza['price']*pizza['amount']))`

b)

print("| \t{}x {:<22}\t${:<6.2f}\t\t${:>5.2f}".format(
    pizza['amount'], pizza['name'],
    pizza['price'], pizza['price']*pizza['amount']))

c)

print("| \t{}x {:<22}\t${:<6.2f}\t\t${:>5.2f}".format(
    pizza['amount'],
    pizza['name'],
    pizza['price'],
    pizza['price']*pizza['amount']))

d)

print("| \t{}x {:<22}".format(
    pizza['amount'], pizza['name']), end="")
print("\t${:<6.2f}\t\t${:>5.2f}".format(
    pizza['price'], pizza['price']*pizza['amount']))


While this may be more suited for Stack Overflow, I'll include it as a help-if-you-want-to sort of thing. You do not need to address this if you do not want to.

repl.it gives an error on the following code:

 user_input = get_input(
    r"$|(?:Y|N|O).*",
    "Would you like to enter another order or view all previous orders? [Yes]/No/Orders:",
    "Only yes/no or \"orders\" responses allowed")

if user_input.lower().startswith("n"):
        sys.exit()
elif user_input.lower().startswith("o"):
        pass # other code here

The error is Instance of 'int' has no 'lower' member'. It does not stop the code from running, the code works fine when run. Why am I getting this error? get_input will always return a string.


The full code is as follows (also available via this repl.it link). I have tried to make it fit the PEP8.

"""Onehunga Pizzas phone orders"""

import re
import sys

""" CONFIG """
# maximum number of pizzas in one order
max_pizzas = 5
# delivery charge (in $)
delivery_charge = 3.00
# list of dictionaries (pizzas) with name and price
pizzas_available = (
    {"name": "Hawaiian",             "price": 8.5},
    {"name": "Meat Lovers",          "price": 8.5},
    {"name": "Pepperoni",            "price": 8.5},
    {"name": "Ham & Cheese",         "price": 8.5},
    {"name": "Classic Cheese",       "price": 8.5},
    {"name": "Veg Hot 'n' Spicy",    "price": 8.5},
    {"name": "Beef & Onion",         "price": 8.5},
    {"name": "Seafood Deluxe",       "price": 13.5},
    {"name": "Summer Shrimp",        "price": 13.5},
    {"name": "BBQ Bacon & Mushroom", "price": 13.5},
    {"name": "BBQ Hawaiian",         "price": 13.5},
    {"name": "Italiano",             "price": 13.5},
)
""" END CONFIG """


# defines exception; this is to be raised when an order is cancelled
class CancelOrder(Exception):
    pass


# class with default order details
class Order():
    def __init__(self):
        self.pickup = True

        self.name = ""
        self.address = None
        self.phone = None

        self.number_pizzas = 0
        self.pizzas = []

        self.total_cost = 0


def get_input(regex, input_message=None, error_message=None, ignore_case=True):
    """Gets valid input, validated using regular expressions."""
    # loops until input is valid ("break" is called)
    while True:
        # input to validate, input prompt is as specified
        u_input = input(str(input_message))

        # check if the user wants to quit or cancel the order
        lower = u_input.lower()
        if lower == "qq" or lower == "quit":
            sys.exit()
        elif u_input == "cc" or u_input == "cancel":
            raise CancelOrder()

        # check if the input matches the regex provided
        if ignore_case:
            if re.match(regex, u_input, re.IGNORECASE):
                break
        else:
            if re.match(regex, u_input):
                break

        # if it doesn't match, and an error message has been specified
        if error_message:
            print(str(error_message))

    return u_input


def print_order(order):
    print("| Name: {}".format(order.name))
    print("| Order type: {}".format("Pickup" if order.pickup else "Delivery"))
    if not order.pickup:
        print("| Delivery address: {}".format(order.address))
        print("| Customer phone number: {}".format(order.phone))
    print("|\n| Order summary:\t\t\t\tPrice each:\tSubtotal:")
    for pizza in order.pizzas:
        print("| \t{}x {:<22}\t${:<6.2f}\t\t${:>5.2f}".format(
            pizza['amount'], pizza['name'],
            pizza['price'], pizza['price']*pizza['amount']))

    if not order.pickup:
            print("| \tDelivery charge\t\t\t\t\t$ {:>5.2f}".format(
                delivery_charge))
    print("| {:61}--------".format(""))
    print("| {:54} Total: ${:.2f}".format("", order.total_cost))


print("== Onehunga Pizzas ==")
print("==  Order Manager  ==")
print("Enter 'CC' to cancel order, or 'QQ' to exit program at any time")
print("The first letter of a word is usually only required as input")
print("A word [enclosed] in brackets is the default option")

# list to hold all completed orders
orders = []

# sorts pizza list by price, then alphabetically
pizzas_available = sorted(
    pizzas_available,
    key=lambda k: (k["price"], k["name"]))

# keep getting orders, only exits through sys.exit()
while True:
    # try ... except to catch CancelOrder exception
    try:
        print("\nNew Order")
        order = Order()

        # get delivery/pickup type
        user_input = get_input(
            r"$|(?:P|D)",
            "Pickup or delivery? [Pickup]:",
            "Please enter a 'p' (pickup) or a 'd' (delivery)")
        if user_input.lower().startswith("d"):
            order.pickup = False

        # get name info
        order.name = get_input(
            r"[A-Z]+$",
            "Enter customer name:",
            "Name must only contain letters")

        # get address, phone number info (if the customer wants delivery)
        if not order.pickup:
            order.address = get_input(
                r"[ -/\w]+$",
                "Delivery address:",
                "Address must only contain alphanumeric characters")
            order.phone = get_input(
                r"\d+$",
                "Phone number:",
                "Phone number must only contain numbers")

        # get number of pizzas to order,
        # make sure it is more than 0,less than max_pizzas
        while True:
            user_input = get_input(
                r"\d$",
                "Number of pizzas to order:",
                "Must be a digit, 5 or less")
            user_input = int(user_input)
            if 0 < user_input <= max_pizzas:
                order.number_pizzas = user_input
                break
            else:
                print("Must be a digit, 5 or less (but more than 0)")

        # print menu (each pizza is assigned a number)
        print("\nWhat pizzas would you like to order?")
        for i, pizza in enumerate(pizzas_available):
            # each pizza's number is its index (i) + 1,
            # so the first pizza is 1
            print("{}: {}".format(str(i+1).zfill(2), pizza['name']))

        print("\nEnter your selection number for each pizza you want to buy")
        for i in range(order.number_pizzas):
            while True:
                string = "Pizza #{} of {}:".format(i+1, order.number_pizzas)
                user_input = get_input(
                    r"\d\d?$",
                    string,
                    "Pizza selection number must"
                    "correspond to those listed above")
                user_input = int(user_input)
                try:
                    if user_input == 0:
                        raise IndexError
                    # selects the pizza based on user_input
                    to_add = pizzas_available[user_input-1]

                    # if the pizza has already been ordered,
                    # increment the amount ordered
                    for ordered in order.pizzas:
                        if to_add["name"] == ordered["name"]:
                            ordered["amount"] += 1
                            break
                    # else add the pizza to the order list
                    else:
                        order.pizzas.append(to_add)
                        order.pizzas[-1]["amount"] = 1

                    # if there has been no error,
                    # input is valid, break from the while loop
                    break

                except IndexError:
                    print("Pizza selection number must"
                        "correspond to those listed above")

        order.total_cost = sum(
            pizza["price"]*pizza["amount"]
            for pizza in order.pizzas)
        if not order.pickup:
                order.total_cost += delivery_charge

        # add order to list of orders
        orders.append(order)
        print("\nOrder saved. Order was:")
        print_order(order)

        user_input = get_input(
            r"$|(?:Y|N|O).*",
            "Would you like to enter another order or view all"
                "previous orders? [Yes]/No/Orders:",
            "Only yes/no or \"orders\" responses allowed")
        if user_input.lower().startswith("o"):
            for i, order in enumerate(orders):
                print("-" * 73)
                print_order(order)
                if i == len(orders) + 1:
                    print("-" * 73)
        elif user_input.lower().startswith("n"):
            sys.exit()

    except CancelOrder:
        try:
            print("\nOrder cancelled")
            user_input = get_input(
                r"$|(?:Y|N).*",
                "Would you like to enter another order? [Yes]/No",
                "Only yes or no responses allowed")
            if user_input.lower().startswith("n"):
                sys.exit()

        except CancelOrder:
            print("Type 'QQ' to exit the program")
\$\endgroup\$
4
\$\begingroup\$
  • The configuration is really a set of constants, so their names should be uppercase.
  • You shouldn't use exceptions for control flow. Cancelling an order is not an exceptional event, it is part of the normal flow of your application, just like removing an item from an order would be.
  • I generally consider it an anti-pattern to create an object with no parameters and filling it in gradually. Instead
    1. Accumulate everything you need to create an Order in variables.
    2. Create the Order when everything is ready.
    3. Specify any defaults (if necessary) in the constructor parameters.
  • Order is a data class. Shouldn't it have methods like cancel and submit?
  • I would rip out all the print and get_input stuff into a separate user interface component. That would greatly simplify reading the actual logic. Today, interactive text interfaces like this is usually only found in homework assignments. In a real-life system you are much more likely to have a non-interactive tool which takes everything it needs (removing the need to be able to cancel an order and all the print/get_input stuff) and some sort of storage to persist an order.
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I'll leave the question open for another day or so to see if I get more answers, points 3 & 4 are mutually exclusive aren't they? Point 5 is valid, but isn't going to happen as the entire point of this assignment is to take input, store it in variable, do a few calculations (cost), and (essentially) print out the input again. This is an assignment for Year 12 / Level 2 NCEA (NZ) / 16 year olds. I will change my code to fit points 1, 2 & 3 thanks :) \$\endgroup\$ – DarkMatterMatt Apr 2 '17 at 19:42
  • \$\begingroup\$ 3 & 4 are not mutually exclusive. Can you explain why you think that? \$\endgroup\$ – l0b0 Apr 2 '17 at 21:12
  • \$\begingroup\$ Surely if you create the Order() after you have all the variables, the cancel order and submit order functions will not be created until after all the data has been collected (so the cancel order will not be needed)? I will change the Order class to a dictionary to avoid having a Data Class. \$\endgroup\$ – DarkMatterMatt Apr 2 '17 at 21:19
  • \$\begingroup\$ Cancel order not being needed is a bonus, no? \$\endgroup\$ – l0b0 Apr 2 '17 at 21:24
  • \$\begingroup\$ I've updated the code so all the get-input code is in Order() methods. I still have all the variables defined under init because otherwise repl.it gives me warnings about defining class variables from inside methods. \$\endgroup\$ – DarkMatterMatt Apr 3 '17 at 6:04

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.