1
\$\begingroup\$

This is my code. It loops a menu, from which you can select whether you want to add a subject, see all subjects or delete a subject. Is there anything style- or functionality-wise to add or correct?

def add_subject(subjects):
    
    # ask for subject to add
    add = input("\nWhat subject do you want to add? ")

    # if subject not already in list, add subject
    subjects.append(add) if not add in subjects else print("\nSubject already in list.")

def show_subjects(subjects):
    
    # show all subjects
    print("\nThese are your subjects: ", subjects)

def delete_subject(subjects):
    
    # ask for subject to delete
    delete = input("\nPlease enter subject to delete: ")

    # if subject is in list, delete subject
    subjects.remove(delete) if delete in subjects else print("\nSubject not found.")

def main():
    
    # initialise list of sample subjects
    subjects = ["Math","Science","English"]

    # loop action cycle
    while 1:
        try:
            # ask user for selection
            selection = int(input("\nPlease enter option: \n(1) Add subject\n(2) Show all subjects\n(3) Delete a subject\n(4) Quit\n"))

            # call associated function
            if selection == 1:
                add_subject(subjects)
            elif selection == 2:
                show_subjects(subjects)
            elif selection == 3:
                delete_subject(subjects)
            elif selection == 4:
                break
            else:
                raise ValueError

        except ValueError:
            print("\nNot a viable option. Please pick 1, 2 or 3.")



if __name__ == "__main__":
    main()

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Clear the screen

Here is the terminal after three iterations enter image description here

We can clean a lot of this up by simply doing

print(chr(27) + "[2J")

after every iteration.

def clear_scr():
    print(chr(27) + "[2J")

Note that on adding this, you should also do

input("Press any key to continue...") 

After the user selects an option, this way the user can wait and see what is on the screen peacefully without the program interrupting

def wait():
    input("Press any key to continue...")
    print(chr(27) + "[2J")

Or a better version that also prints a confirmation message

def wait(message):
    input(f"{message}. Press any key to continue...")
    print(chr(27) + "[2J")



wait("Subject Added!")

>>> Subject Added! Press any key to coninue... 

Unnecessary comments

# ask user for selection
selection = int(input("\nPlease enter option: \n(1) Add subject\n(2) Show all subjects\n(3) Delete a subject\n(4) Quit\n"))

A comment here is totally not required, it only makes the code look convoluted for no reason. What I do suggest is the usage of docstrings.

def wait():
    """
    Wait for the user to press any key then clear the screen
    """
    ...

Print the subjects in a better fashion

I added a few more subjects in your program. When I entered Show all subjects.

These are your subjects:  ['Math', 'Science', 'French', 'Computer', 'Hindi', 'Social Science', 'Spanish', 'English']

This isn't nice at all, I suggest you print one on each line, with numbering, something like

Subjects

1. Math
2. Science
3. French
3. Computer
...
def show_subjects(subjects):
    """
    print all the subjects line-by-line 
    """
    print("\n\nSubjects\n")
    for i in range(len(subjects)):
        print(f"{i+1}. {subjects[i]}")

Don't use magic numbers

            if selection == 1:
                add_subject(subjects)
            elif selection == 2:
                show_subjects(subjects)
            elif selection == 3:
                delete_subject(subjects)
            elif selection == 4:
                break
            else:
                raise ValueError

1, 2, 3, and 4 are unnamed numeric constants here. They don't really really tell anything about what the branch does. What if it was instead

            if selection == Choices.add.value:
                add_subject(subjects)
            elif selection == Choices.show.value:
                show_subjects(subjects)
            elif selection == Choices.delete.value:
                delete_subject(subjects)
            elif selection == Choices.quit.value:
                break
            else:
                raise ValueError

Without reading anything inside the if block, I can tell exactly what its going to do. That is an example of clean code.

You can achieve this by using an enum

from enum import Enum

class Choices(Enum):
    add = 1
    show = 2
    delete = 3
    quit = 4

print(Choices.add.value) 
>> 1

print(Choices.quit.value)
>> 4

Colored text

This one is optional, but it adds a lot of meaning to your code :).
You can use the coloroma module to color your text to different colors in an easy way. For example, when you find that the user has entered bad input, you can do

enter image description here

Isn't that great! The best part about it is that it is cross-platform

from colorama import Fore, Back, Style
print(Fore.RED + 'some red text')
print(Back.GREEN + 'and with a green background')
print(Style.DIM + 'and in dim text')
print(Style.RESET_ALL)
print('back to normal now')

You have a lot of options, it can help you customize the output a lot. Read the documentation

As suggested by @Reinderien, using the colorama library you can clear the screen too

import colorama as cr
cr.init()

print(cr.ansi.clear_screen()) # clears the screen

Re-written

from colorama import Fore, Back, Style
import colorama as cr
from enum import Enum

cr.init()


class Choices(Enum):
    add = 1
    show = 2
    remove = 3
    quit = 4


def wait(message = None):
    if message is None:
        input("Press any key to continue...")
    else:
        print(Fore.GREEN)
        print(Style.BRIGHT)

        print(message, end = '')

        print(Style.RESET_ALL)
        input("Press any key to continue...")

    print(cr.ansi.clear_screen())

def add_subject(subjects):
    add = input("\nWhat subject do you want to add? ")
    if add not in subjects:
        subjects.append(add)
        wait("Subject Added!")
    else:
        print("Subject already present")
        wait()


def show_subjects(subjects):
    print("Subjects\n")
    print(Fore.YELLOW)
    print(Style.BRIGHT)
    for i in range(len(subjects)):
        print(f"{i+1}. {subjects[i]}")
    wait()

def delete_subject(subjects):
    delete = input("\nPlease enter subject to delete: ")
    if delete not in subjects:
        print("Subject not found")
        wait()
    else:
        subjects.remove(delete)
        wait("Subject removed!")

def main():
    subjects = ["Math","Science","English"]
    while True:
        try:
            selection = int(input("\nPlease enter option: \n(1) Add subject\n(2) Show all subjects\n(3) Delete a subject\n(4) Quit\n"))
            if selection == Choices.add.value:
                add_subject(subjects)
            elif selection == Choices.show.value:
                show_subjects(subjects)
            elif selection == Choices.remove.value:
                delete_subject(subjects)
            elif selection == Choices.add.value:
                break
            else:
                raise ValueError

        except ValueError:
            print(Fore.RED)
            print("\nNot a viable option. Please pick 1, 2 or 3.")
            print(Style.RESET_ALL)
            wait()



if __name__ == "__main__":
    main()

g

\$\endgroup\$
12
  • \$\begingroup\$ I think dataclass over enum for Choices here would be cleaner \$\endgroup\$
    – hjpotter92
    Commented Nov 6, 2020 at 17:12
  • \$\begingroup\$ @hjpotter92 I have to research more about data classes before recommending it, can you give an example? \$\endgroup\$
    – user228914
    Commented Nov 6, 2020 at 17:21
  • \$\begingroup\$ Adding coloured text and clearing the screen are fine, but tightly coupling to raw ANSI escape sequences is a bad idea. Consider using something like github.com/tartley/colorama instead. \$\endgroup\$
    – Reinderien
    Commented Nov 6, 2020 at 17:28
  • \$\begingroup\$ @AryanParekh as an example: tio.run/##Rc29CoUwDAXgvU@RURfBn0kQBJ@k2EgLamoTkfv0vaVYzBByvuHE/… (and the docs: devdocs.io/python~3.8/library/dataclasses#dataclasses.dataclass) \$\endgroup\$
    – hjpotter92
    Commented Nov 6, 2020 at 17:30
  • 1
    \$\begingroup\$ @Reinderien Added the clear_screen call from the colorama library, I didn't expect it to have it earlier \$\endgroup\$
    – user228914
    Commented Nov 6, 2020 at 18:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.