Is KeyedCollection meant only when Key determines the equality of the items in it?

Question

I think this is a common problem we run into.

class Person
{
    public string place;
    public string name;

    public Person(string place, string name)
    {
        this.place = place;
        this.name = name;
    }

    public bool Equals(Person other)
    {
        if (ReferenceEquals(null, other))
            return false;

        return name == other.name;
    }

    public override bool Equals(object obj)
    {
        return Equals(obj as Person);
    }

    public override int GetHashCode()
    {
        return name.GetHashCode();
    }

    public override string ToString()
    {
        return place + " - " + name;
    }
}

Say I have this class. I can implement a KeyedCollection like this:

class Collection : KeyedCollection<string, Person>
{
    protected override string GetKeyForItem(Person item)
    {
        return item.place;
    }
}

The case here is that the default Equals is based on name of the Person but in my case I'm creating a custom collection that will have only one Person per place. In other words place will be unique in collection.

Person p1 = new Person("Paris", "Paul");
Person p2 = new Person("Dubai", "Ali");

var collection = new Collection { p1, p2 };
var p3 = new Person("Paris", "Jean");
if (!collection.Contains(p3))
    collection.Add(p3);   // explosion

I understand the problem. The Contains(Person) overload is that of Collection<T>.Contains(T) which does a value based linear search while Add(Person) does adds the value to the internal dictionary which can cause the duplicate key exception. Here, had equality was based on place, this problem wouldn't have existed.

I can come with a workaround:

class Collection : KeyedCollection<string, Person>
{
    protected override string GetKeyForItem(Person item)
    {
        return item.place;
    }

    new public bool Contains(Person item)
    {
        return this.Contains(GetKeyForItem(item));
    }
}

But this again means if I do a general

var p3 = new Person("Paris", "Jean");
bool b = collection.Contains(p3); //true

returns true, but in reality Jean doesn't exist in collection yet. So my question is, does KeyedCollection<K, T> makes sense only when Equals is based on just the K part of the T? My question is little on the semantics side. I'm not asking for solution, but just knowing is there a general understanding on when would a KeyedCollection make sense? I couldn't find anything relevant on this subject from documentation.

Update:

I have found the exact problem mentioned here http://bytes.com/topic/net/answers/633980-framework-bug-keyedcollection-t

where the asker has submitted a bug report to MS. To quote him (dated Apr 18 '07):

I submitted this as a bug to Microsoft, and they have verified it and accepted it. It's Issue ID 271542, and can be tracked here:

"We have reproduced this bug on WinXP pro SP2 and VSTS2005 SP1, and we are sending this bug to the appropriate group within the Visual Studio Product Team for triage and resolution."

While I don't think this is a bug, this is an annoyance surely. But just wondering how did MS accept this as a bug in the first place (expectedly, the page can't be found now). Imo, its just poorly thought out inheritance model.

Answer 1

There seem to be two things that make sense to ask the collection:

• whether it contains the person named Jean who lives in Paris, and

• whether it contains a person who lives in Paris.

The KeyedCollection<TKey, TItem> Class provides exactly two methods to ask these questions:

• the Contains(TItem) Method, and

• the Contains(TKey) Method.

The person named Jean who lives in Paris is not the same person as the person named Paul who lives in Paris (Person.Equals). But if there is a person who lives in Paris, then according to your rules there cannot be another person who lives in Paris.

So basically you have to ask the collection the right question before adding a new person:

if (!collection.Contains(p3.place))
    collection.Add(p3);

---

For convenience, you could add a TryAdd Method to the class that returns if the person was successfully added or if there is already a person in the collection who lives in the same place:

public bool TryAdd(Person person)
{
    if (Contains(GetKeyForItem(person)))
        return false;
    Add(person);
    return true;
}