Inheriting and storing objects that contain different types of other objects

Question

To start off, this is more of a best-practice question than anything.

Introduction to the Environment

I have myself an abstract class. Let's call this class Item. I also have another abstract class, we'll call this one Container. The Container class inherits the Item class.

Here's my dilemma: I'm trying to right-properly design the OOP portions of it, and I'm having a bit of an issue due to my desire to abstract away as much as possible, while still creating maintainable and understandable code.

Let's start with some code:

public abstract class Item
{
    private Guid _Guid;
    private float _Weight;

    public Guid Guid { get { return _Guid; } set { _Guid = value; } }
    public float Weight { get { return _Weight; } set { _Weight = value; } }

    public virtual ItemType ItemType { get { return ItemType.None; } }
}

public abstract class Container : Item
{
    private float _MaxWeight;
    private List<Item> _Items;

    public List<Item> Items { get { return _Items; } }
    public float MaxWeight { get { return _MaxWeight; } set { _MaxWeight = value; } }
    public abstract ContainerType ContainerType { get; }

    public sealed override ItemType ItemType { get { return ItemType.Container; } }
}

So, with this code the intention is for a Container to be able to be treated as an Item, with a few exceptions. (The container will have additional properties, etc.)

The issue, is that not all Item objects can be added to a Container. In fact, only Container objects that have a smaller Container.MaxWeight or non-Container Item objects can be added.

As of now, this restriction cannot be placed. My initial thought was to create an ItemCollection inside the Container class, that would essentially hold all of the Item objects within it. This would allow me to implement my own ItemCollection.Add method which would perform this filter.

The issue comes into play when I introduce my third class. Let's call it a Backpack. This inherits Container and looks as follows:

public class Backpack : Container
{
    public sealed override ContainerType ContainerType { get { return ContainerType.Backpack; } }
}

Mind you - the Backpack (as we are calling it) will only be able to hold Item objects, or Container objects with a smaller Container.MaxWeight in them.

This also means that I may yet have other classes (let's use, for example, Pouch) that will also inherit Container, but which cannot contain other Container objects.

Lastly, there may also be additional classes that cannot hold Item objects, but only hold Container objects. (See, for example, Inventory.)

/// <summary>
/// Represents a collection of <see cref="Container"/> objects owned by an <see cref="Actor"/>.
/// </summary>
public class Inventory : Container
{
    public sealed override ContainerType ContainerType { get { return ContainerType.Inventory; } }
}

Question Statements

Is this the best way to do it? Should I implement the ItemCollection as I intended? What about Container objects that can only hold other Container objects?

Should I use the ContainerType.Inventory to indicate that this object can only hold other Inventory objects? Should I use the ContainerType.MaxWeight value on it instead? (Any objects that can only hold Container objects won't really have a MaxWeight. The MaxWeight an Actor can hold is determined by their fitness.)

Should I make an additional class that inherit Container that can only hold other Container objects, then inherit Inventory from this? Likewise, should I create another class inheriting from Container that allows or disallows the addition of Item objects only?

Additional Notes/Jargon

The idea is to force the programmers (mostly me) to properly handle each Container and Item as they should be. The issue lies in the fact that there are different types of Container objects. Also, do note that this is not all of the code involved, but the additional code I excluded is irrelevant to this issue. Also note, the two additional classes I mentioned (Inventory and Backpack) are completely concrete. I will be working directly with those. I also want to have common infrastructure between them, so that any Item that is a Container can be worked in a general manner. (Rather than casting to the appropriate concrete class.)

Answer 1

The direction where you are going is great. I would make a few tweaks to this design like so:

public abstract class Container : Item
{
    private float _MaxWeight;
    private List<Item> _Items;

    public List<Item> Items { get { return _Items; } }
    public float MaxWeight { get { return _MaxWeight; } set { _MaxWeight = value; } }
    public abstract ContainerType ContainerType { get; }

    public IEnumerable<IDepositFilter> Filters { get; set; } // Probably a factory can stuff filters as required per each type.

    public void DepositItem(Item item) {
        foreach(var filter in Filters) {
            if(!filter.IsAllowed(item)) {
                throw new ItemNotAllowedException();
            }
        }

        _Items.Add(item);
    }
}

public interface IDepositFilter
{
    bool IsAllowed(Item item);
}

public class DefaultContainerFilter : IDepositFilter {
    // probably do nothing here
}

public class OversizeContainerFilter : IDepositFilter
{
    // Check if the size of the new deposit is larger than leftover space in this container
}

public class ItemTypeFilter : IDepositFilter
{
    // check what is being deposited and allow/disallow accordingly.
}

Notice, I introduced a filter collection that should be used when you are adding something to your container. You can add as many filters as you want to your container of a specific type. Each filter does one and only one check.

I also remove the ItemType as it seems redundant - you can use classes themselves to determine the type.

Reuse the filters on anything you want :)

Hope this helps.

Answer 2

Unfortunately, exposing public List Items {get;} breaks the whole encapsulation and protection mechanisms. The using code can just do container.Items.Add and put whatever they want. (which is a problem in the other answer as well).

More and more I start to prefer composition instead of inheritance, and when some form of inheritance is needed (to avoid duplicated code), a decorator pattern may come in place.

And for the sake of the argument (I understand that the code sample is not complete), I do not see a reason of using an abstract class to start with. Interface may be more appropriate, if we want just to describe what properties an item can have.

Although the above is the way to go, if there is a "real" need to use inheritance, here is an example code how I would approach this:

public abstract class Item
{
   public string SomeData{get;set;)
}

public abstract class Container : Item
{
    private readonly List<Item> _items;
    public IEnumerable<Item> Items { get { return _items;}}
    public Container()
    {
       _items = new List<Item>();
    }

    public void AddItem(Item item)
    {
       if (IsValidItem(item))
       {
           _items.Add(item);
       }
    }

    protected virtual bool IsValidItem(Item item)
    {
       ///some validation logic, etc
    }
}

With the above very simplistic/schematic design its easy to achieve the questions from the original post. In the other types of containers, it is about overriding the protected IsValid method to do whatever it need to be (incl if it has to call the base method, etc.).

Also, we do not break the encapsulation, so external consumers of the class can not add items w/o validation.

When questions start to come about reseting the container, removing items, etc., probably it's better to make your container class to inherit from Item, and implement ICollection interface, using a backing storage of internal list, and forward (trough some validation) the corresponding calls to it:

public class Container : Item, ICollection<Item>

NOTE: As I said earlier, I will not go with this approach, it is a nightmare to test and extend. And it probably in the real world implementation is going to break one or more of the SOLID principles.