89

I am working on a simple video game program for school and I have created a method where the player gets 15 health points if that method is called. I have to keep the health at a max of 100 and with my limited programming ability at this point I am doing something like this.

public void getHealed(){
    if(health <= 85)
        health += 15;
    else if(health == 86)
        health += 14;
    else if(health == 87)
    health += 13; 
}// this would continue so that I would never go over 100

I understand my syntax about isn't perfect but my question is, what may be a better way to do it, because I also have to do a similar thing with the damage points and not go below 0.

This is called saturation arithmetic.

phuclv
  • 37,963
  • 15
  • 156
  • 475
  • 7
    As a side note, I would choose a different name for this method. Per Java standard (and most other languages I know of), a method name starting with "get" should return a value, and not change anything. Likewise method names starting with "set" should change a value and usually not return anything. – Darrel Hoffman Sep 07 '13 at 01:58

14 Answers14

227

I would just do this. It basically takes the minimum between 100 (the max health) and what the health would be with 15 extra points. It ensures that the user's health does not exceed 100.

public void getHealed() {
    health = Math.min(health + 15, 100);
}

To ensure that hitpoints do not drop below zero, you can use a similar function: Math.max.

public void takeDamage(int damage) {
    if(damage > 0) {
        health = Math.max(health - damage, 0);
    }
}
Chris Forrence
  • 10,042
  • 11
  • 48
  • 64
70

just add 15 to the health, so:

health += 15;
if(health > 100){
    health = 100;
}

However, as bland has noted, sometimes with multi-threading (multiple blocks of code executing at once) having the health go over 100 at any point can cause problems, and changing the health property multiple times can also be bad. In that case, you could do this, as mentioned in other answers.

if(health + 15 > 100) {
    health = 100;
} else {
    health += 15;
}
Jordan
  • 1,433
  • 8
  • 14
  • 9
    Should never allow it to go over, this could introduce new problems such as a race condition where character health is assumed to be at most the defined health max (100). Unlikely for this level project I'd guess but should enforce good practices early on. – bland Sep 06 '13 at 12:52
  • 6
    @bland If one were to use this approach, a way of avoiding such a race condition would be to use a temporary variable to store the new health value, and then set health to this new health value in one place, with synchronisation if necessary. – Bob Sep 06 '13 at 15:20
  • 13
    @bland: Race conditions are only relevant when multi-threading. And if he is doing multi-threading *(which I highly doubt)*, the solution would be to lock on all accesses to `health`, or to make sure `health` is only accessed from one thread. The constraint *"Should never allow health to go over 100"* is not a realistic one. – BlueRaja - Danny Pflughoeft Sep 06 '13 at 16:15
  • @BlueRaja-DannyPflughoeft I stated it was unlikely for this project. In general locks are not going to always be in use and if you have a max value then it should be strictly adhered to, gaming or otherwise. Example: If an event is tied to a change of a property, and uses percentage then you've now skewed that processing immensely, as well as having it invoked twice. I'm trying to stay general here and ensure OP learns - I feel this answer, while it works, is too narrow and specific for a student as it will enforce him to not think of the big picture. – bland Sep 06 '13 at 16:30
  • @Bob i think thats unnecessary for something this simple. – Math chiller Sep 22 '13 at 18:51
44

You don't need a separate case for each int above 85. Just have one else, so that if the health is already 86 or higher, then just set it directly to 100.

if(health <= 85)
    health += 15;
else
    health = 100;
rgettman
  • 176,041
  • 30
  • 275
  • 357
  • 25
    A bit too much magic numbers for me (even considering 100 allowed) - when changing 15 to 16 the 85 would need to be adjusted. Wouldn't changing 85 to at least `100 - 15` (or `100 -HEALED_HEALTH`) be an improvement? – Maciej Piechotka Sep 06 '13 at 05:59
37

I think an idiomatic, object oriented way of doing this is to have a setHealth on the Character class. The implementation of that method will look like this:

public void setHealth(int newValue) {
    health = Math.max(0, Math.min(100, newValue))
}

This prevents the health from going below 0 or higher than 100, regardless of what you set it to.

Your getHealed() implementation can just be this:

public void getHealed() {
    setHealth(getHealth() + 15);
}

Whether it makes sense for the Character to have-a getHealed() method is an exercise left up to the reader :)

Daniel Kaplan
  • 62,768
  • 50
  • 234
  • 356
  • 5
    +1: This is a great way to do this in an object-oriented manner! The only thing I might suggest (and this would probably be left to the reader) is to possibly have two methods (`heal(int hp)` and `damage(int hp)`) which each call your `setHealth(int newValue)` method. – Chris Forrence Sep 05 '13 at 23:17
  • This is a very nice solution for this +1 – Jordan Sep 05 '13 at 23:38
  • Calling two library functions (min and max) to avoid a simple `if` is an overkill. – Matteo Sep 06 '13 at 05:26
  • 18
    What is wrong with calling library functions? As named functions, they express intent more clearly than a bunch of conditional logic. – erickson Sep 06 '13 at 06:19
  • 2
    Also many library functions (and very likely these ones) are really built-in and won't perform any call at all. – kriss Sep 06 '13 at 07:49
  • 2
    @Matteo Wrong answer - most likely the library function does exactly the same thing internally, so why repeat yourself and pollute your code? NOT using library functions doesn't follow the basic principles of DRY. – NickG Sep 06 '13 at 09:29
  • @Matteo Would you prefer `newValue > 100 ? 100 : newValue < 0 ? 0 : newValue`? That's a **LOT** less readable. – tckmn Sep 06 '13 at 13:02
  • 1
    Not because of the internals, but with both max and min the statement is not really readable. Just my opinion but the if is more readable. – Matteo Sep 06 '13 at 14:09
  • @Doorknob No I would find an if much more readable (but it's just my opinion I'm not saying that the solution is wrong) – Matteo Sep 06 '13 at 14:09
  • 2
    @Matteo this is not to avoid an `if`. This is to prevent yourself from being able to shoot yourself in the foot. If it's too verbose, just use static imports. Then it looks like this: `health = max(0, min(100, newValue))` If that's still unreadable to you, extract it to a method named `clamp` so the line looks like this: `health = clamp(0, 100, newValue)` – Daniel Kaplan Sep 06 '13 at 17:49
14

I am just going to offer a more reusable slice of code, its not the smallest but you can use it with any amount so its still worthy to be said

health += amountToHeal;
if (health >= 100) 
{ 
    health = 100;
}

You could also change the 100 to a maxHealth variable if you want to add stats to the game your making, so the whole method could be something like this

private int maxHealth = 100;
public void heal(int amountToHeal)
{
    health += amountToHeal;
    if (health >= maxHealth) 
    { 
        health = maxHealth;
    }
}

EDIT

For extra information

You could do the same for when the player gets damaged, but you wouldn't need a minHealth because that would be 0 anyways. Doing it this way you would be able to damage and heal any amounts with the same code.

5tar-Kaster
  • 910
  • 2
  • 12
  • 30
  • 1
    `minHealth` might be negative, say for example, in D&D... :) – JYelton Sep 06 '13 at 07:43
  • 1
    By far the best answer here. – Glitch Desire Sep 06 '13 at 10:42
  • @JYelton, yeah you'd just say (health <= 0) in the if statement. You could handle it however you want, if you want them to have lives you just minus 1 from lifeCount or just if they flat out lose then it can handle that. If you wanted to you could even make them start their new life with the amount of -HP they had. You can paste this code in pretty much anywhere and it'll do the job just fine, that was the point of this answer. – 5tar-Kaster Sep 16 '13 at 08:34
10
health = health < 85 ? health + 15 : 100;
Aroo
  • 198
  • 7
4

I would make a static method in a helper class. This way, rather than repeating code for every value which need to fit within some boundaries, you can have one all purpose method. It would accept two values defining the min and max, and a third value to be clamped within that range.

class HelperClass
{
    // Some other methods

    public static int clamp( int min, int max, int value )
    {
        if( value > max )
            return max;
        else if( value < min )
            return min;
        else
            return value;
    }
}

For your case, you would declare your minimum and maximum health somewhere.

final int HealthMin = 0;
final int HealthMax = 100;

Then call the function passing in your min, max, and adjusted health.

health = HelperClass.clamp( HealthMin, HealthMax, health + 15 );
MildWolfie
  • 2,492
  • 1
  • 16
  • 27
3

I know this is a school project, but if you wanted to expand your game later on and be able to upgrade your healing power, write the function like so:

public void getHealed(healthPWR) {
    health = Math.min(health + healthPWR, 100);
}

and call out the function:

getHealed(15);
getHealed(25);

...etc...

Furthermore you can create your max HP by creating a variable that is not local to the function. Since I don't know what language you're using, I will not show an example because it might have the wrong syntax.

BlackBeltScripting
  • 187
  • 2
  • 13
2

Maybe this?

public void getHealed()
{
  if (health <= 85)
  {
    health += 15;
  } else
  {
    health = 100;
  }
}
Juto
  • 1,246
  • 1
  • 13
  • 24
2

If you want to be cheeky and fit your code on one line, you could use a ternary operator:

health += (health <= 85) ? 15 : (100 - health);

Note that some people will frown upon this syntax due to (arguably) bad readability!

Oleksiy
  • 37,477
  • 22
  • 74
  • 122
  • 4
    I find `health = (health <= 85)?(health+15):100` more readable (if you really want to use a ternary operator) – Matteo Sep 06 '13 at 05:28
1

I believe this will do

if (health >= 85) health = 100;
else health += 15;

Explanation:

  • If the gap for healing is 15 or less, health will become 100.

  • Otherwise if the gap is bigger than 15, it will add 15 to the health.

So for example: if the health is 83, it will become 98 but not 100.

MarmiK
  • 5,639
  • 6
  • 40
  • 49
  • Can you expand on how this will works to resolve the requester's question? – Ro Yo Mi Sep 06 '13 at 04:51
  • If the gap for healing is 15 or less health will become 100 else if the gap is bigger than 15 it will add 15 to the health. so for Example the health is 83 will become 98 but not 100. If any specific concern please let me know, thank you for comment. – MarmiK Sep 06 '13 at 04:58
  • The `&& health < 100` condition is unnecessary. If it is 100, it will be set to 100, no change. The only reason you'd need that is if it were possible to get > 100 somehow and we don't want the healing to reduce you back down to 100. – Darrel Hoffman Sep 07 '13 at 01:54
  • @DarrelHoffman I think you are right if the game does not provide extra health 100+ then you are right, I have corrected my answer :) thank you – MarmiK Sep 08 '13 at 10:02
1

If I wanted to be thread safe I'd do it this way rather than using a synchronized block.

The atomic compareAndSet achieves the same outcome as synchronized without the overhead.

AtomicInteger health = new AtomicInteger();

public void addHealth(int value)
{
    int original = 0;
    int newValue = 0;
    do
    {
        original = health.get();
        newValue = Math.min(100, original + value);
    }
    while (!health.compareAndSet(original, newValue));
}
Robert Sutton
  • 507
  • 4
  • 7
1

Most simple way using the modulus operator.

health = (health + 50) % 100;

health will never equal or exceed 100.

bond
  • 1,054
  • 8
  • 15
0
   private int health;
    public void Heal()
    {
        if (health > 85)
            health = 100;
        else
            health += 15;
    }
    public void Damage()
    {
        if (health < 15)
            health = 0;
        else
            health -= 15;
    }
kiss my armpit
  • 3,413
  • 1
  • 27
  • 50