Fixing methods in Java OOP

Question

I am having trouble finding the issue with my problem. Everything checks out right when I test my program except for one thing. In my print statements while I execute the program, they do not update the health correctly like they should.

import java.util.Random;
import java.util.Scanner;

public class Dragon {

    private static int health;
    private static int attack;
    private static boolean isAlive = true;

    private static int dragonHealth;
    private static int dragonAttack;
    private static boolean isDragonAlive = true;

    public static int getHealth() {
        if(health <= 0)
        {
            health = 0;
        }
        return health;
    }

    public void setHealth(int health) {
        this.health = health;
    }

    public int getAttack() {
        return attack;
    }

    public void setAttack(int attack) {
        this.attack = attack;
    }

    public static int getDamage()
    {
        Random generator = new Random();
        int attack = generator.nextInt(10) + 1;
        health = health - dragonAttack;
        return attack;
    }

    public static boolean getAlive()
    {
        if(getHealth() <= 0)
        {
            isAlive = false;
        }
        return isAlive;
    }    

    ///////////////////////////////////////////////////////

    public static int getDragonHealth()
    {
        if(dragonHealth <= 0)
        {
            dragonHealth = 0;
        }
        return dragonHealth;
    }

    public void setDragonHealth(int dragonHealth)
    {
        this.dragonHealth = dragonHealth;
    }

    public int getDragonAttack() {
        return dragonAttack;
    }

    public void setDragonAttack(int dragonAttack) {
        this.dragonAttack = dragonAttack;
    }

    public static int getDragonDamage()
    {
        Random generator = new Random();
        int dragonAttack = generator.nextInt(10) + 1;
        dragonHealth = dragonHealth - attack;
        return dragonAttack;
    } 

    public static boolean getDragonAlive()
    {
        if(getDragonHealth() <= 0)
        {
            isDragonAlive = false;
        }
        return isDragonAlive;
    }    

    /////////////////////////////

    public String getWelcome()
    {
        String welcome = "Hello and welcome to Dragonslayer!";
        return welcome;
    }

    public static String getStab()
    {
        String stab = "You choose to stab the dragon and dealt " + getDamage() + " damage. The dragon now has " + getDragonHealth() + " health remaining.";
        return stab;
    }

    public static String getSlash()
    {
        String slash = "You choose to slash the dragon and dealt " + getDamage() + " damage. The dragon now has " + getDragonHealth() + " health remaining.";
        return slash;
    }

    public static String getCut()
    {
        String cut = "You choose to cut the dragon and dealt " + getDamage() + " damage. The dragon now has " + getDragonHealth() + " health remaining.";
        return cut;
    }

    public static String dragonAttack()
    {
        String dragonsAttack = "The dragon has done " + getDragonDamage() + " to you. You now have " + getHealth() + " health remaining.";
        return dragonsAttack;
    }

    public static String getWinner()
    {
        String result = "";
        if(getAlive() == false && getDragonAlive() == false)
        {
            result = "It is a horrid day today, as both you and the dragon have fallen.";
        }
        else if(getAlive() == true && getDragonAlive() == false)
        {
            result = "Congratulations you have killed the dragon, and survived!";
        }
        else if(getAlive() == false && getDragonAlive() == true)
        {
            result = "You have sadly fallen to the dragon, better luck next time.";
        }
        else
        {
            result = "SOMETHING WENT WRONG!!!";
        }
        return result;
    }

    public static void main(String[] args)
    {
        Scanner in = new Scanner(System.in);
        System.out.print("You come across the dragon and you have two options.  Do you run or fight? ");


        Dragon dragon1 = new Dragon();
        dragon1.setHealth(50);

        Dragon dragon2 = new Dragon();
        dragon2.setDragonHealth(50);

        while(in.hasNextLine())
        {
            switch(in.nextLine())
            {
                case "run":
                    System.out.println("I am so sorry, you could not outrun the dragon, you have been killed!");
                    break;
                case "fight":
                    while(getAlive() && getDragonAlive())
                    {
                        System.out.println("Do you want to stab, slash, or cut the dragon? ");
                        switch(in.nextLine())
                        {
                            case "stab":
                                System.out.println(getStab() + "\n" + dragonAttack());
                                break;
                            case "slash":
                                System.out.println(getSlash() + "\n" + dragonAttack());
                                break;
                            case "cut":
                                System.out.println(getCut() + "\n" + dragonAttack());
                                break;
                            default:
                                System.out.println("I am sorry that is not valid, try again. ");
                        }
                    }
                    break;
                default:
                    System.out.println(getWinner());
                    break;
            }

            System.out.println("Congratulations, you have slayed the dragon!");
            break;
        }//end of while loop in.hasNextLine().
    }//end of main
}//end of class

You've got gross static over-use and it's killing your code, making fields class fields and not instance fields. This will prevent your code from behaving in a decent OOP way since your Dragon instances will not have their own fields. Get rid of everything static, **EVERYTHING**, except for the main method, and that's it. Please read: [Why are static variables considered evil?](http://stackoverflow.com/questions/7026507/why-are-static-variables-considered-evil). — Hovercraft Full Of Eels, Sep 01 '15 at 23:52
Also a nitpick, but never use `== true` or `== false`. Instead of `if (foo == true)` and `if (bar == false)` do `if (foo)` and `if (!bar)`. — Hovercraft Full Of Eels, Sep 01 '15 at 23:54

score 4 · Answer 1 · edited May 23 '17 at 12:29

In my print statements while I execute the program, they do not update the health correctly like they should.

Your health field is a static field and thus a field of the class not of the instance. Every Dragon will share the exact same health value, which is not what you want. I suggest:

First make more classes as you shouldn't have all this code within Dragon. You should create a Warrior class as well, one that represents the person fighting the Dragon, one that should have his own health and similar fields.
Most all the fields and methods here should be non-static. The only exception that I can see should be the main method, and that's it. Please read Why are static variables considered evil?
I would get all user interface code out of my Warrior and Dragon classes, and instead have them focus only on maintaining their own state (field values) and behaviors (methods).
Use another class for user interaction. If simple, this could all be in the main method, but this type of code does not "scale" well.
Also a nitpick, but never use == true or == false, but instead use more succinct and easily readable code that just tests the boolean variable or expression itself. So instead of if (foo == true) and if (bar == false) do if (foo) and if (!bar).

For example:

class Creature {
    private int health;
    private int attack;
    private String name;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getHealth() {
        return health;
    }

    public void setHealth(int health) {
        this.health = health < 0 ? 0 : health;
    }

    public int getAttack() {
        return attack;
    }

    public void setAttack(int attack) {
        this.attack = attack;
    }

    public boolean isAlive() {
        return health > 0;
    }

}

class Dragon extends Creature {
    // add Dragon specific methods
}

class Warrior extends Creature {
    // add Warrier specific methods
}

score 1 · Answer 2 · answered Sep 02 '15 at 00:14

I think you want to change

public static int getDamage()
{
    Random generator = new Random();
    int attack = generator.nextInt(10) + 1;
    health = health - dragonAttack;
    return attack;
}

To:

 public static int getDamage()
{
    Random generator = new Random();
    setAttack(generator.nextInt(10) + 1);
    health = health - dragonAttack;
    return attack;
}

And similarly for getDragonDamage... You are setting a local variable (int this case attack) and expecting it to be updated in getDragonDamage

Fixing methods in Java OOP

2 Answers2

Linked