Combining two classes to hold general and specific properties of an item

Question

I have an ItemData class in my program that stores saved data about a game inventory item that can change, such as its durability, quality, number of bullets in the clip and similar. ItemClass, on the other hand, lives in a DLL, and holds data that is always true for specific item, such as its name, max durability, size of the clip and similar, regardless of its state.

I believe the two classes should be combined using some standard design pattern, but I'm not sure how.

---

ItemClass is in a DLL that loads data from a specific file type and then presents that data in this form.

public class ItemClass
{
    public byte itemVersion;
    public int type;
    public int timesUsed;
    public int quality;
    public int meta;
    public ItemClass[] parts = new ItemClass[0];
    public List<ItemClass> moreParts= new List<ItemClass>();
    public bool activated;
    public byte index;

    //Some methods specific to this class
}

I actually wrote the library, so I could change it, but I would rather not break encapsulation or make the library dependent on the program.

---

This class is in my program:

public class ItemData
{
    public static List<ItemData> itemList = new List<ItemData>();

    public const int MAX_QUALITY = 5;
    public const int DEFAULT_STACKNUMBER = 100;

    public int id;
    public string name;
    public int stackNumber = 0;

    public bool isBlock = false;
    public bool hasQuality = false;
    public int degradationMin = 0;
    public int degradationMax = 0;
    public int magazineSize = 0;
    public string partType = "";
    public string[] morePartsNames;
    public string[] partNames;
    public string[] magazineItems;
    public byte[] iconPixels;

    public bool isDeveloper = false;

    public static ItemData GetItemDataById(int id)
    {
        foreach (ItemData itemData in itemList)
        {
            if (itemData.id == id)
            {
                return itemData;
            }
        }

        return null;
    }

    //Some more methods specific to this class
}

The program reads some XML files to extract rules regarding ItemClass above. There are about 1500 different items with different values and rules. All are stored in a static list in the ItemData class.

---

When dealing with the ItemClass, I need info from ItemData in order to know how to present the data to the user and how to manipulate it in a way that won't break any of the rules of the item (eg. not allow the user to set bullets in the clip to 10 if clip size is only 8). They are the two sides of the same item.

I can match each ItemClass.type to exactly one ItemData.id.

However, I'd like to treat connected ItemClass and ItemData as a single class, since they do add up to a single item:

public class ItemBinder
{
    public ItemClass itemClass;
    public ItemData itemData;

    public int type;
    public int quality;
    public int timesUsed;
    public int meta;
    public byte ammoIndex;
    public bool activated;
    public ItemClass[] parts;
    public List<ItemClass> moreParts;

    public int id;
    public string name;

    public bool isBlock;
    public bool hasQuality;
    public int degradationMin;
    public int degradationMax;
    public int magazineSize;
    public string partType;
    public string[] morePartsNames;
    public string[] partNames;
    public string[] magazineItems;
    public byte[] iconPixels;

    public bool isDeveloper;

    public ItemBinder(ItemClass itemClass)
    {
        this.itemClass = itemClass;

        type = itemClass.type;
        quality = itemClass.quality;
        itemsUsed= itemClass.timesUsed;
        meta = itemClass.meta;
        ammoIndex = itemClass.ammoIndex;
        activated = itemClass.activated;
        parts = itemClass.parts;
        moreParts = itemClass.moreParts;

        itemData = ItemData.GetItemDataById(type);

        id = itemData.id;
        name = itemData.name;
        isBlock = itemData.isBlock;
        hasQuality = itemData.hasQuality;
        degradationMin = itemData.degradationMin;
        degradationMax = itemData.degradationMax;
        magazineSize = itemData.magazineSize;
        partType = itemData.partType;
        morePartsNames = itemData.morePartsNames ;
        partNames = itemData.partNames;
        magazineItems = itemData.magazineItems;
        iconPixels = itemData.iconPixels;
        isDeveloper = itemData.isDeveloper;
    }

    //Some methods for manipulating the class
}

I have this whole situation in two places in my program, and as it expands, there may be more.

I'm guessing that abstract factory would be a way to go here, right? I would give the concrete ItemBinderFactory factory ItemClass as a parameter and it would return appropriate ItemBinder.

But there's an itch that abstract factory isn't scratching, which makes me think this can be improved on another level. Abstract factory would streamline the way this objects are created, but the emphasis here is that the two objects are just two parts of a whole — all fields in both objects are present in the final one. There must be some better way to deal with this.

Also note that the ItemBinder class has to contain additional methods that uses data from both ItemClass and ItemData. So ItemBinder would inherit all the (non-static or const) fields, but none of the methods, and would add some of its own.

Answer 1

You make it more complicated then necessary. The duplication of all properties in the ItemBinder is a manintenace hell.

I have this whole situation in two places in my program, and as it expands, there may be more.

If you add more properties then you need to update two data structures.

It would be enough if you just created a FullItem like:

public class FullItem
{
    public ItemClass itemClass;
    public ItemData itemData;
}

and build it with a simple join:

var classess = new ItemClass[] 
{
    new ItemClass{ type = 1 },
    new ItemClass{ type = 2 },
};

var data = new ItemData[]
{
    new ItemData { id = 1 },
    new ItemData { id = 2 },
};

var fullItems = classess.Join(data, c => c.type, d => d.id, (c, d) => new FullItem 
{ 
    itemClass = c, 
    itemData = d 
}).ToList();

now you can extend either class and don't worry about copying any properties.

---

Code

We really don't use public fields. All of them should be properties whose names are in PascalCase:

public class ItemClass
{
    public byte ItemVersion { get; set; }
}

---

public static List<ItemData> itemList = new List<ItemData>();

Storing the results in a static field (it actaully should be a property if any) in the same class is not a good idea.

The method loding the data should return a collection and you should store the result in your application not in the library.

---

type & id

This is very confusing as the names do not implicate any relation. If the ItemClass has a Type property then then foreign-key in the ItemData should be called ItemClassType. The Join would be much more natural then and self explaining:

var fullItems = classess.Join(
    data, 
    itemClass => itemClass.Type, 
    itemData => itemData.ItemClassType, 
    (itemClass, itemData) => new FullItem 
    { 
        ItemClass = itemClass, 
        ItemData = itemData 
    }).ToList();

---

Alternative solution (not recommended - can cause maintenace problems) but this way you at least avoid copying the values and use the original objects for storing them:

public class FullItem
{
    public FullItem(ItemClass itemClass, ItemData itemData)
    {
        ItemClass = itemClass;
        ItemData = itemData;
    }

    private ItemClass ItemClass { get; }

    private ItemData ItemData { get; }

    public string Value 
    { 
        get { return ItemClass.Value; } 
        set { ItemClass.Value = value; }
    }
}

Answer 2

If you have no opportunity to change the two classes, you'll have to create some kind of wrapper for the pair of related ItemData and ItemClass instances.

I think your approach is OK, but I have the following comments:

I would not hide the creation of ItemData in the ItemBinder constructor. Instead I would call the constructor with the pair of ItemData and ItemClass instances.

Instead of fields for the fields of ItemData and ItemClass I would create corresponding Properties with the appropriate accessability:

      public class ItemBinder
      {
        ItemClass m_itemClass;
        ItemData m_itemData;

        public ItemBinder(ItemClass itemClass, ItemData itemData)
        {
          m_itemClass = itemClass;
          m_itemData = itemData;
        }

        public int Id => m_itemData.id;
        public int Type => m_itemClass.type;

        public int TimesUsed
        {
          get { return m_itemClass.timesUsed; }
          set
          {
            m_itemClass.timesUsed = value;
          }
        }
// and so on...

      }

Using Linq you can reduce GetItemDataById() to:

  public class ItemData
  {

    public static ItemData GetItemDataById(int id)
    {
      return itemList.FirstOrDefault(item => item.id == id);
    }
}