IS it OK to use an int for the key in a KeyedCollection

Question

Often times I need a collection of non-sequential objects with numeric identifiers. I like using the KeyedCollection for this, but I think there's a serious drawback. If you use an int for the key, you can no longer access members of the collection by their index (collection[index] is now really collection[key]). Is this a serious enough problem to avoid using the int as the key? What would a preferable alternative be? (maybe int.ToString()?)

I've done this before without any major problems, but recently I hit a nasty snag where XML serialization against a KeyedCollection does not work if the key is an int, due to a bug in .NET.

Answer 1

It might be best to add a GetById(int) method to a collection type. Collection<T> can be used instead if you don't need any other key for accessing the contained objects:

public class FooCollection : Collection<Foo>
 { Dictionary<int,Foo> dict = new Dictionary<int,Foo>();

   public Foo GetById(int id) { return dict[id]; }

   public bool Contains(int id) { return  dict.Containskey(id);}

   protected override void InsertItem(Foo f)
    { dict[f.Id] = f;
      base.InsertItem(f);
    }

   protected override void ClearItems()
    { dict.Clear();
      base.ClearItems();
    }

   protected override void RemoveItem(int index)
    { dict.Remove(base.Items[index].Id);
      base.RemoveItem(index);
    }

   protected override void SetItem(int index, Foo item)
    { dict.Remove(base.Items[index].Id);
      dict[item.Id] = item;
      base.SetItem(index, item);
    }
 }









 }

Answer 2

The key in a KeyedCollection should be unique and quickly derivable from the object being collected. Given a person class, for example, it could be the SSN property or perhaps even concatenating FirstName and LastName properties (if the result is known to be unique). If an ID is legitimately a field of the object being collected than it is a valid candidate for the key. But perhaps try casting it as a string instead to avoid the collision.

Answer 3

An easy solution might be to wrap the int into another type to create a distinct type for overload resolution. If you use a struct, this wrapper doesn't have any additional overhead:

struct Id {
    public int Value;

    public Id(int value) { Value = value; }

    override int GetHashCode() { return Value.GetHashCode(); }

    // … Equals method.
}

Answer 4

Basically you need to decide if users of the class are likely to be confused by the fact that they can't, for example, do:

for(int i=0; i=< myCollection.Count; i++)
{
    ... myCollection[i] ...
}

though they can of course use foreach, or use a cast:

for(int i=0; i=< myCollection.Count; i++)
{
    ... ((Collection<MyType>)myCollection)[i] ...
}

It's not an easy decision, as it can easily lead to eisenbugs. I decided to allow it in one of my apps, where access from users of the class was almost exclusively by key.

I'm not sure I'd do so for a shared class library though: in general I'd avoid exposing a KeyedCollection in a public API: instead I would expose IList<T> in a public API, and consumers of the API who need keyed access can define their own internal KeyedCollection with a constructor that takes an IEnumerable<TItem> and populates the collection with it. This means you can easily build a new KeyedCollection from a list retrieved from an API.

Regarding serialization, there is also a performance problem that I reported to Microsoft Connect: the KeyedCollection maintains an internal dictionary as well as a list, and serializes both - it is sufficient to serialize the list as the dictionary can easily be recreated on deserialization.

For this reason as well as the XmlSerialization bug, I'd recommend you avoid serializing a KeyedCollection - instead only serialize the KeyedCollection.Items list.

I don't like the suggestion of wrapping your int key in another type. It seems to me wrong to add complexity simply so that a type can be used as an item in a KeyedCollection. I'd use a string key (ToString) rather than doing this - this is rather like the VB6 Collection class.

FWIW, I asked the same question some time ago on the MSDN forums. There is a response from a member of the FxCop team, but no conclusive guidelines.