-1

I started creating a basic roleplaying game, and now I work on the basics. I have a code duplication for creating new characters and for existed character, which is a very bad things. I'll explain my problem - At the beginning a player can choose a Character Class (like fighter) by calling using CharacterCreator. I have a Character class that describes all the information regarding the character. I also have an abstract class named CharacterClass that describes specific attributes and other stuff of character classes (like Fighter, not java class). CharacterClass has some subclasses (like Fighter, Mage etc.). The code works, but has a bad design.

How can I get rid of the code duplication of Character and CharacterClass? Should I change the design?

public class Game {
    public static void main(String[] args) {
        Character hero =  CharacterCreator.CharacterCreator();
    }
}


public class CharacterCreator {

    public static Character CharacterCreator() {
        System.out.println("Choose a character: ");
        System.out.println("1. Fighter");
        System.out.println("2. Rogue");
        System.out.println("3. Mage");
        System.out.println("4. Cleric");


        Scanner sc = new Scanner(System.in);
        int scan = sc.nextInt();
        String choice = getCharacterClass(scan);

        System.out.println("Choose Name:");
        Scanner nameIn = new Scanner(System.in);
        String name = nameIn.next();

        CharacterClass chosenClass = null;
        Character hero = null;

        switch (choice){
        case "Fighter":
            chosenClass = new Fighter();
            break;
        case "Rogue":
            chosenClass = new Rogue();
            break;
        case "Mage":
            chosenClass = new Mage();
            break;
        case "Cleric":
            chosenClass = new Cleric();
            break;
        }

        try {
            hero = new Character(name, chosenClass);
            System.out.println("A hero has been created");
            hero.displayCharacter();
        } catch (Exception e){
            System.out.println("There was a problem assigning a character class");
        }

        return hero;

    }

    public static String getCharacterClass(int scan){

        String classIn;

        switch (scan) {
        case 1:
            classIn = "Fighter";
            break;
        case 2:
            classIn = "Rogue";
            break;
        case 3:
            classIn = "Mage";
            break;
        case 4:
            classIn = "Cleric";
            break;
        default:
            System.out.println("Enter again");
            classIn = "def";
        }

        return classIn;
    }
}

public class Character {

    private String name;
    private String characterClass;
    private int level;
    private int hp;
    private int currentHp;
    private int armorClass;

    private long xp;
    /*private int BAB; /*Base attack bonus*/

    private int strength;
    private int constitution;
    private int dexterity;
    private int intelligence;
    private int wisdom;
    private int charisma;

    Character(String name, CharacterClass chosenClass){

        this.name = name;
        this.characterClass = chosenClass.getCharacterClass();
        level =  chosenClass.getLevel() ;
        hp = ( chosenClass.getHp() + getModifier( chosenClass.getConstitution() )  );
        currentHp = hp;
        setArmorClass(10 + getModifier( + chosenClass.getDexterity()));
        strength = chosenClass.getStrength();
        constitution = chosenClass.getConstitution();
        dexterity = chosenClass.getDexterity();
        intelligence = chosenClass.getIntelligence();
        wisdom = chosenClass.getWisdom();
        charisma = chosenClass.getCharisma();
        xp = 0;


    }

    void displayCharacter() throws IOException {
        System.out.print("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n");
        System.out.println("Name: " + getName());
        System.out.println("Class: " + getCharacterClass());
        System.out.println("Level: " + getLevel());
        System.out.println("HP: " + getHp());
        System.out.println("Armor Class: " + getArmorClass());

        System.out.println("***************");
        System.out.println("Attributes: ");
        System.out.println("Strength: " + getStrength());
        System.out.println("Constitution: " + getConstitution());
        System.out.println("Dexterity: " + getDexterity());
        System.out.println("Intelligence: " + getIntelligence());
        System.out.println("Wisdom: " + getWisdom());
        System.out.println("Charisma: " + getCharisma());
        System.out.println("***************");
        System.out.println("XP: " + getXp());

    }

    public int getModifier(int number){
        int mod = (int)((number -10)/2);
        return mod;
    }

    public String getName() { return name; }
    public String getCharacterClass() { return characterClass; }
    public int getLevel() { return level; }
    public int getHp() { return  hp; }
    public int getCurrentHp() { return  currentHp; }
    public int getArmorClass() { return  armorClass; }
    public int getStrength(){ return strength; }
    public int getConstitution(){ return constitution; }
    public int getDexterity(){ return dexterity; }
    public int getIntelligence(){ return intelligence; }
    public int getWisdom(){ return wisdom; }
    public int getCharisma(){ return charisma;}
    public long getXp(){ return xp;}


    protected void setLevel(int lvl){ level = lvl; }
    protected void setHp(int hitPoints){ hp = hitPoints; }
    protected void setCurrentHp(int curHp){ currentHp = curHp; }
    protected void setArmorClass(int ac){ armorClass = ac; }
    protected void setStrength(int str){ strength = str; }
    protected void setConstitution(int con){ constitution = con; }
    protected void setDexterity( int dex) { dexterity = dex; }
    protected void setIntelligence(int intel){ intelligence = intel; }
    protected void setWisdom(int wis){ wisdom = wis; }
    protected void setCharisma(int cha){charisma = cha; }



}
abstract class CharacterClass {

    private String characterClass;
    private int level;
    private int hp;

    private int strength;
    private int constitution;
    private int dexterity;
    private int intelligence;
    private int wisdom;
    private int charisma;

    protected CharacterClass(){

        setCharacterClass("Character Class");
        setLevel(1);
        setHp(10);
        setStrength(10);
        setConstitution(10);
        setDexterity(10);
        setIntelligence(10);
        setWisdom(10);
        setCharisma(10);
    }

    public String getCharacterClass() { return characterClass; }
    public int getLevel() { return level; }
    public int getHp() { return  hp; }
    public int getStrength(){ return strength; }
    public int getConstitution(){ return constitution; }
    public int getDexterity(){ return dexterity; }
    public int getIntelligence(){ return intelligence; }
    public int getWisdom(){ return wisdom; }
    public int getCharisma(){ return charisma; }

    protected void setCharacterClass(String characterClass){ this.characterClass = characterClass; }
    protected void setLevel(int lvl){ level = lvl; }
    protected void setHp(int hitPoints){ hp = hitPoints; }
    protected void setStrength(int str){ strength = str; }
    protected void setConstitution(int con){ constitution = con; }
    protected void setDexterity( int dex) { dexterity = dex; }
    protected void setIntelligence(int intel){ intelligence = intel; }
    protected void setWisdom(int wis){ wisdom = wis; }
    protected void setCharisma(int cha){charisma = cha; }

}


class Fighter extends CharacterClass {

    Fighter(){
        setCharacterClass("Fighter");
        setLevel(1);
        setHp(10);
        setStrength(14);
        setConstitution(16);
        setDexterity(14);
        setIntelligence(10);
        setWisdom(10);
        setCharisma(10);
    }
}
fabian
  • 80,457
  • 12
  • 86
  • 114
Niminim
  • 668
  • 1
  • 7
  • 16
  • As a side note, avoid using the class name `Character` (or anything else in `java.lang` or `java.util`), as it tends to cause conflicts at surprising times. – chrylis -cautiouslyoptimistic- Oct 24 '15 at 07:27
  • Try making your issue more clear - where/what specifically is the "code duplication" that you are struggling with? – VILLAIN bryan Oct 24 '15 at 08:34
  • The problem is that both Character and CharacterClass has lots of same getter/setters, because I create a subclass of CharacterClass, and then assign it to Character's construction (Plus some additions to Character's construction). – Niminim Oct 24 '15 at 08:35
  • If you have CharacterClass as your abstract class (with gets/sets for all of your character data) and Fighter, Rogue, etc. are extending that abstract class, you shouldn't need to "re-implement" your gets/sets. That's one of the benefits of having this superclass implementation (ex: public class Fighter extends CharacterClass) – VILLAIN bryan Oct 24 '15 at 08:38
  • Yes, but long code can make people focus on the code, rather than the concept. The conceptual design is probably more important than the code. – Niminim Oct 24 '15 at 08:46
  • @mcw - but I want to change Character object through Character. Plus, Character has more info and methods than CharacterClass. – Niminim Oct 24 '15 at 08:48
  • Looking very very briefly at your source, it looks like you could just rename class Character to something like CharacterData and you could just put that object in the abstract class CharacterClass .. then all of the Fighter, Rogue, etc will have a CharacterData object as part of their package. edit: looking a little more closely it's going to be a little more complicated than that since your code is (severely more complicated than it needs to be) from an OOP perspective .. Look up some basic OOP principles and how to implement them in Java – VILLAIN bryan Oct 24 '15 at 08:52
  • @mcw That's my first Java project, that's why I wanted to consult with others regarding the design. Creating Character as an abstract class and making Fighter, Mage etc. extend Character can make it easier I guess (without CharacterClass at all), it also can solve the code duplication that I have with Character and CharacterClass, though it can put some limitation in the future. The problem in this solution that it's also a bad design, because a character has a character class, rather than a character is a character class, so aggregation is better than inheritance. – Niminim Oct 24 '15 at 09:15

2 Answers2

2

suggestions:

  1. CharacterCreator.CharacterCreator(). Method should be verb and describe action, i.e createCharacter
  2. look ad design pattern Factory. Your Creator is 'Factory'. The method 'createCharacter' should take parameter characterType. That means, that getting info from System.in should be done in class who invoke that method.
  3. Add enum for characterClass with mapping to numbers (look to inner map in enum).
Natalia
  • 4,362
  • 24
  • 25
  • 3. Some people here suggested to use classes rather than enums (I strated with enums. Plus, I'm not sure that I'll have only numbers). 4. That's what I thought at the beginning, but some people here pointed out that it's not a good design, because a character has a character class rather than a character is a character class. Plus, it will be pain to add more than one character class for a character in the future, if I choose to do so. – Niminim Oct 24 '15 at 07:07
  • as I understand, characterClass is just the predefined configuration for character. You copy all values from characterClass to class. So, it looks just like config. – Natalia Oct 24 '15 at 07:11
  • It's a partial initial configuration (most info in Character comes from CharacterClass, but not all of it). Plus, not necessarily that one character class defines a character, it can change in the future. – Niminim Oct 24 '15 at 07:14
0

Whilst this is not a direct solution to your problem, this should help you in the long run. When it comes to games, especially RPG genre, where you have lots of objects which seem to be similar yet different, inheritance isn't best foundation for design. On top of the example you have, one will also have problems when designing consumable items / weapons / gear / NPC, etc. This means you end up with having duplicate code in many classes simply because using abstract class would mean all subclasses have same behavior, but this is not true.

A better approach in this case is to avoid inheritance and use ECS. This means everything is a type of Entity. In order to add some "functionality" to an entity you would use Component types. For example, items don't have HP property, but say when dropped on the ground, they can be attacked and destroyed. Meaning we need to add a dynamic property to it. We can do that as follows:

entityItem.addComponent(new HPComponent(50));

This will allow other systems like Attack/Damage to "see" that the entity has HP component and can be attacked.

This is just a tiny example of ECS and there's lots more to it. I'd suggest reading more about it, as this will make game development design (for most games) significantly smoother.

AlmasB
  • 3,377
  • 2
  • 15
  • 17
  • It's a very interesting approach. It looks like it can be very useful when things go much more complex. In my case I want to keep it more simple.Assuming that I'm doing it for the sake of the exercise and fun and not going to dive so deep in to it, then how can I design it in a better way? – Niminim Oct 24 '15 at 09:00
  • Creating Character as an abstract class and making Fighter, Mage etc. extend Character can make it easier I guess (without CharacterClass at all), it also can solve the code duplication that I have with Character and CharacterClass, though it can put some limitation in the future. The problem in this solution that it's also a bad design, because a character has a character class, rather than a character is a character class. – Niminim Oct 24 '15 at 09:08