HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinorCanonical

Pizza Ordering Program (v2)

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Here are some observations and suggestions:

  • I think you can either define the PIZZAS_AVAILABLE in 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 Order class - it is currently tied to the user inputs that come from the multiple get_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 the Order class 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, name and other things you get via get_*() 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.