Generating RPG Characters (Objects)

Question

I am a beginner Java programmer. I have just finished up an assignment and would appreciate some advice and/or constructive criticism on my program. I am trying to ensure I do not advance my knowledge using bad practices. For context, I have the assignment details, followed by the full code below:

Part 1: Write an abstract Human class that implements the Human interface above. You’ll need to include the interface in your project.

Part 2: Implement the Knight and Wizard classes.

Part 3: Write a test program called LastNameWorld, which:

1. Declares 4 Humans
2. Instantiates a Knight
3. Instantiates a Wizard and moves it
4. Clones the Knight and changes its name
5. Checks if the Knight clone is not equal to the original Knight, and if so: moves the clone
6. Clones the Wizard
7. Checks if the Wizard clone is equal to the original Wizard, and if so: moves the clone
8. Prints the total number of Humans living in this World
9. Prints a description (toString) of all Humans to the console

Part 4: Add a third Human (e.g. Archer, King, Queen, Paladin, etc) type class to your simulation. This class must contain an equals and a clone method. Add code to your test harness to fully test your new class.

Main Class

public class BakosWorld {

    public static void main(String[] args) {

        // Declare 4 humans
        Human[] humans = new Human[4];

        // Create original knight and wizard, then move wizard
        humans[0] = new Knight("Rob", 36, 100, 6.1, 50, 50, "Grey Wind");

        humans[1] = new Wizard("Gandalf", 32, 80, 5.11, 25, 25, 50);
        humans[1].move(45, 45);

        // Creating a new knight, that is a clone of the first knight
        humans[2] = ((Knight) humans[0]).clone();
        // Checks to see if the two knights are NOT equal
        if (((Knight) humans[0]).equals(((Knight) humans[2])) == false) {
            humans[2].move(85, 85);
        }

        // Creating a new wizard, that is a clone of the first wizard
        humans[3] = ((Wizard) humans[1]).clone();
        // Checks to see if the two knights ARE equal
        if (((Wizard) humans[1]).equals(((Wizard) humans[3]))) {
            humans[3].move(60, 60);
        }

        // Print information
        System.out.println("Number of humans: " + Human.getNumHumans());
        for (int i = 0; i < humans.length; i++) {
            System.out.println(humans[i].toString());
        }

        // Creating new human, assassin, the clone, and moving the clone
        Human arya = new Assassin("Arya", 16, 70, 5.9, 20, 20, 80);
        Human aryaClone = ((Assassin) arya).clone();

        if (((Assassin) arya).equals((Assassin) aryaClone)) {
            aryaClone.move(120, 120);
        }

        // Gandalf killed (original) Arya's father, so it's time for her to get her revenge
        ((Assassin) arya).inflictPoison(humans[1]);

        System.out.println("\n\n-UPDATED-\n\n" + arya.toString() + "\n"
                + aryaClone.toString() + "\n" + humans[1].toString());
    }
}

Interface

public interface HumanInterface {

    //return the name of the human
    public String getName();

    //change the name of the human
    public void setName(String name);

    //get the age of the human
    public int getAge();

    //change the age of a human
    public void setAge(int a);

    //return the height of the human
    public double getHeight();

    //change the height of a human
    public void setHeight(double h);

    //return the health of a human
    public int getHealth();

    //change the health of a human
    public void setHealth(int h);

    //move a human
    public void move(int x, int y);

    //get Y position of a human
    public int getYPos();

    //get X position of a human
    public int getXPos();

    //return a string representation of a human object
    public String toString();
}

Abstract Class

abstract public class Human implements HumanInterface {

    protected String name;
    protected int age;
    protected double height;
    protected int health;
    protected int xPos;
    protected int yPos;

    private static int numHumans;

    public Human(String n, int a, int hea, double hei, int x, int y) {
        name = n;
        age = a;
        health = hea;
        height = hei;
        xPos = x;
        yPos = y;
        numHumans++;
    }

    public void move(int x, int y) {
        xPos = x;
        yPos = y;
    }

    public int getXPos() {
        return xPos;
    }

    public int getYPos() {
        return yPos;
    }

    public void setHealth(int h) {
        health = h;
    }

    public int getHealth() {
        return health;
    }

    public void setName(String n) {
        name = n;
    }

    public String getName() {
        return name;
    }

    public void setAge(int a) {
        age = a;
    }

    public int getAge() {
        return age;
    }

    public void setHeight(double h) {
        height = h;
    }

    public double getHeight() {
        return height;
    }

    public void setNumHumans(int n) {
        numHumans = n;
    }

    public static int getNumHumans() {
        return numHumans;
    }

    // General toString method
    public String toString() {
        String message;
        if (health <= 0) {
            message = "\n[HUMAN]"
                    + "\n" + name + " is dead.";
        } else {
            message = "xPos: " + xPos
                    + "\nyPos: " + yPos
                    + "\nName: " + name
                    + "\nAge: " + age
                    + "\nHealth: " + health
                    + "\nHeight: " + height;
        }
        return message;
    }
}

Sub-Class: Knight

public class Knight extends Human {

    private String horseName;

    public Knight(String n, int a, int hea, double hei, int xPos, int yPos, String hN) {
        super(n, a, hea, hei, xPos, yPos);
        horseName = hN;
    }

    public void setHorseName(String hN) {
        hN = horseName;
    }

    public String getHorseName() {
        return horseName;
    }

    // Clone the object
    public Knight clone() {
        Knight theClone;
        theClone = new Knight(this.name, this.age, this.health, this.height,
                this.xPos, this.yPos, horseName);
        return theClone;
    }

    // Check to see if 2 objects are the same
    public boolean equals(Knight k) {
        return k.age == this.age
                && k.name.equals(this.name)
                && k.health == this.health
                && k.height == this.height
                && k.xPos == this.xPos
                && k.yPos == this.yPos
                && k.horseName.equals(this.horseName);
    }

    // Knight toString method
    public String toString() {
        String message;
        if (health <= 0) {
            message = "\n[KNIGHT]"
                    + "\n" + name + " is dead.";
        } else {
            message = "\n[KNIGHT]"
                    + "\nxPos: " + xPos
                    + "\nyPos: " + yPos
                    + "\nName: " + name
                    + "\nAge: " + age
                    + "\nHealth: " + health
                    + "\nHorse Name: " + horseName
                    + "\nHeight: " + height;
        }
        return message;
    }
}

Sub-Class: Wizard

public class Wizard extends Human {

    private int magicka;

    public Wizard(String n, int a, int hea, double hei, int x, int y, int m) {
        super(n, a, hea, hei, x, y);
        magicka = m;
    }

    public void castSpell() {

    }

    public void setMagicka(int m) {
        magicka = m;
    }

    public int getMagicka() {
        return magicka;
    }

    // Clone the object
    public Wizard clone() {
        Wizard theClone;
        theClone = new Wizard(this.name, this.age, this.health, this.height,
                this.xPos, this.yPos, magicka);
        return theClone;
    }

    // Check to see if 2 objects are the same
    public boolean equals(Wizard w) {
        return w.age == this.age
                && w.name.equals(this.name)
                && w.health == this.health
                && w.height == this.height
                && w.xPos == this.xPos
                && w.yPos == this.yPos
                && w.magicka == this.magicka;
    }

    // Wizard toString method
    public String toString() {
        String message;
        if (health <= 0) {
            message = "\n[WIZARD]"
                    + "\n" + name + " is dead.";
        } else {
            message = "\n[WIZARD]"
                    + "\nxPos: " + xPos
                    + "\nyPos: " + yPos
                    + "\nName: " + name
                    + "\nAge: " + age
                    + "\nHealth: " + health
                    + "\nMagicka: " + magicka
                    + "\nHeight: " + height;
        }
        return message;
    }
}

Sub-Class: Assassin

public class Assassin extends Human {

    int poison;

    public Assassin(String n, int a, int hea, double hei, int x, int y, int p) {
        super(n, a, hea, hei, x, y);
        poison = p;
    }

    public void inflictPoison(Human h) {
        h.setHealth(0);
        poison = this.poison - 20;
    }

    public void setPoison(int p) {
        poison = p;
    }

    public int getPoison() {
        return poison;
    }

    // Clone the object
    public Assassin clone() {
        Assassin theClone;
        theClone = new Assassin(this.name, this.age, this.health, this.height,
                this.xPos, this.yPos, poison);
        return theClone;
    }

    // Check to see if 2 objects are the same
    public boolean equals(Assassin a) {
        return a.age == this.age
                && a.name.equals(this.name)
                && a.health == this.health
                && a.height == this.height
                && a.xPos == this.xPos
                && a.yPos == this.yPos
                && a.poison == this.poison;
    }

    // Assassin toString method
    public String toString() {
        String message;
        if (health <= 0) {
            message = "\n[ASSASSIN]"
                    + "\n" + name + " is dead.";
        } else {
            message = "\n[ASSASSIN]"
                    + "\nxPos: " + xPos
                    + "\nyPos: " + yPos
                    + "\nName: " + name
                    + "\nAge: " + age
                    + "\nHealth: " + health
                    + "\nPoison: " + poison
                    + "\nHeight: " + height;
        }
        return message;
    }
}

Answer 1

Your code is quite well structured. A few points:

Useless cast before calling a method

These cast bother me:

// Creating a new knight, that is a clone of the first knight
humans[2] = ((Knight) humans[0]).clone();
// Checks to see if the two knights ARE equal
if (((Wizard) humans[1]).equals(((Wizard) humans[3]))) { //...

The clone method will clone the object. Whatever their type, they'll be cloned to the same class, be it Wizard, Assassin, or whatever. But you don't need to know their exact type to compare them using equals, because that method conveniently only requires the compared object to be Object.

So this does the same thing:

 // Creating a new knight, that is a clone of the first knight
humans[2] = humans[0].clone();
// Checks to see if the two knights ARE equal
if (humans[1]).equals(humans[3])) { //...

But for that to work, you'd need to have a proper equals and a proper clone methods...

Clonable interface

From the Java API documentation on clone():

[...] if the class of this object does not implement the interface Cloneable, then a CloneNotSupportedException is thrown.

So you need to decide who should carry out this function (the Human interface, the Abstract class or whatnot) and make it extend/implement cloneable.

Implement equals properly

Your declaration of equals is unorthodox, and probably does not do what you're thinking You are probably expected to Override the Object.equals(Object obj) method. Right now, your Human objects cannot compare theselves. Only Assassin with Assassin, Wizard with Wizard, etc. but not Assassin versus Wizard.

So when you call assassin.equals(otherAssassin), you're calling your own function Assassin.equals. But when calling human.equals(otherHuman) you're calling the Object.equals(Object), and it might not suit you perfectly because your did not override it. This is the reason you needed to cast earlier... now you won't!

I propose that you shed a lot of boilerplate by centralizing the tedious parts of the usual equals implementation (type checking, null checking etc.), by using double dispatch :

public abstract class Human implements HumanInterface {
    [...blah blah blah]
    public boolean equals(Object 
        if (other != null && other instanceof Human) {
            Human otherHuman = (Point) obj;
            return otherHuman.equals(this); // Order is important!
        }else{
            return false;
        }
    }

    // I like to provide default implementation, but at least one MUST be overriden
    public boolean equals(Knight knight){ return false; }
    public boolean equals(Wizard wizard){ return false; }
    public boolean equals(Assassin assassin){ return false; }

}

And then each concrete class only has to override their corresponding method:

public class Knight extends Human {
    [...blah blah blah...]
    @Override
    public boolean equals(Knight knight){
         [...your own implmentation here...]
    }
}

---

You see, toString, clone, equals and hashCode are methods of Object, so absolutely ALL classes must abide by the contract defined by the Object class. And you still have that last one to code...

This also means the public String toString(); declaration in HumanInterface is useless.

String handling

message = "\n[ASSASSIN]"
                + "\nxPos: " + xPos
                + "\nyPos: " + yPos
                + "\nName: " + name
                + "\nAge: " + age
                + "\nHealth: " + health
                + "\nPoison: " + poison
                + "\nHeight: " + height;

Please use a StringBuilder when concatenating many Strings, or (better here) a String.format to make this safe and more efficient.

Comment clutter

// Assassin toString method
public String toString() {

And

// Check to see if 2 objects are the same
public boolean equals(Assassin a) {

These comment brings nothing, remove it, or turn it into a proper Javadoc. For toString, its probably useless.

But inflictPoison(Human h) would need some. Especially since it can make poison go negative... and still work.

Storing the humans

Why use an Array, and not a List? you create and kill humans, it would make sense. You would then also be able to remove() them when dead.

Then counting them would be as simple as list.size();

Moreover, I would rather keep an individual (correctly typed) reference to each of them, s that their special skills are available without casting!

This would read way better:

 // Create original knight and wizard, then move wizard
 Knight rob = new Knight("Rob", 36, 100, 6.1, 50, 50, "Grey Wind");
 Wizard gandalf = new Wizard("Gandalf", 32, 80, 5.11, 25, 25, 50);
 rob.move(45, 45);

 // Creating a new knight, that is a clone of the first knight
 Knight robClone = rob.clone();
 // Checks to see if the two knights are NOT equal
 if (!rob.equals(robclone)) {
     robclone.move(85, 85);
 }

 // Creating a new wizard, that is a clone of the first wizard
 Wizard gandalfClone = gandalf.clone();
 // Checks to see if the two wizards ARE equal
 if (gandalf.equals(gandalfClone)) {
     gandalfClone.move(60, 60);
 }

 // [...]

 // Creating new human, assassin, the clone, and moving the clone
 Assassin arya = new Assassin("Arya", 16, 70, 5.9, 20, 20, 80);
 Human aryaClone = arya.clone();

 if (arya.equals(aryaClone)) {
     aryaClone.move(120, 120);
 }

 // Gandalf killed (original) Arya's father, so it's time for her to get her revenge
 arya.inflictPoison(gandalf);

 System.out.println("\n\n-UPDATED-\n\n" + arya.toString() + "\n"
            + aryaClone.toString() + "\n" + gandalf.toString());

Take advantage of your BakosWorld class

Make it a useful object, let it hold the list! Then instantiate a World in your main, and start world.killEachOther()!

In general, a main method should only be an entry point, so should look like

public static void main(String[] args) {
    SomeObject business = new SomeObject(args); // instantiate somehow
    business.start(); // Everything happens here, so it is reusable!
}

Naming

CamelCase and such are good.

However some names must be made more explicit. I can't understand:

    public Knight(String n, int a, int hea, double hei, int xPos, int yPos, String hN) {

Especially... without comments/Javadoc! This was the one place some would have been useful.

---

That's it for now! Happy coding