Inventories, Containers and Filters

Question

Coming off a comment on this question, I figure it cannot hurt to post my Inventory work here as well.

Basically, I have Item, which is a base item, and then Container, which is a specialized item that has sub-items in it. The Container object, however, can be filtered to only permit (or restrict) certain items by some "property" X.

The Container objects can have any number of filters on them, restricting different things. For example, all Container objects have an OversizeContainerFilter, which essentially prevents the container from filling up over the maximum Weight it can hold. The Inventory Container also has an ItemTypeContainerFilter to restrict it to only be permitted to contain other Container items.

Now, this is actually a pretty complex portion of the setup. Essentially, an Item is as follows:

/// <summary>
/// A base class for an item.
/// </summary>
public abstract class Item
{
    /// <summary>
    /// Gets or sets a unique <code>System.Guid</code> to represent the <see cref="Item"/> globally.
    /// </summary>
    public Guid Guid { get; set; }

    /// <summary>
    /// Gets or sets a <code>string</code> of the description of the <see cref="Item"/>.
    /// </summary>
    public string Description { get; set; }

    /// <summary>
    /// Gets or sets a <code>string</code> of the name of the <see cref="Item"/>.
    /// </summary>
    public string Name { get; set; }

    /// <summary>
    /// Gets or sets a <code>float</code> weight for the <see cref="Item"/>.
    /// </summary>
    public virtual float Weight { get; set; }

    /// <summary>
    /// Gets or sets a <see cref="SingleIcon"/> representing the icon image of the <see cref="Item"/>.
    /// </summary>
    public SingleIcon Icon { get; set; }

    /// <summary>
    /// Gets or sets a <code>List&lt;Guid&gt;></code> that represents the list of the <see cref="Material"/>s that make up this <see cref="Item"/>.
    /// </summary>
    public List<Guid> Materials { get; set; }

    /// <summary>
    /// Determines how damaged the <see cref="Item"/> is.
    /// </summary>
    public ushort Wear { get; set; }

    /// <summary>
    /// Determines what <see cref="ItemType"/> the <see cref="Item"/> is. Must be overriden in derived classes.
    /// </summary>
    public virtual ItemType ItemType { get { return ItemType.None; } }
}

These are pretty simple, an Item has certain, base, properties.

Where it starts getting potentially murky is when Container objects come into play:

/// <summary>
/// The base interface for a container.
/// </summary>
public abstract class Container : Item, IList<Item>
{
    private List<Item> _items;
    private List<IDepositFilter> _filters;

    /// <summary>
    /// Gets or sets the maximum weight the <see cref="Container"/> can hold.
    /// </summary>
    public float MaxWeight { get; set; }

    /// <summary>
    /// Gets or sets the preference of the <see cref="Container"/>. Lower values mean a higher preference.
    /// </summary>
    public byte Preference { get; set; }

    /// <summary>
    /// Returns <see cref="ItemType.Container"/>.
    /// </summary>
    public sealed override ItemType ItemType { get { return ItemType.Container; } }

    /// <summary>
    /// Gets the <see cref="Item.Weight"/> of the <see cref="Container"/> and of all <see cref="Item"/> objects within the <see cref="Container"/>.
    /// </summary>
    public sealed override float Weight { get { float totalWeight = base.Weight; foreach (Item item in _items) { totalWeight += item.Weight; } return totalWeight; } set { base.Weight = value; } }

    /// <summary>
    /// Creates a new instance of a <see cref="Container"/> object.
    /// </summary>
    public Container()
    {
        _items = new List<Item>();
        _filters = new List<IDepositFilter>();

        _filters.Add(new OversizeContainerFilter());
    }

    /// <summary>
    /// Gets the list of <see cref="IDepositFilter"/> objects associated with the current <see cref="Container"/>.
    /// </summary>
    public List<IDepositFilter> Filters { get { return _filters; } }

    public bool CanAddItem(Item item)
    {
        foreach (var filter in Filters)
        {
            if (!filter.IsAllowed(this, item))
                return false;
        }

        return true;
    }

    public int IndexOf(Item item) { return _items.IndexOf(item); }

    public void Insert(int index, Item item)
    {
        if (!CanAddItem(item))
            throw new ItemNotAllowedException();

        _items.Insert(index, item);
    }

    public void RemoveAt(int index) { _itemsRemoveAt(index); }

    public Item this[int index] { get { return _items[index]; } set { _items[index] = value; } }

    public void Add(Item item)
    {
        if (!CanAddItem(item))
            throw new ItemNotAllowedException();

        _items.Add(item);
    }

    public void Clear() { _items.Clear(); }

    public bool Contains(Item item) { return _items.Contains(item); }

    public void CopyTo(Item[] array, int arrayIndex) { _Items.CopyTo(array, arrayIndex); }

    public int Count { get { return _items.Count; } }

    public bool IsReadOnly { get { return false; } }

    public bool Remove(Item item) { return _items.Remove(item); }

    IEnumerator<Item> IEnumerable<Item>.GetEnumerator() { return _items.GetEnumerator(); }

    IEnumerator IEnumerable.GetEnumerator() { return _items.GetEnumerator(); }
}

A Container can hold other Item objects, but only if they meet the IDepositFilter requirements:

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

Examples of some IDepositFilter restrictions:

public class DefaultContainerFilter : IDepositFilter
{
    public bool IsAllowed(Container destination, Item item)
    {
        return true;
    }
}

public class ItemTypeContainerFilter : IDepositFilter
{
    private List<ItemType> _allowTypes;
    private List<ItemType> _blockTypes;

    /// <summary>
    /// Creates a new instance of <see cref="ItemTypeContainerFilter"/>.
    /// </summary>
    /// <param name="allowTypes">The <see cref="ItemType"/> enumeration values that should be explicitly allowed. If this is not an empty list, then only these types are permitted.</param>
    /// <param name="blockTypes">The <see cref="ItemType"/> enumeration values that should be explicitly blocked. If this is not an empty list, then these types are disallowed.</param>
    /// <remarks>
    /// If neither parameters are specified, then all <see cref="ItemType"/> enumeration values are blocked. If both are specified, then items in the allow list take priority.
    /// </remarks>
    public ItemTypeContainerFilter(List<ItemType> allowTypes, List<ItemType> blockTypes)
    {
        _allowTypes = allowTypes;
        _blockTypes = blockTypes;
    }

    public bool IsAllowed(Container destination, Item item)
    {
        if (_allowTypes != null && _allowTypes.Contains(item.ItemType))
            return true;

        if (_blockTypes != null && _blockTypes.Contains(item.ItemType))
            return false;

        if (_allowTypes != null)
            return false;
        else
            return true;
    }
}

public class OversizeContainerFilter : IDepositFilter
{
    public bool IsAllowed(Container destination, Item item)
    {
        return destination.MaxWeight >= (item.Weight + destination.Weight);
    }
}

And lastly, the Inventory:

/// <summary>
/// Represents a collection of <see cref="Container"/> objects owned by an <see cref="Actor"/>.
/// </summary>
public class Inventory : Container
{
    /// <summary>
    /// Creates a new instance of the <see cref="Inventory"/>.
    /// </summary>
    public Inventory() : base()
    {
        Filters.Add(new ItemTypeContainerFilter(new List<ItemType> { ItemType.Container }, null));
    }
}

Note: after posting this I did _camelCase all the private variables that were _PascalCase.

Answer 1

For your Container class, the indexer probably should be read only. The Add and Insert methods have CanAddItem checks. But this line:

public Item this[int index] { get { return _Items[index]; } set { _Items[index] = value; } }

could allow someone to replace the value with an item that fails the CanAddItem check. I wouldn't suggest adding extra checks in the setter. It really should be a readonly property.

public Item this[int index] { get { return _Items[index]; } }