Usability of KeyedCollection<int, MyType>

Question

Not really an FxCop question, but I guess the FxCop guys are a good source for usability guidelines.

FxCop 1.35 doesn't complain about a KeyedCollection with an integer key, e.g. the first example in MSDN: http://msdn2.microsoft.com/en-us/library/ms132438.aspx

 

public class SimpleOrder : KeyedCollection<int, OrderItem> {     ...     protected override int GetKeyForItem(OrderItem item)       {         // In this example, the key is the part number.         return item.PartNumber;       }   }

 

There is a potential usability issue in that users of the type may be surprised to see that items can't be accessed by index (the indexer "public T this[int index]" is masked by "public T this[TKey key]"), so that the following code will fail:

SimpleOrder order; ... for(int i=0; i<order.Count; i++) {    OrderItem item = order[ i ]; }

 

Question

- Do the FxCop usability experts consider this to be a usability issue?

- If so, what would be the recommended alternative?  E.g. one alternative would be to fake a string key "item.PartNumber.ToString(CultureInfo.InvariantCulture)", something like we used to do with VB6 collections.

 

 

Answer 1

This definitely looks like a usability problem.

 

There are a couple of ways I would approach this:

 

Define a type that represents the part number, for example:

 

public class PartNumber {     private readonly int _id;     public class PartNumber(int id)       {        _id = id;     }         public override int GetHashCode()       {            return _id;     } }   public class SimpleOrder : KeyedCollection<PartNumber, OrderItem> { }

 

Or if the part number (when I was in retail this was likely) could every be made up of both numeric and alpha characters (like LM5000), then I would do as you proposed, use a string to represent the part number.

 

Regards

 

David

If part numbers could are strings They way I would approach this, is by either

Answer 2

David,

Thanks for the response.

 

I'm really more interested in the general case than the specific, rather contrived, example from MSDN.

 

Creating a custom type such as PartNumber to wrap an integer Id might work in some cases (though the type should probably be a struct and implement conversion operators to/from Int32).

 

But in the general case, I might want a KeyedCollection of an existing type that has an integer Id property, and it is for this case that I was looking for feedback.  It won't always be possible or desirable to replace the integer Id by a custom type that wraps the integer.

 

My conclusion after thinking about it for a while is that it is acceptable to use KeyedCollection<int, MyType>, possibly adding warnings to the XML comments about the indexer.

 

As for alphanumeric ids like LM5000, clearly these would be represented as a string, so the issue doesn't arise.  My (alternative) proposal was to cast integer id's to/from string when accessing by key, like VB6 collections, e.g.:

public class SimpleOrder : KeyedCollection<string, OrderItem> {     ...     protected override int GetKeyForItem(OrderItem item)       {         // In this example, the key is the part number.         return item.PartNumber.ToString(CultureInfo.InvariantCulture);       }   } ... SimpleOrder myOrder = ...; ... // Access by key int partNumber = ...; OrderItem = myOrder[partNumber.ToString(CultureInfo.InvariantCulture)];

 

But this approach has its own usability problems.

 

In any case, thanks for your feedback.

 

Answer 3

 

The two approaches I have seen are to either add an additional member for your KeyedCollection (or a generic subclass for int keyed KeyedCollections):

 

 

public class SimpleOrder : keyedCollection<int, OrderItem>
{
  ...
  public OrderItem GetItemByIndex(int index)
  {
      return this.Items[index];
  }
}

or to cast the KeyedCollection to a Collection and then use the indexer:

 

 

SimpleOrder order;
...
Collection orderCollection = (Collection) order;
for(int i = order.Count - 1; i >= 0; i--)
{
 OrderItem item = orderCollection[ i ];
}