Answered by:
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 AMModerator -
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 AMModerator -
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 7, 2011 2:44 AM Fixed reference in 2nd code block
Thursday, January 6, 2011 11:47 PM