AutoDictionary for objects with a key property

Question

The normal dictiory separates the key from the value... but what if the key is a part of the item it stores? It then forces you to enter the same things twice, once for the key and agian for the item. I never liked it so I thought I write really simple class that takes this unecessary step away a makes life easier again by allowing you to specify a key selector like the ToDictionary extension does.

public class AutoDictionary<TKey, TItem> : ICollection<TItem>, IEnumerable<TItem>
{
    private readonly Func<TItem, TKey> _keySelector;

    private readonly IDictionary<TKey, TItem> _items;

    public AutoDictionary(Func<TItem, TKey> keySelector)
    {
        _keySelector = keySelector;
        _items = new Dictionary<TKey, TItem>();
    }

    public AutoDictionary(Func<TItem, TKey> keySelector, IEqualityComparer<TKey> equalityComparer)
    : this(keySelector)
    {
        _items = new Dictionary<TKey, TItem>(equalityComparer);
    }

    public AutoDictionary(Func<TItem, TKey> keySelector, IDictionary<TKey, TItem> other)
    : this(keySelector)
    {
        _keySelector = keySelector;
        _items = new Dictionary<TKey, TItem>(other);
    }

    public TItem this[TKey key]
    {
        get { return _items[key]; }
        set { _items[key] = value; }
    }

    public int Count => _items.Count;

    public bool IsReadOnly => false;

    public void Add(TItem item) => _items.Add(_keySelector(item), item);

    public void Clear() => _items.Clear();

    public bool TryGetItem(TKey key, out TItem item) => _items.TryGetValue(key, out item);

    public bool Contains(TItem item) => _items.ContainsKey(_keySelector(item));

    public void CopyTo(TItem[] array, int arrayIndex) { }

    public bool Remove(TItem item) => _items.Remove(_keySelector(item));

    public IEnumerator<TItem> GetEnumerator() => _items.Values.GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

I wanted to make all methods use lambdas but the constructor doesn't allow this :-(

---

Usage:

class Foo
{
    public string Name { get; set; }
    public int Bar { get; set;}
    public double Baz { get; set;}
}

var autoDic = new AutoDictionary<string, Foo>(x => x.Name)
{
    new Foo { Name = "foo1", Bar = 1 },
    new Foo { Name = "foo2", Bar = 2 },
    //new Foo { Name = "foo1", Bar = 1 }, // bam!
};

var foo1 = autoDic["foo1"];

I guess I've reinvented the wheel, haven't I? ;-)

Answer 1

The single most surprising thing that jumps at me is that an AutoDictionary doesn't implement IDictionary, and therefore can't be used as such. This is a rather major oversight, enough to be a showstopper in deciding whether or not I would use this data structure or not.

Short of implementing and exposing an Add(object, object) method, this AutoDictionary is simply not a dictionary as far as client code is concerned - it's a [keyed] collection, or an enumerable, but not a dictionary.

I would definitely try to find a way to implement the generic IDictionary<TKey,TValue> interface on top of IDictionary, too.

How? Well you already have a TKey generic type parameter, so a generic dictionary implementation would definitely be easy. The non-generic interface could validate the type of the key parameter (e.g. the first object in the Add(object, object) method) and throw some ArgumentException if the type can't be cast to TKey.

I'd also add a constructor that intakes the IDictionary dependency:

public AutoDictionary(IDictionary<TKey, TItem> source, Func<TItem, TKey> keySelector)
    : this(keySelector)
{
    _items = source;
}

Accepting a generic dictionary constrained with TKey would make the _items assignment compile and guaranteed to work, but would would still need to validate that the existing keys are those returned by the keySelector, and throw an exception otherwise.

Why is the second generic type parameter named TItem, when every IDictionary reference in the framework names it TValue? That's another rather surprising detail.

---

This is bad:

public void CopyTo(TItem[] array, int arrayIndex) { }

The method should at least throw a NotImplementedException. This no-op member is part of the interface, the contract you've signed when you decided to implement it: IMO leaving it unimplemented breaks the Liskov Substitution Principle, which means your implementation cannot be used in place of any other ICollection implementation. It should populate the array parameter with all values in stored in _items.

---

The class isn't sealed, which means a new class could be derived from it. Consider implementing the interface members as virtual, so their implementation could be overridden in inherited types - or outright seal the class.