1
\$\begingroup\$

In this python program, I used lists and control flow to allow users to add items to a shopping list and display the list.

I need advice on my code in terms of (architecture, risks, opportunities, design) so that I can learn how I can code better & grow my skills.

shopping_list = []

# Add a function to allow users to ask for help when they need to
def show_help():
    print('What should we pick up at the store?')
    print("""
  Enter 'DONE' to stop adding items.
  Enter 'HELP' for additional info.
  Enter 'SHOW' to see your shopping list
  """)

# Create a function that adds an item to the list
def add_to_list(item):
    shopping_list.append(item)
    print('{} was added to your shopping list!'.format(item))
    print('You have {} items on your list.'.format(len(shopping_list)))


# Create a function to print all the items in the shopping list
def show_list():
    print('My Shopping List:')
    for item in shopping_list:
        print(item)


show_help()

while True:
    new_item = input('> ')

    # If the user inputs 'DONE' exit the loop
    if new_item == 'DONE':
        break
    # If the user inputs 'HELP' show the help documentation
    elif new_item == 'HELP':
        show_help()
        continue
    # if the user inputs 'SHOW' show the list
    elif new_item == 'SHOW':
        show_list()
        continue

    # Call add_to_list with new item as an argument
    add_to_list(new_item)

show_list()
\$\endgroup\$
1
\$\begingroup\$

Code Tells You How, Comments Tell You Why?

You have tried to document your code through comments, which is great! However, comments using # are mostly intended for other programmers, and is usually reserved for explaining terse parts of the code. A common way to document your code -- which is intended for the user -- is with PEP 257 -- Docstring Conventions.

As stated in the title comments (#) should be used sparsely. First you should first strive to make your code as simple as possible to understand without relying on comments as a crutch. Only at the point where the code cannot be made easier to understand should you begin to add comments. I would further break it down into three

  1. Clarify the intent of your code through functions following the single responsibility principle
  2. Clearify what each variable do through clear variable names that follows the standard python formating conventions
  3. If the "why" is not clear through the first two points, then, and only then do we add comments.

PEP8 - Style Guide for Python Code

As to pick the lowest hanging fruits. Consistency is a key part of programming, as it greatly increases readability. It also tells a bit about the coders eye for detail. Lets' look at how you have named your global constants:

shopping_list = []

Pep8 recommends the following naming-conventions:

  • CAPITALIZED_WITH_UNDERSCORES for constants
  • UpperCamelCase for class names
  • lowercase_separated_by_underscores for other names

Now to be fair you mostly follow this standard, which is excellent! I am just pointing it out that it is smart to follow these standards.


Handling user input

Jokes aside as a rule you should never except the user to use the correct input syntax. Instead you should build in checks, exceptions to handle these errors.

What happens if I write in help? A better way is to let a function handle the user input.

def get_valid_user_input():

    options = ["add", "done", "show"]
    print(MENU)
    while True: 
        choice = input("> ").lower()
        if choice not in options:
           print(f"Ooops, you need to enter one of the valid options {options}")
        else:
           return choice

Use of functions

A part that stuck out to me was

def show_help():
    print('What should we pick up at the store?')
    print("""
  Enter 'DONE' to stop adding items.
  Enter 'HELP' for additional info.
  Enter 'SHOW' to see your shopping list
  """)

A function should be a block of organized, reusable code that is used to perform a single, related action. Now, let us go through this checklist to see whether or not this should be a function

  • Is the function called more than twice? ✔️

  • Does the function serve a single purpose? ✔️

  • Does the function perform an action? ❌

    MENU = "What should we pick up at the store?\n\n
    Enter 'DONE' to stop adding items.\n
    Enter 'HELP' for additional info.\n
    Enter 'SHOW' to see your shopping list"
    

Since the answer to the last question was no, your function takes no input, so it could be a constant.

Note: As a general rule of thumb triple quotation marks should be reserved for docstrings and not be used for printing.


f-strings

f-strings are the new way of formating strings in Python, and you should usually use them. For instance

 print('{} was added to your shopping list!'.format(item))
 print('You have {} items on your list.'.format(len(shopping_list)))

becomes

 print(f"{item} was added to your shopping list!")
 print(f"You have {len(shopping_list)} items on your list.")

Nitpicking

  • Put the parts of your code that are the ones calling for execution behind a if __name__ == "__main__": guard. This way you can import this python module from other places if you ever want to, and the guard prevents the main code from accidentally running on every import.

A deeper look

 What should we pick up at the store?

 Enter 'DONE' to stop adding items.
 Enter 'HELP' for additional info.
 Enter 'SHOW' to see your shopping list

 >

This would be better if it read

 Enter 'DONE' to stop adding items.
 Enter 'HELP' for additional info.
 Enter 'SHOW' to see your shopping list

 What should we pick up at the store?
 >

Secondly what happens if I am unsure and type in help? I am then prompted with the same menu.

 What should we pick up at the store?

 Enter 'DONE' to stop adding items.
 Enter 'HELP' for additional info.
 Enter 'SHOW' to see your shopping list

 > HELP
 What should we pick up at the store?

 Enter 'DONE' to stop adding items.
 Enter 'HELP' for additional info.
 Enter 'SHOW' to see your shopping list

 >

Not much help there I suppose. While this is semantics, you are handling user input both as an item, which is confusing. For instance

# If the user inputs 'DONE' exit the loop
if new_item == "DONE":
    break

A better solution would be to use a different name, such as user_input and later change it to item, when it should be added to a cart. I present a slightly different way of handling this below.


My own attempt

  • I translated your comments into docstrings. They were well written and descriptive!
  • I included the if __name__ == "__main__": guard.
  • Bells and whistles I added typing hints, see PEP484.
  • Your shopping list is better structured as a simple class. This makes it easier to maintain and add functionality. (What if you wanted to remove an item from your shopping list?)
  • A more robust handling of user input. Try to write in something silly as input, what happens?
  • I've separate business logic from user interface by doing my printing in main and keeping ShoppingCart clean.
  • HELP was removed and instead a screen_clear() was added in addition to showing the menu every time.
  • A numerical menu was added as an user enchantment. Whether you think this menu style is an improvement is up to you. If you decide to keep your style, at the very least do input("> ").upper() to allow users to do add. Numerical inputs are faster to input, can be clearer, and are easier to expand to more options.
  • The menu and adding items were split into two functions (Remember, you used item for both).

code

import platform  # For getting the operating system name
import subprocess  # For executing a shell command


def clear_screen():
    """Clears the terminal screen."""

    # Clear command as function of OS
    command = "cls" if platform.system().lower() == "windows" else "clear"

    # Action
    return subprocess.call(command) == 0


MENU_OPTIONS = ["add item", "see your shopping list", "exit"]


def get_user_menu_choice() -> int:
    """Gives the user a menu to choose from"""
    low, high = 1, len(MENU_OPTIONS)
    error_msg = f"Woops, your input must be an integer in {low}...{high}\n"
    while True:
        print("Input a number from the list below:")
        for index, option in enumerate(MENU_OPTIONS):
            print(f" {1+index:>3d}: {option}")
        try:
            choice = int(input("\nPlease enter a number: "))
            if low <= choice <= high:
                return choice
        except ValueError:
            pass
        print(error_msg)


class ShoppingCart:
    def __init__(self):
        self.items = []

    def add(self, item: str):
        """Create a function that adds an item to the list"""
        if item != "":
            self.items.append(item)
        return item

    def get_items_from_user(self) -> None:
        clear_screen()
        print("\n\n\nWrite in the item you want to add to your shopping list!")
        print("[leave blank to return to menu]\n")

        item = self.add(input("> "))
        while item:

            clear_screen()
            print(f"{item} was added to your shopping list!")
            item_str = "items" if len(self.items) > 1 else "item"
            print(f"You have {len(self.items)} {item_str} in your list.")

            print("\nWrite in the item you want to add to your shopping list!")
            print("[leave blank to return to menu]\n")

            item = self.add(input("> "))

    def __str__(self) -> str:
        """Create a function to show all the items in the shopping list"""
        string = "My Shopping List:\n"
        for index, item in enumerate(self.items):
            string += f"\n{1+index:>3d}. {item}"
        return string + "\n"


def main() -> None:

    choice, add_item, display_items, exit_program = 0, 1, 2, 3

    cart = ShoppingCart()

    clear_screen()
    while choice != exit_program:
        choice = get_user_menu_choice()
        if choice == add_item:
            cart.get_items_from_user()
            clear_screen()
        elif choice == display_items:
            clear_screen()
            print(cart)


if __name__ == "__main__":
    main()
\$\endgroup\$
6
  • \$\begingroup\$ I don't see how your numeral menu is an improvement over the OP's strings. \$\endgroup\$ – Reinderien Jun 18 at 16:26
  • 2
    \$\begingroup\$ As a general rule of thumb triple quotation marks should be reserved for docstrings and not be used for printing is simply not true. Triple-quoted heredocs are useful any time that there's a multi-line string literal. \$\endgroup\$ – Reinderien Jun 18 at 16:26
  • \$\begingroup\$ @Reinderien Ì've had issues with tripple quotation marks using Sphinx, doctest and other programs that reads and handles docstrings. Which is why I avoid using them. For your second point writing 1, 2 or 3 is much faster, than having to type in exactly DONE. An improvement is to allow both, but that is outside the scope of this answer. \$\endgroup\$ – N3buchadnezzar Jun 18 at 16:34
  • \$\begingroup\$ It's fair to say that the numeral approach is a usability enhancement, but if so you should call it out in your answer and explain your theory as to why \$\endgroup\$ – Reinderien Jun 18 at 16:36
  • \$\begingroup\$ Also, SCREEN_CLEAR is not portable, and even for operating systems that support it, they might be in pipe mode for which this escape sequence will explode. \$\endgroup\$ – Reinderien Jun 18 at 16:37

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.