Making additional subclasses at any time should be as painless as possible

I'm writing a small text-based game in Java to learn the language. I'm concerned that I may be making some poor design decisions. I'll introduce 2 elements: A character and monsters. A singleton character should be able to fight a wide array of monsters.

Here is the monster class:

public abstract class Monster {

    protected static Random random = new Random();
    protected static int STATS_PER_LEVEL = 3;

    public int combatLvl;
    public int defense, strength, attack, hp;
    protected String name;

    protected Monster(int defense, int strength, int attack, int hp) {
        this.defense = defense;
        this.strength = strength;
        this.attack = attack;
        this.hp = hp;
        this.combatLvl = (int) (defense+strength+attack+Math.round((double)hp/10)-4)/STATS_PER_LEVEL;
    }

    public int attack() {
        //removed the body
    }

    public String toString(String name) {
        return this.name;
    }
}

Here's a subclass:

```
public class Bandit extends Monster {

public static double averageLevelDouble = -1;
public static String averageLevelString = "dummy";

public static int minDefense = 1;
public static int maxDefense = 1;
public static int minStrength = 1;
public static int maxStrength = 3;
public static int minAttack = 1;
public static int maxAttack = 3;
public static int maxHp = 4;

public Bandit() {
super(random.nextInt(maxDefense-minDefense+1) + minDefense,
random.nextInt(maxStrength-minStrength+1) + minStrength,
random.nextInt(maxAttack-minAttack+1) + minAttack,
random.nextInt(maxHp-minHp+1) + minHp);
double min = minDefense+minStrength+minAttack+((double)minHp/10);
double max = maxDefense+maxStrength+maxAttack+((double)maxHp/10);
double avg = ((max+min)/2)-4;
averageLevelDouble = (double)Math.round((avg/STATS_PER_LEVEL)*10)/10;
averageLevelString = (Math.round(min)-4)/STATS_PER_LEVEL

You don't need a separate class for it!

I don't see any real difference between a Bandit and a Skeleton and a regular monster. You don't add or change any methods in the subclasses, and you don't add any new variables (other than the static ones).

The most important change you can make here is to only use one class. And then you'd want different methods to instanciate that class. This can be considered factory methods

A current smell of your code is that your call to the super constructor is a very long one and it's using many variables and some calculations on them. Factory methods will deal with this.

I also think that you are overusing static variables. There are a bunch of ways to deal with this but in this example I will just put them as local variables in a method. A future extension would be to create a MonsterConfiguration class that keeps these variables as fields, then you can create several MonsterConfigurations, and perhaps read some MonsterConfigurations from XML?

In your Monster class, add String name as a parameter to the constructor and remove the abstract keyword from it.

public Monster createBandit() {
    int minDefense = 1; 
    int maxDefense = 1;
    int minStrength = 1;
    int maxStrength = 3;
    int minAttack = 1;
    int maxAttack = 3;
    int maxHp = 4;

    int defense = random.nextInt(maxDefense-minDefense+1) + minDefense;
    int strength = random.nextInt(maxStrength-minStrength+1) + minStrength;
    int attack = random.nextInt(maxAttack-minAttack+1) + minAttack;
    int hp = random.nextInt(maxHp-minHp+1) + minHp;
    return new Monster("Bandit", defense, strength, attack, hp);
}

I would suggest moving these things to fields in the class or perhaps preferably, methods of the Monster class:

double min = minDefense+minStrength+minAttack+((double)minHp/10);
    double max = maxDefense+maxStrength+maxAttack+((double)maxHp/10);
    double avg = ((max+min)/2)-4;
    averageLevelDouble = (double)Math.round((avg/STATS_PER_LEVEL)*10)/10;
    averageLevelString = (Math.round(min)-4)/STATS_PER_LEVEL + "-" + 
                 (Math.round(max)-4)/STATS_PER_LEVEL;

These methods could be called something like: getMin, getMax, getAvg, getAverageLevelDouble, getAverageLevelString.

Now if you want to have a Skeleton, just make a createSkeleton method.

StackExchange Code Review Q#44698, answer score: 12