Trying to create an Item object class

I’d like to have an ItemRegister.java and Item.java combo where the ItemRegister performs the initialization via new Item() to construct each Item and add it to some type of storage (eg. List<Item>). Also, when I go to add an Item to the player inventory, I’d like to call a getItemByID() or getItemByName() so it’s easier. Right now I can call Item.itemReg#getItemByID, but I know there is a much better way than this. Thanks for not flaming. I’m still learning Java.

Basically what I have is this.

public class Item {
    
    public enum Type{
        FOOD,
        POTION,
        MATERIAL,
        EQUIPMENT,
        MISC
    }
    
    private int id;
    private String name;
    private Type type;
    
    public static ItemRegistry itemReg = new ItemRegistry();
    
    public Item(int id, String name, Type type){
        this.id = id;
        this.name = name;
        this.type = type;
    }
    
    public Item(int id, String name){
        this.id = id;
        this.name = name;
        this.type = Type.MISC;
    }
    
    public int getID(Item item){
        return item.id;
    }
    
    public String getName(Item item){
        return item.name;
    }
    
    public Type getType(Item item){
        return item.type;
    }
    
    public Item getItemByID(int id){
        return itemReg.getItemByID(id);
    }
    
    public static void initItems(){
        itemReg.add(new Item(1, "Pear", Type.FOOD));
        itemReg.add(new Item(1, "Bread", Type.FOOD));
        itemReg.add(new Item(1, "Apple", Type.FOOD));
    }
}

and

import java.util.ArrayList;
import java.util.List;

public class ItemRegistry {
    
    protected List<Item> itemRegister;
    
    public ItemRegistry(){
        this.itemRegister = new ArrayList<Item>();
    }
    
    public Item getItemByID(int id){
        return itemRegister.get(id);
    }
    
    public void add(Item item){
        if(itemRegister.contains(item)){
            System.out.println("WARNING: Attempted to add duplicate item to registry.");
        } else {
            itemRegister.add(item);
        }
    }
}

If i understand correctly:

HashMap<Integer,Item> itemById
HashMap<String,Item> itemByName

could do it?

@Empire Phoenix said: If i understand correctly:

HashMap<Integer,Item> itemById
HashMap<String,Item> itemByName

could do it?

Yes, I originally used a hashmap, but then I switched to an ArrayList because I only need to store the Item objects. The Item object contains all the other data. I may change back to hashmap if I decide to allow “stackable” items that would require more metadata.

Thanks for the info, I got rid of the register class altogether and put everything in the Item class.

I do something similar with a registry because it allows for mod support, since I have an external API. If you don’t plan on adding mod support, I suppose it doesn’t really matter what you do, but having a ItemRegistry utility class with static methods can make it easier for modders and the likes (and possibly easier for yourself).


import java.util.ArrayList;
import java.util.List;
 
public class ItemRegistry {
    
    protected static Map<Integer, Item> itemRegister = new HashMap<>();
    
    public static Item getItemByID(int id){
        return itemRegister.get(id);
    }

    public static Item getItemByName(String name) {
        for (Item i: itemRegister.values()) {
            if (i.getName().equals(name)) {
                return i;
            }
        }
        return null;
    }
    
    public static void add(int id, Item item){
        if(itemRegister.containsValue(item)){
            System.out.println("WARNING: Attempted to add duplicate item to registry.");
        } else {
            itemRegister.put(id, item);
        }
    }
}

I’d just make everything in the registry static, since it IS a utility class and all.

Bit sde note, but in Item class:
getItemByID should be static
all other getters should NOT accept item as a parameter, but return data from ‘this’ object. Or, if you are into very fancy programming, make them static.

Additionally, itemRegister.get(id) does something else that you think. It retrieves n-th item which was added, rather than item with given id.

@navid113 said: I do something similar with a registry because it allows for mod support, since I have an external API. If you don't plan on adding mod support, I suppose it doesn't really matter what you do, but having a ItemRegistry utility class with static methods can make it easier for modders and the likes (and possibly easier for yourself).

import java.util.ArrayList;
import java.util.List;
 
public class ItemRegistry {
    
    protected static Map<Integer, Item> itemRegister = new HashMap<>();
    
    public static Item getItemByID(int id){
        return itemRegister.get(id);
    }

    public static Item getItemByName(String name) {
        for (Item i: itemRegister.values()) {
            if (i.getName().equals(name)) {
                return i;
            }
        }
        return null;
    }
    
    public static void add(int id, Item item){
        if(itemRegister.containsValue(item)){
            System.out.println("WARNING: Attempted to add duplicate item to registry.");
        } else {
            itemRegister.put(id, item);
        }
    }
}

I’d just make everything in the registry static, since it IS a utility class and all.

@abies said: Bit sde note, but in Item class: getItemByID should be static all other getters should NOT accept item as a parameter, but return data from 'this' object. Or, if you are into very fancy programming, make them static.

Additionally, itemRegister.get(id) does something else that you think. It retrieves n-th item which was added, rather than item with given id.

Thanks peeps, I read the replies on my phone and forgot to comment when I got home. Thanks for all the feedback and ideas. I’ve decided to work on my custom chase camera for a bit though, so thread /kill

Thanks again!
-Navybofus

i suggest something like this


public interface Item {
    public int getId();
    public String getName();
}

public class Bread implements Item {
    private int id;
    private String name;
    public Bread(int id, String name) {
        this.id = id;
        this.name = name;
    }
    public int getId() {
        return id;
    }
    public String getName() {
        return name;
    }
    // Your own comparsion
    public boolean equals(Object obj) {
        return obj instanceof Bread && id == ((Bread)obj).id && name.equals(((Bread)obj).name);
    }
    // This is with equals, they say its for Map searching optimalization, it should return any number,
    // but it must generate everytime same number for same data specified in equals
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + id;
        result = prime * result + name.hashCode());
        return result;
    }
}

public class ItemRegister {
    HashMap<Integer, Item> items1 = new ...;
    HashMap<String, Item> items2 = new ...;

    public void addItem(Item item) {
        if(items1.containsValue(item)) {
            throw new IllegalArgumentException("Attempted to add duplicate item to registry.");
        }
        items1.put(item.getId());
        items2.put(item.getName());
    }

    public Item getItem(int id) {
        return items1.get(id);
    }

    public Item getItem(String name) {
        return items2.get(name);
    }
}

ItemRegister itemRegister = new ...;
try {
    itemRegister.add(new Bread(1, "BreadOne")); // good
    itemRegister.add(new Bread(2, "BreadTwo")); // good
    itemRegister.add(new Bread(2, "BreadTwo")); // exception
    itemRegister.add(new Bread(1, "BreadTwo")); // good, if you remove name check from equals, then exception
} catch(Exception ex) {
    Logger.getLogger("").log(Level.SEVERE, null, ex);
}

Item bread = itemRegister.getItem(1);
Item bread = itemRegister.getItem("BreadOne");

so every class which implements interface Item, can be used as item
notice that getItem() is defined twice - method overloading

Hi,

Take a look at this “Pattern” :slight_smile: Don’t worry, a Good one!
http://www.oodesign.com/factory-pattern.html
Note: I usually make fun of people who using design patterns a lot, but i’m one. :slight_smile: Because life is short and you want to get the work done!

Ok, I guess you may want to have a “Item” system:

  • Registry (List) of Types
  • Prototypes of Original Instances
  • Create Objects by some initial infos (params, type, name)…

What you want usually call a Factory in Factory pattern. The way to implement it is various:

  • Type can be Class, Enum, Int, String, …
  • Protype can be a Map of references, or a solid Object,
  • or a abstract way to construct that object…

The abstract code should look like

class ItemFactory{
     public void create(String type){
          return map[type];
// return a Prototype or a Solid object
// may also register with an unique ID
Object newObject = map[type];
ids[newObject] = ++globalId;
return newObject;
// may be Pooled to avoid memory allocation
Object newObject = pool.get();
((Type)newObject).clone(map[type]);
return newObject;
// may be get from a Cache... or several steps of linkage
Object newObject = Cache.get(++globalId, type);
newObject.lazyInit();
Cache.identify(newObject);
return Cache.referenceTo(newObject);
    }
}

I usually copy + paste these code:


class Item{
  int id;
  ItemType type;
  String name;
  int cost;
  int status;
}

class ItemManager {
  enum ItemType{ A(0,0),B(1,1),C(2,2),D(3,3);               // A Type is like a Class with a Value -> Enum
    ItemType(int typeId, int typeCost){ //other attributes of this Type
    }
  }
  Item create(ItemType type){
    Item newItem = (Item) Pool.getInstance().get();
    newItem.clone(prototypes[type]);
    return newItem;
  }
}

Cheers,