patternpythonMinorCanonical
Pizza Ordering Program (v2)
Viewed 0 times
programorderingpizza
Problem
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 runnable on repl.it, and is also pasted below.
This is a repost with an updated version of my code for review. Pizza Ordering Program v1 is here. Compared to v1, I have capitalized the constants, and moved the logic to inside the Order class, simplifying the actual run loop.
The full, working code is below:
```
"""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
if input_message:
user_input = input(str(input_message))
else:
user_input = input()
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
This is a repost with an updated version of my code for review. Pizza Ordering Program v1 is here. Compared to v1, I have capitalized the constants, and moved the logic to inside the Order class, simplifying the actual run loop.
The full, working code is below:
```
"""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
if input_message:
user_input = input(str(input_message))
else:
user_input = input()
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
Solution
Here are some observations and suggestions:
-
you can remove redundant
-
you can use the inline if/else for the
or, even better - define the default
-
instead of defining a separate
where
- I think you can either define the
PIZZAS_AVAILABLEin the correct expected order or sort right after defining it instead of sorting it inside the main execution block
-
you can remove redundant
() after the class name:class Order:-
you can use the inline if/else for the
user_input variable:user_input = input(str(input_message) if input_message else '')or, even better - define the default
input_message value as an empty string. Then, you would be able to do:user_input = input(str(input_message)).lower().strip()-
instead of defining a separate
print_order() function, consider defining the __str__() magic method on the Order class. Then, you'll be able to do:print(order)where
order is an instance of the Order class- I don't particularly like the overall design of the
Orderclass - it is currently tied to the user inputs that come from the multipleget_input()calls which makes the user interaction logic scattered across the whole program. Not sure how big of an issue this is, but I would expect theOrderclass to be agnostic of where the order data is coming from (referring to Single responsibility principle)
- the
get_details()is probably not the best name for a method that is checking if an order should be canceled or not
- consider defining
pickup,nameand other things you get viaget_*()methods as properties
Code Snippets
class Order:user_input = input(str(input_message) if input_message else '')user_input = input(str(input_message)).lower().strip()print(order)Context
StackExchange Code Review Q#159869, answer score: 2
Revisions (0)
No revisions yet.