1
\$\begingroup\$

Thanks for the enormous response on my last question! I've returned with another question, hoping for more insight.

I'm developing an Item class and system, and I know it can be improved. I have a main Item class, with four subclasses (Weapon, Armor, Medical, Misc) .

  1. Code Redundancy

    The subclasses all have near identical class construction. The only difference between the Weapon and Armor class is the name of the classes. I would like to maybe have one class that manages all the different type of items, and can identify the differences between them so they can be used accordingly in my program.

  2. Code Efficiency

    Determining what items are determined are based on blocks of if statements. I would like to cut those down, so the code is more user friendly and more compact, and obviously more efficient.

  3. Item Determination

    Right now, when a new Item object is created, the item is randomly chosen WITHIN the Item class. I would hope for this to be managed by an outside class, which can reduce the complexity of the class.

Item.java

import java.util.Random;

/*
* Item Class
*/
public class Item {

    /*
    * Private Instance Variables
    */
    private Random r;
    private final String name;
    protected int value; /* allows subclass to use */
    private int level; /* Used to determine what level of item to set */

    /*
    * Arrays used for item level determination (ranges, not each specific level)
    */
    Object[] normal_drops = {new Medical("Medicinal Herb", this.level), new Misc("Antidontial Herb"), new Misc("Moonwort Bulb"), new Misc("Holy Water")};
    int[] normal_drop_values = {3, 2, 5, 4};
    Object[] rare_drops = {new Misc("Slime Drop"), new Armor("Boxer Shorts", this.level), new Armor("Leather Hat", this.level), new Weapon("Oak Staff", this.level)};
    int[] rare_drop_values = {10, 15, 12, 8};

    /*
    * Item Constructor (no defualt)
    */
    public Item(int level) {
        this.level = level;
        this.r = new Random();
        this.determineItem();
    }

    /*
    * determineItem method for deciding if it will drop an item at all, and
    * if so, decides if it's rare or normal
    */
    private void determineItem() {
        int x = this.r.nextInt(100 - 1) + 1;
        if(x > 95) { /* Rare Drop Chance (5%) */
            this.rareDrop();
        } else if(x > 90) { /* Normal Drop Chance (10%) */
            this.normalDrop();
        }
    }

    /*
    * normalDrop method for normal drop items
    */
    private void normalDrop() {
        int index = this.r.nextInt(4 - 0) + 0; /* Isn't inclusive, so 4 is needed */
        if(this.level >= 1 && this.level <= 10) { /* Level Range 1-10 */
            this.name = normal_drops[index];
            this.value = normal_drop_values[index];
        }
        /*
        * Now, reset the array each time for new items
        */
        if(this.level >= 11 && this.level <= 20) { /* Level Range 11-20 */
            normal_drops = {new Misc("Cougulant"), new Armor("Pot Lid", this.level), new Misc("Evencloth"), new Armor("Leather Kilt", this.level)};
            updateValues(false, 2);
            this.name = normal_drops[index];
            this.value = normal_drop_values[index];
        }
        if(this.level >= 21 && this.level <= 30) { /* Level Range 21-30 */
            normal_drops = {new Armor("Scale Shield", this.level), new Medical("Strong Medicine", this.level), new Misc("Wing of Bat"), new Misc("Cowpat")};
            updateValues(false, 3);
            this.name = normal_drops[index];
            this.value = normal_drop_values[index];
        }
        if(this.level >= 31 && this.level <= 40) { /* Level Range 31-40 */
            normal_drops = {new Medical("Superior Medicine"), new Misc("Seashell"), new Misc("Lambswool"), new Misc("Kitty Litter")};
            updateValues(false, 4);
            this.name = normal_drops[index];
            this.value = normal_drop_values[index];
        }
        if(this.level >= 41 && this.level <= 50) { /* Level Range 41-50 */
            normal_drops = {new Misc("Magic Beast Horn"), new Misc("Rockbomb Shell"), new Misc("Lambswool"), new Misc("Manky Mud")};
            updateValues(false, 5);
            this.name = normal_drops[index];
            this.value = normal_drop_values[index];
        }

    }

    /*
    * rareDrop method for rare drop items
    */
    private void rareDrop() {
        int index = this.r.nextInt(4 - 0) + 0; /* Not inclusive, so 4 is needed */
        if(this.level >= 1 && this.level <= 10) { /* Level Range 1-10 */
            this.name = rare_drops[index];
            this.value = rare_drop_values[index];
        }
        /*
        * Reset array each time for new items
        */
        if(this.level >= 11 && this.level <= 20) { /* Level Range 11-20 */
            rare_drops = {new Misc("Iron Nails"), new Armor("Garish Garb", this.level), new Misc("Angel Bell"), new Misc("Fisticup")};
            updateValues(true, 2);
            this.name = rare_drops[index];
            this.value = rare_drop_values[index];
        }
        if(this.level >= 21 && this.level <= 30) { /* Level Range 21-30 */
            rare_drops = {new Armor("Gold Ring", this.level), new Armor("Agility Ring", this.level), new Armor("Strength Ring", this.level), new Armor("Leather Cape", this.level)};
            updateValues(true, 3);
            this.name = rare_drops[index];
            this.value = rare_drop_values[index];
        }
        if(this.level >= 31 && this.level <= 40) { /* Level Range 31-40 */
            rare_drops = {new Misc("Flintstone"), new Weapon("Iron Claws", this.level), new Misc("Softwort"), new Weapon("Long Spear", this.level)};
            updateValues(true, 4);
            this.name = rare_drops[index];
            this.value = rare_drop_values[index];
        }
        if(this.level >= 41 && this.level <= 50) { /* Level Range 41-50 */
            rare_drops = {new Armor("Fur Poncho", this.level), new Armor("Ice Shield", this.level), new Weapon("Assassins Dagger", this.level), new Weapon("Crow's Claws", this.level)};
            updateValues(true, 5);
            this.name = rare_drops[index];
            this.value = rare_drop_values[index];
        }
    }

    /*
    * Method updates the value of each item in the array
    */
    private void updateValues(boolean isRare, int amount) {
        /*
        * Resets values so it doesn't keep stacking with each level
        */
        normal_drop_values = {3, 2, 5, 4};
        rare_drop_values = {10, 15, 12, 8};

        if(!isRare) {
            for(int i = 0; i <= this.normal_drop_values.length - 1; i++) {
                this.normal_drop_values[i] *= amount;
            }
        } else {
            for(int i = 0; i <= this.rare_drop_values.length - 1; i++) {
                this.rare_drop_values[i] *= amount;
            }
        }
    }

    /*
    * Getters for item value and name
    */
    public int getValue() {
        return this.value;
    }

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

}

Weapon.java

public class Weapon extends Item {

    /*
    * Private Instance Variables
    */
    private final String name;
    private int level;
    private final int damage;

    /*
    * Weapon constructor (no default)
    */
    public Weapon(String name, int level) {
        this.name = name;
        this.level = level;
        this.setAttributes();
    }

    /*
    * setAttributes, determines weapon damage
    */
    private void setAttributes() {
        this.damage = this.level + 2;
    }

}

Armor.java

public class Armor extends Item {

    /*
    * Private Instance Variables
    */
    private final String name;
    private int level;
    private final int defence;

    /*
    * Armor constructor (no default)
    */
    public Armor(String name, int level) {
        this.name = name;
        this.level = level;
        this.setAttributes();
    }

    /*
    * setAttributes, determines defence level
    */
    private void setAttributes() {
        this.defence = this.level + 2;
    }

}

Medical.java

public class Medical extends Item {

    /*
    * Private Instance Variables
    */
    private final String name;
    private int level;
    private final int heal_value;

    /*
    * Medical constructor (no default)
    */
    public Medical(String name, int level) {
        this.name = name;
        this.level = level;
        this.setAttributes();
    }

    /*
    * setAttributes, determines heal value
    */
    private void setAttributes() {
        this.heal_value = this.level * 2;
    }

    /*
    * Getters for heal value
    */
    public int getHealValue() {
        return this.heal_value;
    }

}

Misc.java

public class Misc extends Item {

    /*
    * Private Instance Variables
    */
    private final String name;

    /*
    * Misc constructor (no default)
    */
    public Misc(String name) {
        this.name = name;
    }

}
\$\endgroup\$
2
\$\begingroup\$

Well, you can implement a game item in a hundred ways depending on your use-case.

About your questions: Avoid needles inheritance. Actually never implement inheritance when you just want to share some code. Purpose of Inheritance is not a code reuse. Inheritance is meant to model Is-A relationship. And you don't need to model Is-A relationship very often. Code sharing is a side effect of this. Composition is a way better technique of code reuse. Take a look at this quick draft of what Composition based Item might look like.

public class Item {

    enum Type {
        Armor,
        Consumable,
        Weapon,
        Misc
    }

    private final String name;
    private final Type type;
    private final Money value;
    private final Attributes attributes;

    public Item(String name, Type type, Money value) {
        this.name = name;
        this.type = type;
        this.value = value;
        this.attributes = Attributes.None;
    }

    public Item(String name, Type type, Money value, Attributes attributes) {
        this.name = name;
        this.type = type;
        this.value = value;
        this.attributes = attributes;
    }

    // ...

    public static void main(String[] args) {
        new Item("Iron Nails", Item.Type.Misc, new CopperCoins(5));
        new Item("Fur Poncho", Item.Type.Armor, new SilverCoins(5));
        new Item("Health Potion", Item.Type.Consumable, new SilverCoins(1),
            new Attributes(
                new Attribute(Attribute.Type.Health, new Stat(Stat.Type.Plus, 150)),
                new Attribute(Attribute.Type.Hunger, new Stat(Stat.Type.Minus, 2))
            )
        );
        new Item("Assassins Dagger", Item.Type.Weapon, new GoldCoins(5),
            new Attributes(
                new Attribute(Attribute.Type.Damage, new Stat(Stat.Type.Plus, 5)),
                new Attribute(Attribute.Type.BleedingDamage, new Stat(Stat.Type.Plus, 10)),
                new Attribute(Attribute.Type.Agility, new Stat(Stat.Type.Plus, 2)),
                new Attribute(Attribute.Type.Weight, new Kilograms(1))
            )
        );
    }
}

You are right about the second point. Item should not be responsible for its construction nor its drop-rate. Drop-rate can be easily added to enemies. Such as draft below. Actual building of items is again full of options. You could for example have them defined in a files or in a database and have some object construct them for you. As I said. All depend on what you want to use it for.

new Enemy(
    "Enraged Barbarian",
    Class.Warrior,
    new Drop(
        new ChanceInPercent(5),
        new Item("Barbarian's Mighty Axe", Item.Type.Weapon, new GoldCoins(50),
            new Attributes(
                new Attribute(Attribute.Type.Damage, new Stat(Stat.Type.Plus, 50)),
                new Attribute(Attribute.Type.Ability, new Decapitate(new HealthThreshold(20)))
            )
        )
    )
);
\$\endgroup\$
1
\$\begingroup\$

I have comments on the technical and functional (design) levels:

Technical notes

  1. I can see you know about variable inheritance when you defined value to be protected. So why are name and level private if every subclass has them defined as well? do you really need to have a different name for a Weapon and for its super Item?

  2. You need to take all the "magic numbers" out of the source code and put them in properties files. by "magic numbers" I don't mean just the numerical literals (level ranges, values etc) I also mean all the item names. There are at least three good reasons for that:
    the first and obvious is to avoid typos, so if you ever need to compare some item's name, it absolutely has to be defined in one place and one place only. so declaring a constant like public static final String SLIME_DROP = "Slime Drop"; would suffice, but then there is also
    reason #2) you will want to tweak and balance the items. for example, you may find that a certain weapon is too weak etc. it is way more easier to modify values in properties files than inside the source code.
    reason #3) let us not forget modding - letting others modify and extend your game. by externalizing data as much as possible, you make their work that much easier as well.

Design notes

While it is feasible to have Weapon, Armor and the rest as sub classes of Item, I would go for the design of composition, meaning that Weapon, Armor and the rest are types of Items - Weapons are all items that do damage - i.e. reduce the player's stats (health. mana etc), Armor do the opposite, increase player's stats, and so on. I would create a class ItemType that can be either Weapon, Armor, Medical or Misc. the Item class will have a reference to an ItemType instance so that Item with "Medicinal Herb" name will have a Medical type. the advantage of this approach is that you can have items that have multiple types: a magic spell that takes health from the target and gives it to the caster. an armor that gives electrical shock when is hit with melee weapon.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.