HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Printing RPG game character properties

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
propertiescharactergameprintingrpg

Problem

I have written this code representing a class hierarchy of character classes, and a world class to print their properties.

Is anything unclear about this code? Am I utilizing good practices?

class Human{    
    protected String name;
    protected int health;
    protected int armorLevel;
    protected int magicLevel;
    protected int experience;
    protected int level;
        Human(){
        name = "Andrew The Magic";
        health = 100;
        armorLevel = 1;
        magicLevel = 1;
        experience=0;
        level =1;
        }

    Human(String name){
        this(); //calling main constructor
        this.name = name;
    }

void fight() {}
void levelUp() {}
void showAbility() { System.out.println("Name: "+name+" HP: "+health+" ARMOR: "+armorLevel+" MAGIC: "+magicLevel+" EXP: "+experience+" LVL: "+level); }
void yell(){    System.out.println("IM A CHARACTER ;-(");   }
}

//--------------------------------------------------------------        
class Warrior extends Human{

    Warrior(){ 
        super();    
    }

    Warrior(String name){
        super(name);
    }

    void fight(){
        System.out.println("I attack with sword");
        experience +=10;
        if (experience >= 20 ){ levelUp();  }
        }

    void levelUp(){
        System.out.println("LEVEL UP!");
        health +=100;
        armorLevel +=25;
        level++;
    }

    void yell(){
        System.out.println("IM A GLORIOUS WARRIOR "+name);
    }
}    
//--------------------------------------------------------------

public class World{

    static void greeting(Human p){
        p.yell();
    }

        public static void main(String[] args){
          Warrior w = new Warrior("Noname the Warrior");
            w.showAbility();
            w.fight();
            w.showAbility();
            w.fight();
            w.showAbility();

            greeting(w);
   }
}

Solution

Is anything unclear about this code?

The purpose of this code is unclear.


Do I break any good-programming rules?

Some bad practices that immediately jump into the eye:

  • Formatting is horrible



  • The parameterless constructor of Warrior is pointless and can be safely omitted



  • Methods with empty bodies, such as fight and levelUp in Human should at least have a comment explaining why the implementation is left empty



-
showAbility is poorly named:

  • None of the attributes are abilities: name is certainly not an ability, and the other fields are questionable too



  • "show" is a bit ambiguous. Show where? In a pop-up, maybe? I'd use the word "print" instead.



Maybe printStats would be better.

-
It's probably not a big concern for your programming exercise, but for the record, print statements inside a program are generally frowned upon. The preferred way is to use a logging framework instead. (Again, this is just for the record, since you asked about bad practices.)

Instead of Warrior inheriting from Human,
consider using interfaces.
For example you could have Warrior and Mage interfaces,
so that you can have mighty mage-warriors.
With inheritance this would be impossible.
Using interfaces will lead to a more extensible design.

Context

StackExchange Code Review Q#74240, answer score: 14

Revisions (0)

No revisions yet.