8
\$\begingroup\$

I wrote a basic menu application for my intro to python. I'm fairly new, and python does look promising.

the_burger = 16.99;
french_fries = 5.99;
currie_sauce = 19.99;
napkins_with_chocolates = 10.50;
juice_box = 89.01;
takeout = 18.99;
total = 0.0;
DONE = False
print("""
+-------------------------------------------+
| The Restaurant at the End of the Universe |
+---------------------------------+---------+
| A\tThe "Big Boy" Burger      | $""" + str(the_burger) + """  |
+---------------------------------+---------+
| B\tFrench Fries              | $""" + str(french_fries) + """   |
+---------------------------------+---------+
| C\tCurrie sauce              | $""" + str(currie_sauce) + """  |
+---------------------------------+---------+
| D\tNapkins with Chocolates   | $""" + str(napkins_with_chocolates) + str(0) + """  |
+---------------------------------+---------+
| E\tJuice Box                 | $""" + str(juice_box) + """  |
+---------------------------------+---------+
| F\tTakeout                   | $""" + str(takeout) + """  |
+---------------------------------+---------+
""");
while(not DONE):
  print("Total:", total);
  Item = input("Select a letter or 'done': ");
  if Item is "A":
    total += the_burger;
  elif Item is "B":
    total += french_fries;
  elif Item is "C":
    total += currie_sauce;
  elif Item is "D":
    total += napkins_with_chocolates;
  elif Item is "E":
    total += juice_box;
  elif Item is "F":
    total += takeout;
  elif Item is "done":
    print("Final total:", total);
    DONE = True
\$\endgroup\$
  • 4
    \$\begingroup\$ It will let you use Unicode - and probably much more of Unicode than you're already using... \$\endgroup\$ – Toby Speight Oct 8 '18 at 14:52
  • 4
    \$\begingroup\$ Did you ask nicely? (What I mean is, how did you try? And how did it refuse? I know this isn't Stack Overflow, so the non-working code wouldn't be on-topic, but I'd be interested to know). \$\endgroup\$ – Toby Speight Oct 8 '18 at 14:58
  • 2
    \$\begingroup\$ Unicode is supposed to be available out-of-the-box. Example here. \$\endgroup\$ – Mathias Ettinger Oct 8 '18 at 16:19
  • 2
    \$\begingroup\$ Time to use a better IDE then: ideone.com/uK6XJ6 \$\endgroup\$ – Mathias Ettinger Oct 8 '18 at 17:15
  • 2
    \$\begingroup\$ I'm not quite sure what the downvote is for... I've followed all of the rules for CR that I can tell. \$\endgroup\$ – FreezePhoenix Oct 8 '18 at 18:04
11
\$\begingroup\$

Flag variables suck, and should be avoided. Moreover, variables should not be named in ALL_CAPS to look like constants. All you need to get out of the loop is a break.

You've hard-coded the parts of the menu in three places:

  • the prices
  • the ASCII table
  • the loop

All of the menu information should be defined in one place. You can programmatically generate the ASCII table using the astropy.io.ascii package, but I've put together a quick-and-dirty implementation below.

The if statements in the loop should be replaced by a dictionary lookup. Furthermore, is is the wrong operator to use; string comparison should be done using ==. In fact, entering "done" doesn't correctly end the loop, because of that.

You used + str(0) as a hack to get a price ending in "0" to display properly. To represent fixed-point numbers, you should use a Decimal instead.

This program is long enough that it would be a good idea to make a main() function.

Statements should generally not be terminated with semicolons in Python. Also, PEP 8, the official style guide, specifies that indentation should be four spaces. This is an important convention in Python, where indentation matters a lot.

Suggested solution

from collections import OrderedDict, namedtuple
from decimal import Decimal
from string import ascii_uppercase

def tabular(table, widths):
    def sandwich(delim, contents):
        return delim + delim.join(contents) + delim
    def cell(value, width):
        return ' ' + str(value).ljust(width - 2)
    def cells(row):
        return sandwich('|', (cell(col, w) for col, w in zip(row, widths))) + '\n'
    horiz_rule = sandwich('+', ('-' * (w - 1) for w in widths)) + '\n'
    return sandwich(horiz_rule, (cells(row) for row in table))

# In Python 3.7, this should be a @dataclass instead:
class Item(namedtuple('Item', 'name price')):
    def __new__(cls, name, price):
        return super().__new__(cls, name, Decimal(price))

def main():
    menu_items = OrderedDict(zip(ascii_uppercase, [
        Item('The "Big Boy" Burger', '16.99'),
        Item('French Fries', '5.99'),
        Item('Currie sauce', '19.99'),
        Item('Napkins with Chokolates', '10.50'),
        Item('Juice Box', '89.01'),
        Item('Takeout', '18.99'),
    ]))

    print(
        tabular([['The Restaurant at the End of the Universe']], [36 + 9]) +
        tabular(
            (('{0} {1.name}'.format(*stuff), '${1.price}'.format(*stuff))
              for stuff in menu_items.items()),
            [36, 9]
        )
    )

    total = Decimal('0.00')
    while True:
        print('Total: ${0}'.format(total))
        selection = input("Select a letter or 'done': ")
        if selection == 'done':
            break
        total += menu_items[selection].price
    print('Final total: ${0}'.format(total))

if __name__ == '__main__':
    main()
\$\endgroup\$
  • 2
    \$\begingroup\$ in python 3.6 I would use f-strings instead of str.format and do (f'{letter} {item.name}', f'{item.price}' for letter, item in menu_items.items()) \$\endgroup\$ – Maarten Fabré Oct 8 '18 at 20:49
  • 2
    \$\begingroup\$ Also, in Python 3.7+ the OrderedDict could be a normal dict. \$\endgroup\$ – Graipher Oct 8 '18 at 21:13
  • 1
    \$\begingroup\$ Very in-depth, just a question why is class Item(namedtuple('Item', 'name price')): def __new__(cls, name, price): return super().__new__(cls, name, Decimal(price)) not a simple namedtuple? \$\endgroup\$ – Caridorc Oct 9 '18 at 17:50
  • 1
    \$\begingroup\$ @Caridorc It's to avoid having to repetitively write the Decimal() call in Item('The "Big Boy" Burger', Decimal('16.99')) and every subsequent item. \$\endgroup\$ – 200_success Oct 9 '18 at 17:52
6
\$\begingroup\$

Huge bug in 3.5.2

Entering done does nothing in Python 3.5.2, use == instead of is to fix this. In general is asks if two objects are the same object not if the contents are the same, this can give results different from what you expect for lists, so I suggest using == overall.

https://dbader.org/blog/difference-between-is-and-equals-in-python

Code repetition / Extensibility

A famous problem is code extensibility, it is quite inconvenient to add another item to the menu, also if you take a look at this list of ifs

  if Item is "A":
    total += the_burger;
  elif Item is "B":
    total += french_fries;
  elif Item is "C":
    total += currie_sauce;
  elif Item is "D":
    total += napkins_with_chocolates;
  elif Item is "E":
    total += juice_box;
  elif Item is "F":

you will notice that the only thing that happens is adding the cost to the total each time so there is a lot of repetition.

So let me explain a better solution.

I will use a list of tuples (pairs) of the form (food, price).

Than both the printing and the accounting of the money can be done from this data structure, you will only need to add another line inside and both printing and accounting will be generated automatically.

I left out the proper printing alignement as an exercise for the reader, resource: https://stackoverflow.com/questions/5676646/how-can-i-fill-out-a-python-string-with-spaces

Code with didactic comments: (real code would not be commented so much)

import string
ALPHABET = string.ascii_uppercase

FOOD_WITH_PRICES = [
    ("Apple", 0.5),
    ("Ham",   4),
    ("Bread", 1)
]

# for x in list: is standard syntax to iterate over a list, x becomes progressively equal to each element
# enumerate adds the index to each element
def print_stilish_menu(food_with_prices):
    print("""
+-------------------------------------------+
| The Restaurant at the End of the Universe |
+---------------------------------+---------+""")
    for (index, (food, price)) in enumerate(food_with_prices):
        print("""\
| {letter}\tThe "{food}"      | $ {price}  |
+---------------------------------+---------+
""".format(letter=ALPHABET[index], food=food, price=price))

# list[:n] means the first n elements of a list
# for more info look for `python list slice`
print_stilish_menu(FOOD_WITH_PRICES)
total = 0
while(True):
    print("Total:", total);
    x = input("Select a letter or 'done': ")
    if x in ALPHABET[:len(FOOD_WITH_PRICES)]:
        total += FOOD_WITH_PRICES[ALPHABET.index(x)][1]
    elif x == 'done':
        break
    # Some kind of message if the input is invalid
    # is good practice
    else:
        print("Invalid Input")
print("You spent {}".format(total))
\$\endgroup\$

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.