locked
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.

     

     

    Tuesday, April 24, 2007 4:33 PM

Answers

  • 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

    Wednesday, April 25, 2007 6:14 AM
  • 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.

     

    Wednesday, April 25, 2007 7:25 PM

All replies

  • 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

    Wednesday, April 25, 2007 6:14 AM
  • 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.

     

    Wednesday, April 25, 2007 7:25 PM
  •  

    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 ];
    }
    
    

     

    • Edited by Thomas S. Trias Friday, January 07, 2011 2:44 AM Fixed reference in 2nd code block
    Thursday, January 06, 2011 11:47 PM