14
\$\begingroup\$

Goal:

Write a character creator program for a role-playing game. The player should be given a pool of 30 points to spend on four attributes: strength, health, wisdom, and dexterity. The player should be able to spend points from the pool on any attribute and should also be able to take points from an attribute and put them back into the pool.

Python Programming for the Absolute Beginner by Michael Dawson

My attempt:

print "create a character! you have points to assign to strength, health, wisdom, and dexterity."
name=raw_input("what's your character's name? ")
points=30
attributes=("health", "strength", "wisdom", "dexterity")
strength=0
health=0
wisdom=0
dexterity=0
while True:
    print
    print "you have", points, "points left."
    print \
    """
    1-add points
    2-take points
    3-see points per attribute
    4-exit
    """
    choice=raw_input("choice: ")
    if choice=="1":
        attribute=raw_input("which attribute? strength, health, wisdom, or dexterity? ")
        if attribute in attributes:
            add=int(raw_input("how many points? "))
            if add<=points and add>0:
                if attribute=="strength":
                    strength+=add
                    print name, "now has", strength, "strength points."
                elif attribute=="health":
                    health+=add
                    print name, "now has", health, "health points."
                elif attribute=="wisdom":
                    wisdom+=add
                    print name, "now has", wisdom, "wisdom points."
                elif attribute=="dexterity":
                    dexterity+=add
                    print name, "now has", dexterity, "dexterity points."
                points-=add
            else:
                print "invalid number of points."
        else:
            print "invalid attribute."
    elif choice=="2":
        attribute=raw_input("which attribute? strength, health, wisdom, or dexterity? ")
        if attribute in attributes:
            take=int(raw_input("how many points? "))
            if attribute=="strength" and take<=strength and take>0:
                strength-=take
                print name, "now has", strength, "strength points."
                points+=take
            elif attribute=="health" and take<=health and take>0:
                health-=take
                print name, "now has", health, "health points."
                points+=take
            elif attribute=="wisdom" and take<=wisdom and take>0:
                wisdom-=take
                print name, "now has", wisdom, "wisdom points."
                points+=take
            elif attribute=="dexterity" and take<=dexterity and take>0:
                dexterity-=take
                print name, "now has", dexterity, "dexterity points."
                points+=take
            else:
                print "invalid number of points."
        else:
            print "invalid attribute."
    elif choice=="3":
        print "strength -", strength
        print "health -", health
        print "wisdom -", wisdom
        print "dexterity -", dexterity
    elif choice=="4":
        if points==0:
            break
        else:
            print "use all your points!"
    else:
        print "invalid choice."
print "congrats! you're done designing "+name+'.'
print name, "has", strength, "strength points,", health, "health points,", wisdom, "wisdom points, and", dexterity, "dexterity points."

I'm learning my first language, Python, and would really appreciate it if anyone could tell me if I'm writing the most efficient code possible.

|improve this question
\$\endgroup\$
  • 1
    \$\begingroup\$ what do you mean by efficient? Running speed? This kind of program will never be slow, you should focus more about readability than speed. and the mandatory link: c2.com/cgi/wiki?PrematureOptimization \$\endgroup\$ – João Portela Jan 25 '12 at 18:25
  • \$\begingroup\$ I meant readability and efficiency in expression (am I writing more than I need to to achieve my goal?), but thanks anyway for the interesting link! \$\endgroup\$ – krushers Jan 25 '12 at 23:34
  • \$\begingroup\$ You may notice that when entering incorrect input, the error handling of this solution is not great - while my solution is class-based the error-handling could be applied to either representation \$\endgroup\$ – theheadofabroom Feb 6 '12 at 14:21
8
\$\begingroup\$

One of the first things I would do is try and group your information into neater packages, rather than having a bunch of free variables. I assume you're not too familiar with classes, but try putting your character's attributes into a data structure like a List, or even better - a Dictionary:

attributes = {'health': 0, 'strength': 0, 'wisdom': 0, 'dexterity': 0 }

If you want to change attributes, you can then do

attributes['health'] = some_value

or to increment,

attributes['health'] += some_value

Secondly, the best way to improve your program is to make it more readable by splitting your code up into functions. For instance, the code below is an example of what it may look like if you took some of the stat changing logic out of the main program:

if choice=="1":
    stat = input("Which stat? ")
    modifier = input("By how much? ")
    add_stats(modifier, stat)
elif choice=="2":
    stat = input("Which stat? ")
    modifier = input("By how much? ")
    take_stats(modifier, stat)
elif choice=="3":
    print(show_player_stats())

In general, any where you find yourself getting "too deep" in nests of if statements and loops, or you find yourself repeating your code, try and break it out into a function. Bear in mind this is a crude example, and you'll have to develop your own solution, but in terms of readability it's a vast improvement.

I've made a start on a version of your game using my own approach, although I haven't implemented all of it (you can't subtract points for instance). However, you can probably already see where the program has improved on things.

Things to note about my version compared to yours:

  • There is commenting (although basic). Commenting code is a MUST, even if it's for yourself. It will help you understand your own code and help anyone else who uses it, even if it's just to show different parts of the program.
  • The code is broken down into functions, which improves readability and allows you to re-use bits of code in the future. For instance, every time you want to print your character's info, you just call print_character()!
  • The code is neater - I'm packing information into a data structure, strings are formatted with linebreaks, logic is compartmented into smaller, manageable chunks.

However, I have used some Python you may not be familiar with, like the keys() method. But it's important to go through the code and try and work out what's happening. This will help to expose you to the "Python" way of doing things.

##### GAME FUNCTIONS #####

def add_character_points():
    attribute = raw_input("\nWhich attribute? Strength, Health, Wisdom or Dexterity?\n")

    if attribute in my_character.keys():
        amount = int(raw_input("By how much?"))

        if (amount > my_character['points']) or (my_character['points'] <= 0):
            print "Not enough points!"
        else:
            my_character[attribute] += amount
            my_character['points'] -= amount
    else:
        print "\nThat attribute doesn't exist!\n"


def print_character():
    for attribute in my_character.keys():
        print attribute, " : ", my_character[attribute]


##### MAIN FUNCTION #####

my_character = {'name': '', 'strength': 0, 'health': 0, 'wisdom': 0, 'dexterity': 0, 'points': 20}
running = True

print "Create a character! You have points to assign to strength, health, wisdom, and dexterity."

my_character['name'] = raw_input("What is your character's name? ")

while running:
    print "\nYou have ", my_character['points'], " points left.\n"
    print "1. Add points\n2. Remove points\n3. See current attributes\n4. Exit\n"

    choice = raw_input("Choice:")

    if choice == "1":
        add_character_points()
    elif choice == "3":
        print_character()
    elif choice == "4":
        running = False         
    else:
        pass
|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Wow, thank you so much. I'm just learning about dictionaries (and their methods) and functions, so thanks for showing me these great ways to implement them. I will definitely comment my code from now on. One question though: why did you use the "running" variable to control the loop? Why not create an infinite loop with "while True" and end it with "break" when the user chooses "4"? Is it for readability? Thanks again. \$\endgroup\$ – krushers Jan 25 '12 at 23:30
  • \$\begingroup\$ @krushers In this particular scenario there's no difference between using "while True" and breaking, but a break would exit the loop immediately where as running = False would execute any code after the if statement. But the main reason is, as you say, while running and running = False is perhaps more readable. Nothing beats a good comment if you're worried something is ambiguous, though! \$\endgroup\$ – persepolis Jan 25 '12 at 23:42
  • \$\begingroup\$ Might I just also re-iterate as a caveat that my code example is not by any means the best way to do this, I've tried to keep it simple so you can get a general "idea" of the kind of way to structure your program. One thing you might notice is that my_character is not passed as the argument to a function, when in a larger program this might not be such a good idea... \$\endgroup\$ – persepolis Jan 25 '12 at 23:49
  • 1
    \$\begingroup\$ I think you have a small mistake in your code, where you wrote (amount > my_character['points']) or (my_character['points'] <= 0) I think you actually meant to write, (amount > my_character['points']) or (amount <= 0), correct? \$\endgroup\$ – João Portela Jan 26 '12 at 10:11
  • \$\begingroup\$ @JoãoPortela : Actually, I did intend to write that, but your version of the code is an improvement because it covers mine plus the case where amount is nonpositive. But like I say, I threw this together to demonstrate the overall structural improvement for the code, not as a complete solution. \$\endgroup\$ – persepolis Jan 26 '12 at 13:32
3
\$\begingroup\$

I'm also a beginner in programming. Here's how I would have done it. It's probably not any better than yours at all, but maybe slightly more readable. Good answer from persepolis btw!

points = int(30)
attributes = {u'strength': 0, u'health': 0, u'wisdom': 0, u'dexerity': 0}
functions = [u'add points', u'take points', u'see points', u'exit']

def add_points():   #Function that let the user add points to the attributes
    global points
    print u"You have " + unicode(points) + u" points left"
    choice = raw_input(u"Which attribute? Strength, health, wisdom, or dexterity? ")
    print u"\n"
    if choice in attributes:            #If the user types a valid attribute
        add = int(raw_input(u"How many points? "))
        if add <= points:               #If there is enough points left
            attributes[choice] += add
            points -= add
            print choice + u': ' + unicode(attributes[choice])
        else:
            print u"Invalid number of points"
    else:
        print u"Invalid attribute"

def sub_point():    #Function that let the user add points to the attributes
    choice = raw_input(u"Which attribute? Strength, health, wisdom, or dexterity? ")
    if choice in attributes:            #If the user types a valid attribute
        sub = int(raw_input(u"How many points? "))
        if sub >= attributes[choice]:   #If there is enough points in that attribute
            attributes[choice] -= sub
            points += sub
            print choice + u': ' + unicode(attributes[choice])
        else:
            print u"Invalid number of points"
    else:
        print u"Invalid attribute"

def see_points():   #Function that let the user see the attributes and points
    for x in attributes:
        print x + u": " + unicode(attributes[x])

# Main loop

name = raw_input(u"What is your characters name? ")
print u"Hi " + name
while True:
    print u"\n"
    print u"Would you like to add points, take points, see points per attribute or exit?"
    print u"\n"
    for x in functions:     #Prints out the user's choices
        print x + u': ' + u'press ' + unicode(functions.index(x) + 1)
    print u"\n"
    func = int(raw_input())
    if func == 1:
        add_points()
    if func == 2:
        sub_points()
    if func == 3:
        see_points()
    if func == 4:
        break
print u"Congratz, you have created a caracter!"

I apologize for small faults in the function. I use 3.2 and converted it to 2.x

|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Cool, thanks for your answer! I really like your use of functions; makes the final product much easier to understand. I have a few questions: Is there any reason you used "int(30)" to define "points"? I think you can just say "points=30" and Python would understand that it's an integer value. Also, why are there "u"s before all your strings? Lastly, what is the purpose of the "global" statement and the "unicode()" method? I looked them up online but couldn't really understand the point of using them in this context. Thanks again for your answer. I appreciate it. \$\endgroup\$ – krushers Jan 26 '12 at 1:23
  • \$\begingroup\$ For some weird reason, python wouldn't accept it without the int(). I usally dont use it. the "u"'s is something that happens in the conversion from 3.x to 2.x. There is some differences in unicode/string and especially the print() function. The global thing should also not be neccesary, but something was wrong with python(or me) today. \$\endgroup\$ – Martin Hallén Jan 26 '12 at 5:28
  • \$\begingroup\$ @mart0903 global points would be required before modifying points, as x += 1 expands to x = x + 1. If x is in the global scope, then at the start of that expression it defines a new local variable x, and then assigns it the value of the global variable x plus 1. This is because integars are immutable, and so you are not in fact modifying points, but changing what the name resolves to within your current scope. Using the global keyword explicitly tells python how to resolve points. \$\endgroup\$ – theheadofabroom Feb 6 '12 at 11:47
1
\$\begingroup\$

While the answer by persepolis is great for this small task, if this were for part of a game where you were going to use this data, you may wish to do something like this:

class Character(object):
    name        = ''
    strength    = 0
    health      = 0
    wisdom      = 0
    dexterity   = 0
    points      = 20
    _attributes = ['name', 'strength', 'health', 'wisdom', 'dexterity', 'points']
    def __init__(self, name):
        assert self.valid_name(name)
        self.name = name
    def add_points(self):
        accepted = ['strength', 'health', 'wisdom', 'dexterity']
        accepted_dict = dict(enumerate(accepted, start=1))
        prompt = "\nWhich attribute?\n\t" + "\n\t".join("%d. %s"%n for n in accepted_dict.items())+"\n?"
        attribute = False
        while attribute not in accepted:
            attribute = raw_input(prompt)
            try:
                attribute = accepted_dict[int(attribute)]
            except:
                print "Input was invalid. Please enter either an attribute or its corresponding number"
        amount = None
        while type(amount) != int or amount > self.points:
            try:
                amount = int(raw_input("By how much?"))
                assert amount <= self.points
            except AssertionError:
                print "You do not have that many points remaining"
            except:
                print "You must enter an integer amount"
        self.__setattr__(attribute, self.__getattribute__(attribute) + amount)
        self.points -= amount
    def __str__(self):
        return "\n".join("%s\t:\t%s"%(n, self.__getattribute__(n)) for n in self._attributes)
    @staticmethod
    def valid_name(name):
        if bool(name) and type(name) == str:
            return True
        else:
            return False
if __name__ == "__main__":
    running = True
    print "Create a character! You have points to assign to strength, health, wisdom, and dexterity."
    name = ''
    while not Character.valid_name(name):
        name = raw_input("Please enter your character's name:")
    CHAR = Character(name)
    OPTIONS_LIST = ["Add points", "Remove points", "See current attributes", "Exit"]
    OPTIONS_DICT = dict(enumerate(OPTIONS_LIST, start=1))
    PROMPT = "\n".join("\t%d. %s"%n for n in OPTIONS_DICT.items())+"\nChoice:"
    while running:
        CHOICE = raw_input(PROMPT)
        try:
            CHOICE = int(CHOICE)
        except:
            pass
        if CHOICE in OPTIONS_DICT.keys():
            CHOICE = OPTIONS_DICT[CHOICE]
        if CHOICE == "Add points":
            CHAR.add_points()
        elif CHOICE == "Remove points":
            raise NotImplementedError()
        elif CHOICE == "See current attributes":
            print CHAR
        elif CHOICE == "Exit":
            running = False

As the character is now an object it can be passed around. Notice also how use of enumerate can give more intuitive menus - these will accept either the number, or the text. Using the text in your if ... elif ... elif ... else block also helps the clarity of the code. For instance while typing the above code out I put the NotImplementedError in the wrong bit - when I re-read the code it was obvious I had made a mistake. While this code is not perfect, it may give an idea of slightly better practices that will help you when you attack larger projects.

|improve this answer
\$\endgroup\$

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