I'm new around here and want to involve myself with fellow developers and hopefully improve my skills here while attempting to help in any way I can with the knowledge I have so far. I'm a new Python programmer, been on and off it for years but have stuck to it recently officially.
I plan on making a career off of developing with it, whether it's data science, cyber security etc.
I plan on implementing some OOP in there instead of just straight functions:
import os
#create shopping list with automatic calculation
def display_help():
print('''
=======> Help <=======
'HELP' to display this menu
'START' to start creating your shopping list
'SHOW' to display your current items
'QUIT' to exit the program
''')
#create a list with the items needed
#
def clear_screen():
os.system("cls" if os.name == "nt" else "clear")
#Create function to change values of current items or update price of items in the cart
def shopping_list():
clear_screen()
print('It\'s time to go shopping!')
#Convert all floating point numbers to max digits of 2 after the decimal
clear_screen()
item_dict = {}
print('Enter HELP at any moment if you need help or EXIT to return to the main menu')
cart = 0
def show_items():
for key, value in item_dict.items():
print(key + '-' * 5 + str(value))
print('Current cart total: ',sum(item_dict.values()))
while True:
shop = input('Please enter an item: ')
if shop.upper() == 'HELP':
display_help()
continue
elif shop.upper() == 'EXIT':
#Show total with taxes (or without taxes) with all the items in cronological order
#start without taxes first
print("Goodbye")
break
elif shop.upper() == 'SHOW':
show_items()
continue
print(f'You have added {shop} to your list\n')
try:
cost = float(input(f'Price of {shop}: '))
item_dict[shop] = cost
cart = sum(item_dict.values())
print(f'You entered {cost} as the price of {shop}\n')
print(f'You now have {len(item_dict)} items in your list.\n')
print(f'The total currently on your cart is {cart}')
add_more_items = input('Would you like to add more items?(Y/N) ')
if add_more_items == 'Y'.lower():
continue
else:
print(f'Here\'s your cart and your total: \n')
raise KeyboardInterrupt('Goodbye')
continue
except ValueError:
print('Please enter a number')
def begin_attempt():
print('Here\'s a list of possible commands: HELP, SHOP, EXIT')
print('Welcome! What would you like to do?')
while True:
choose = input('> ')
if choose.upper() == 'SHOP':
print('Shopping has begun\n')
shopping_list()
elif choose.upper() == 'HELP':
display_help()
continue
elif choose.upper() == 'EXIT':
print("Goodbye")
break
begin_attempt()
I still would like to add a lot more things, such as entering a specific state (or city in this case since I don't want to look up the taxes of every state) to calculate taxes, if there are discounts anywhere (I don't think I'll be adding that anytime soon or at all, but just an idea).
-
\$\begingroup\$ I was just having a quick look through this and was wondering whether at the current stage you want suggestions on how to implement some OOP in there or just things like formatting \$\endgroup\$13ros27– 13ros272018年06月09日 11:54:24 +00:00Commented Jun 9, 2018 at 11:54
-
\$\begingroup\$ Hi there. Yes I am currently trying to implement some OOP in there. I started working on it while also improving my old code. Everything is looking better than my old version. \$\endgroup\$nudood– nudood2018年06月10日 17:18:01 +00:00Commented Jun 10, 2018 at 17:18
1 Answer 1
Welcome to Code Review SE!
I'll list some things I found while skimming over your code. This is unordered, and by no means exhaustive.
Most programming languages have a certain style guide associated with them. In the case of Python, it's PEP-8. It is in your best interest to comply with the points outlined there, as doing so will significantly improve readability.
Keep vertical whitespace to a minimum. Blank lines can aid in grouping related pieces of code together1, but in my opinion, you've gone a bit overboard. Having just two blank lines in between top-level function definitions is recommended.
Your function and variable names follow the PEP-8 naming conventions2. Good job, lots of people get this wrong at first.
Whenever you find yourself using an escaped quote character (
\'
or\"
), you can simply surround the string with the other type of quotes.3 This makes the code more readable and reduces the chances of getting syntax errors. To take an example from the piece of code you wrote:print('It\'s time to go shopping!')
... becomes:
print("It's time to go shopping!")
It's good practice to separate arguments to functions by a single space.4 An example from
shopping_list()
:print('Current cart total: ',sum(item_dict.values()))
... becomes:
print('Current cart total: ', sum(item_dict.values()))
Block comments should start with a space.5
It's nice that you added comments, but most of them are plain wrong. Never add comments that are simply a rewording of the code, and always assure your comments are up to date and correct. Let's go over your comments one by one.
Lines 3 through 6:
#create shopping list with automatic calculation def display_help():
... not even close. I guess you reordered your functions at some point and forgot to update this comment.
Lines 14 through 17:
#create a list with the items needed # def clear_screen():
... the same goes for this comment. You don't create a list here. You don't do anything here.
Lines 22 through 26:
#Create function to change values of current items or update price of items in the cart def shopping_list():
... finally, a comment that's in the right place. Unfortunately, this one falls under the 'documenting self-documenting code' category.6 Nitpick: you normally don't create a function, but define it.
...
I'm happy you found a way to avoid global variables, but nesting functions is rarely the right solution.
show_items()
should really be a top-level function, taking one argument.By using f-strings, you're greatly limiting portability. I'd make a safe bet and use
str.format()
for now.Let's see what happens when I exit the program:
Welcome! What would you like to do? > EXIT Goodbye
So far, so good. How about quitting from the 'SHOP' submenu?
Please enter an item: QUIT You have added QUIT to your list Price of QUIT:
Whoops, that's not what I was hoping to see. The help message clearly says:
'QUIT' to exit the program
Peculiar. Now that we're stuck with having to add a new item, let's go through with it and then exit.
Price of QUIT: 50 You entered 50.0 as the price of QUIT You now have 1 items in your list. The total currently on your cart is 50.0 Would you like to add more items?(Y/N) n Here's your cart and your total: Traceback (most recent call last): File "ttt.py", line 86, in <module> begin_attempt() File "ttt.py", line 78, in begin_attempt shopping_list() File "ttt.py", line 64, in shopping_list raise KeyboardInterrupt('Goodbye') KeyboardInterrupt: Goodbye
Oh my, that's ugly.
On a more serious note, there should only be one way to exit the program, and it should not be by means of an uncaught exception. You could do this a number of ways:
Put all of the prompts in a huge
while
-loop, then break from the loop when the user wants to exit. This gets messy very quickly. Avoid it.Put all of the prompts in a huge
while
-loop, then call a function to exit. This is a much better alternative.
As an aside, it's almost never a good idea to raise a
KeyboardInterrupt
manually. Even aRuntimeError
would be a better fit, but as I explained, cleanly exiting the script is ideal. Thesys
module providessys.exit()
, which is the best way of quitting Python scripts. Don't useexit()
. It is a special function and should only be used in the Python REPL.The main menu is broken:
=======> Help <======= 'HELP' to display this menu 'START' to start creating your shopping list 'SHOW' to display your current items 'QUIT' to exit the program > start > show > SHOW > :(
try
/except
-constructs should capture the least amount of code possible. A clear violation of this principle, starting on line 52:try: ... [12 lines skipped] except ValueError: print('Please enter a number')
... the
try
-block only needs to encompass line 53, because that's the only line that could raise aValueError
.On line 60, the input validation is off. The prompt suggests 'Y' and 'N' are valid inputs, but:
if add_more_items == 'Y'.lower():
... indicates it is not. If you want to do case-insensitive comparisons, call
str.lower()
, like you did in other instances. Alternatively, you could callstr.casefold()
, which also handles special characters like 'ß':>>> "Straße".lower() 'straße' >>> "Straße".casefold() 'strasse'
shopping_list
is a bad function name. Function names are typically verbs. Maybeplay_shopping_game
?String concatenation is in many ways inferior to string formatting. It's slower, harder to read, and less versatile: each argument needs to be casted to
str
if it is not already a string. Always prefer string formatting over concatenation.
References
4 PEP-8: Whitespace in Expressions and Statements
5 PEP-8: Comments: Block Comments
6 Don't get me wrong, documenting functions is very important. The 'create a function' part is too verbose nevertheless. If you want to add formal documentation to functions, add docstrings! Docstrings are multiline strings placed below a function or class signature. For example:
def add(a, b):
"""Return `a` + `b`. Both arguments must be numbers."""
return a + b
You can access the docstring as the object's __doc__
attribute:
>>> add.__doc__
'Return `a` + `b`. Both arguments must be numbers.'
... and in the Python REPL, you can use the built-in help()
:
add(a, b)
Return `a` + `b`. Both arguments must be numbers.
There's also an official style guide for docstrings, called PEP-257.
-
\$\begingroup\$ Wow thanks a lot! I really appreciate this and will continue to work on the app. I try to follow the guidelines as much as I can, but practice is what makes perfect. I wanted a good review and you went in depth which I really appreciate. I left some things out in blank since I forgot to add or remove things. The comments I haven't edited due to what you mentioned, I forgot and was busy editing things. May I send you anything updated or post updates here? Not constantly but if I feel I made huge edits. \$\endgroup\$nudood– nudood2018年05月25日 17:36:57 +00:00Commented May 25, 2018 at 17:36
-
\$\begingroup\$ @nudood Of course, if you made improvements and want to put the new code up for review, simply add a new question. Make sure to link back to the previous iteration every time you add a newer version, to avoid having the question flagged as a duplicate. :) \$\endgroup\$Daniel– Daniel2018年05月25日 17:54:47 +00:00Commented May 25, 2018 at 17:54