5
\$\begingroup\$

Q1. Can the following code be made more efficient and simple?

Q2. How is the general structure of the code?

Q3. Would this be better written in def/classes? If yes, what would the code look like?

Any other opinions/ tips are completely welcome. Thank you.

# Challenge 3 // Chapther 5 Python for The Absolute Beginner Book
# Python 3.4.3 // MAC 0SX (Latest) // 01.08.2017
from time import sleep 

pool_points = 30
menu_option = None
attribute_words = ("Strength, Health, Wisdom & Dexterity")
attributes = {"strength": 0,
              "health": 0,
              "wisdom": 0,
              "dexterity": 0}
GENERAL_MENU = ("""\t\tMENU
    0 = Exit and Final Result
    1 = Add Points to Attributes
    2 = Remove Points from Attributes
    3 = Print Menu
    """)

print("Welcome to Character Builder!")
print()
print(GENERAL_MENU)

while menu_option != "0":
    print()
    sleep(1) #provides user friendly reading
    menu_option = input("Please select an option: ")
    print()

    #exit
    if menu_option == "0":
        print("Creating Character Profile... ")

    #add
    elif menu_option == "1": 
        print("ADD Points To An Attribute!")
        print("Attributes Available:", attribute_words)
        print("Points Available:", pool_points)
        print()
        attribute_name = input("Attribute Name: ").lower()
        if attribute_name in attributes:
            value_points = int(input("Value: "))
            if value_points <= pool_points and value_points >= 0:
                pool_points -= value_points
                attributes[attribute_name] += value_points
                print("Points ADDED to,", attribute_name.title())
            else:
                print(value_points, "is an incorrect value. Try again...")
        else:
            print("Unfortunately,", attribute_name, "doesn't exist!")

    #remove
    elif menu_option == "2":
        print("REMOVE Points From An Attribute!")
        print("Attributes Available:", attribute_words)
        print("Points Available:", pool_points)
        print()
        attribute_name = input("Attribute Name: ").lower()
        if attribute_name in attributes:
            value_points = int(input("Value: "))
            if value_points <= int(attributes[attribute_name]) and value_points >= 0:
                pool_points += value_points
                attributes[attribute_name] -= value_points
                print("Points REMOVED from,", attribute_name.title())
            else:
                print(value_points, "is an incorrect value. Try again...")
        else:
            print("Unfortunately,", attribute_name, "doesn't exist!")

    #print menu
    elif menu_option == "3":
        print(GENERAL_MENU)

    #if all else fails!        
    else:
        print("INVALID! Select an option from the menu... ")
        print()
        print(GENERAL_MENU)

#final results
sleep(1)
print("-----------------------")
print("Points Remaining", "=>", pool_points, "|")
print("-----------------------")
for i in attributes.keys():
    print(i.title(), "=>", attributes[i], "\t       |")
    print("-----------------------")

input()
\$\endgroup\$
3
\$\begingroup\$

As for the overall structure of the code, I would try to avoid the if-elif-else chains. Instead you can use either a dictionary with a keys "0", "1", etc. and with functions such as exit_menu, add_points etc. or a list that stores just stores exit_menu add_points etc. in the order and in a particular order.

To go even beyond this, you could add a function attribute to exit_menu, add_points, etc. called display_name and generate the start menu by iterating through the list and loading each display_name.

Here is an (incomplete) example of what it should look like:

def create_profile():
    print("Creating Character Profile... ")

option_dispatch = [create_profile]

while menu_option != 0:
    menu_option = int(input("Please select an option: "))
    option_dispatch[menu_option]()
\$\endgroup\$
  • \$\begingroup\$ Hi @Dair Thank you for your prompt answer. Could you provide an example of how this code would look without the if-elif-else chains? As I don't totally understand how this would function? Thank you. Greg. \$\endgroup\$ – Greg Aug 1 '17 at 10:40
  • \$\begingroup\$ @Greg I added some code, basically it is what this should roughly look like if you had only the 0th option. \$\endgroup\$ – Dair Aug 2 '17 at 3:32

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.