FAQ: Why does DoNotExposeGenericLists recommend that I expose Collection instead of List? [David Kean]
DoNotExposeGenericLists fires when I publicly expose List<T> via a field, method, property, or parameter. Why?
Although Krzysztof briefly touched on this last year, we wanted to expand on this a little through the use of examples.
The first reason is that List<T> is designed for speed and for use as an internal implementation detail, whereas Collection<T> is designed for extensibility.
Collection<T> provides 4 overridable methods; ClearItems, InsertItem, RemoveItem and SetItem, which allow a derived class to be notified when a collection has been modified. In contrast, List<T> provides none.
As an example, let’s say in V1.0 of your framework, you have a Person class that exposes a collection of addresses. You could choose to expose this via a property that returns either a List<Address> or Collection<Address> :
List<Address>:
public class Person
{
private List<Address> _Addresses = new List<Address>();
[...]
public List<Address> Addresses
{
get { return _Addresses; }
}
}
Collection<Address>:
public class Person
{
private Collection<Address> _Addresses = new Collection<Address>();
[...]
public Collection<Address> Addresses
{
get { return _Addresses; }
}
}
Along comes V2.0 of your framework, and your users have asked you to expose an event that gets raised every time an address is added or removed from the collection. Had you originally headed down the List<T> route, you’ve now painted yourself in a corner. There is no way to be notified (without breaking existing clients) of when an external object modifies the collection. However, had you chose to expose Collection<T>, you can now do something like the following:
public class Person
{
private AddressCollection _Addresses = new AddressCollection(this);
[...]
public event EventHandler AddressChanged;
public Collection<Address> Addresses
{
get { return _Addresses; }
}
protected virtual void OnAddressChanged(EventArgs e)
{
if (AddressChanged != null)
{
AddressChanged(this, e);
}
}
private void RaiseAddressChanged()
{
OnAddressChanged(EventArgs.Empty);
}
private class AddressCollection : Collection<Address>
{
private Person _Owner;
public AddressCollection(Person owner)
{
_Owner = owner;
}
protected override void InsertItem(int index, Address item)
{
base.InsertItem(index, item);
_Owner.RaiseAddressChanged();
}
protected override void RemoveItem(int index)
{
base.RemoveItem(index);
_Owner.RaiseAddressChanged();
}
protected override void ClearItems()
{
base.ClearItems();
_Owner.RaiseAddressChanged();
}
}
}
From the above code you can see that you are still exposing the same Collection<Address> property, so no breaking change was made, however, your users can now be notified when someone changes the collection.
The second reason why you shouldn’t expose List<T>, is because it exposes too many members, many of which are not relevant in most situations. In contrast, Collection<T> exposes only a small number. For those that are relevant, it is easy to encapsulate List<T> and expose those that you want to make use of via a class that derives from Collection<T>. For example, continuing on from above:
public class AddressCollection : Collection<Address>
{
public AddressCollection() : base(new List<Address>())
{
}
public Address Find(Predicate<Address> match)
{
// We know Items is actually a List<Address> as we passed it to the constructor
List<Address> items = (List<Address>)Items;
return items.Find(match);
}
}
As you can see, the AddressCollection simply calls the underlying List<T>'s Find implementation, saving you from having to implement your own.
If you have any questions or issues with Managed Code Analysis or FxCop (including DoNotExposeGenericLists), don’t hesitate to head over to the FxCop Forum.
Comments
- Anonymous
May 01, 2006
But data binding doesn't work against Collection<T> - Anonymous
May 01, 2006
Ayende,
Are you sure about that? I just tried binding a Collection<T> to a combobox and it worked like a charm.
It implements the same interfaces as List<T>, so can you point to a situation (maybe a code sample) where it doesn't work? That way we can either fix our docs or file a bug on the databinding side.