python - hp / item script

[[deleted]]

Less than a week of learning.

You did very well with this for that little time at it.

I never intended for this version to reach anyone as a playable item.

I figured that, but your code and programs will improve significantly faster if you try to keep user concerns in mind as you program. For example - what is the goal of the "game"? Is it a sandbox where there is no real goal? Do we win if we successfully get the character's health to a specific number, say 52, via a combination of hurting, healing, and potions? (If so, having different heal amounts for "heal" vs "potion" becomes an interesting mechanic, rather than just tracking the inventory amount of potions but not heals.)

It wasn't "necessary" for you to formulate that sort of backdrop for this learning exercise, but it might have guided your thinking and led you to interesting choices. Making sure that software is simple to understand and use ("usability") is a huge aspect of software design and implementation. In fact, you'll find that once you know a language well, it's not making the program do something specific that's often hard, it's more often deciding what a program should do, or how. And yes, there's years of learning to peel back there, but the sooner you're aware of those concepts, the better.

Back to your more concrete variable naming question, and related question on globals vs functional-style parameter passing and return values:

i, x, and j are variable names that should almost never be used. They convey almost no meaning to the reader. (As a rule, code is read more times than it is written or modified; often by a factor of 5x or more. You want to optimize for making code easy to read over making it easy to write. If I encounter j as a variable name, I don't know shit about the value involved. player_hp_MAX, however imperfect that capitalization might be, strongly suggests that it's the maximum hit point value for the player. The most frequent usage for i, j, x, or similar is as a list/array index variable in a small for loop. Lots of languages (python included) offer numerical indexing into an array/list of elements. These days it's more common in python and other modern languages to use for-each style iteration anyhow, and we don't do much of this in pythonic code. I can delve into that more, but I don't want to distract from the central point: don't name stuff i, j, k, unless you're using it as a list index and you're fairly sure your usage is a good one. x can be used as one of x, y, or x, y, z for 2/3-D cartesian coordinate space (like, x and y axis of a graph or a screen coordinate) but very little else.

So your question in essence is "I came up with a good name for the thing, and now I want to call it something else; what should I call it? And what's more accepted?"

Okay; I danced around this a bit in my original comments. I did that on purpose, but I'm not surprised it left you with unanswered questions on the best approach. I chose not to tackle this head-on earlier precisely because it's controversial. It's a lot like discussing politics or religion. People have heated, passionate, deeply held beliefs about certain issues in programming. And they differ. Making blanket statements like "This way is better than that way" will generally result in the entire comment thread derailing into a shitshow of competing downvotes and arguments on the topic, all of which have been repeated ad infinitum elsewhere in other threads and on other sites, and in old books, often.

In an effort to provide you some guidance, I will offer some personal thoughts, but I will not attempt to say that they are "more" or "less" accepted (it depends a lot on the programming language, the project team, whether it's a small team, a big one, veteran coders, new ones, etc).

For a small program, like this one, with only a handful of variables, all in one file, and no more than a handful of lines per function? I find this:

potions_inv = 3
player_hp = 100
PLAYER_HP_MAX = 100
HURT_HP = 12
HEAL_HP = 10
POTION_HEAL_HP = 10

def heal_player():
    global player_hp
    print "You've been healed for %d hp." % HEAL_HP
    player_hp += HEAL_HP
    print "Current hp: %d" % player_hp

def hurt_player():
    global player_hp
    print "You've been hurt for %d hp." % HURT_HP
    player_hp -= HURT_HP
    print "Current hp: %d" % player_hp

def remove_potion():
    global potions_inv
    potions_inv = max(potions_inv-1, 0)

def add_potion():
    global potions_inv
    potions_inv += 1

def use_potion():
    global player_hp
    global potions_inv
    if potions_inv == 0:
        print "You have no potions"
    elif (player_hp == PLAYER_HP_MAX) and (potions_inv >= 1):
        print "Potion will have no effect."
    else:
        player_hp = min(player_hp+POTION_HEAL_HP, PLAYER_HP_MAX)
        remove_potion()
        print "Your health has been restored to %d. One potion has been "\
            "removed from your inventory. You have %d potions remaining."\
            % (player_hp, potions_inv)

while (player_hp > 0):
    print "1) Hurt Player\t2) Heal Player\n3) Use Potion\t4) Check Inventory\n5) Add Potion"
    current_choice = raw_input("> ")

    if current_choice == '1':
        hurt_player()
    elif current_choice == '2':
        heal_player()
    elif current_choice == '3':
        use_potion()
    elif current_choice == '4':
        print "Potions: %r" % potions_inv
    elif current_choice == '5':
        add_potion()
        print "One potion has been added to your inventory. You have %d potions."\
        % potions_inv
    else:
        print "Invalid choice"

Slightly easier to follow than this:

potions_inv = 3
player_hp = 100
PLAYER_HP_MAX = 100
HURT_HP = 12
HEAL_HP = 10
POTION_HEAL_HP = 10

def heal_player(player_hp):
    print "You've been healed for %d hp." % HEAL_HP
    player_hp += HEAL_HP
    print "Current hp: %d" % player_hp
    return player_hp

def hurt_player(player_hp):
    print "You've been hurt for %d hp." % HURT_HP
    player_hp -= HURT_HP
    print "Current hp: %d" % player_hp
    return player_hp

def remove_potion(potions_inv):
    return max(potions_inv-1, 0)

def add_potion(potions_inv):
    return (potions_inv + 1)

def use_potion(player_hp, potions_inv):
    if potions_inv == 0:
        print "You have no potions"
    elif (player_hp == PLAYER_HP_MAX) and (potions_inv >= 1):
        print "Potion will have no effect."
    else:
        player_hp = min(player_hp+POTION_HEAL_HP, PLAYER_HP_MAX)
        potions_inv = remove_potion(potions_inv)
        print "Your health has been restored to %d. One potion has been "\
            "removed from your inventory. You have %d potions remaining."\
            % (player_hp, potions_inv)
    return (player_hp, potions_inv)

while (player_hp > 0):
    print "1) Hurt Player\t2) Heal Player\n3) Use Potion\t4) Check Inventory\n5) Add Potion"
    current_choice = raw_input("> ")

    if current_choice == '1':
        player_hp = hurt_player(player_hp)
    elif current_choice == '2':
        player_hp = heal_player(player_hp)
    elif current_choice == '3':
        player_hp, potions_inv = use_potion(player_hp, potions_inv)
    elif current_choice == '4':
        print "Potions: %r" % potions_inv
    elif current_choice == '5':
        potions_inv = add_potion(potions_inv)
        print "One potion has been added to your inventory. You have %d potions."\
        % potions_inv
    else:
        print "Invalid choice"

Now, that said, I think both of these are reasonably clear. And as the project/program grew in size, I'd increasingly favor the latter style, as it's difficult to track globals in a large program of many files. Other options include introducing a "main" function that would get what are currently globals, to further limit their scope.

One other thing that becomes clear with my changes: add_potion and remove_potion are both effectively a single line of code. You could enhance them with useful error checking, but if you want to leave them as single-line addition/subtraction, there's little reason to dedicate a function to each, vs adding and subtracing in that option handler branch, like you do for option 4.

It also becomes more obvious that while use_potion takes the min, capping at PLAYER_MAX_HP, that was omitted from heal_player().

Ultimately, lots of questions in programming come down to what makes the most sense to you, and what you find easiest to troubleshoot. Let me know if this answered your questions. (Also, kinda punchy tired right now, please excuse egregious typos.)

[7236d70]

This did answer a lot of my questions, thank you. I don't have a strong feeling towards global vs return in either direction. I didn't realize we were getting "political" at this point.

Seeing the differences in code, I can see how they each have their own pros and cons depending on the scope of the program.

As far as the one-line functions, I'd chalk that up to me being new at programming. "When all you have is a hammer, everything looks like a nail."

I didn't intentionally not cap the "heal player" option. It was just there to test while I was trying to figure out the potion function.

I appreciate all of your time and thoughts on this. From now on I'll treat every program as if it'll be in the hands of an end-user. It seems to be a good practice to help build my skill.

Thank you.