We have a class that has a public property that is of type List<T>.
FXCop generates a DoNotExposeGenericLists error, indicating
"System.Collections.Generic.List<Tis a generic collection designed
for performance not inheritance and, therefore, does not contain any
virtual members. The following generic collections are designed for
inheritance and should be exposed instead of
System.Collections.Generic.List<T>.
* System.Collections.ObjectModel.Collection<T>
* System.Collections.ObjectModel.ReadOnlyCollection< T>
* System.Collections.ObjectModel.KeyedCollection<TKe y, TItem"
Our property is not "virtual" and thus cannot be overridden. We used
List<Tbecasue we wanted a generic array. A collection is unordered
and thus not as good a match, even if it is better designed for
inheritance.
Can anyone explain in more depth why having a non-virtual, List<t>
property is bad? Why should I care is List<Thas any virtual members
when I am simply using an instance of and _not_ inheriting from
List<T>?
List<Tis intended to be a workhorse class used in implementation. By exposing
a List<Tin the interface of your class, you are breaking encapsulation
by giving client code access to an implementation detail of your class--especially
if your property provides direct access to the internal List<T>. For example,
this breaks encapsulation:
public class EmployeeCensus
{
private List<Employeem_Employees;
public EmployeeCensus(IEnumerable<Employeeemployees)
{
m_Employees = new List<Employee>(employees);
}
public List<EmployeeEmployees { get { return m_Employees; } }
}
Any client code of the EmployeeCensus class could easily corrupt its state
by calling methods on the List<Employeeexposed by the Employees property.
And, if client code is responsible for maintaining the state of the EmployeeCensus
class, there's a design issue.
Proper encapsulation is to *not* expose the List<Tbut to expose a Collection<T>,
ReadOnlyCollection<Tor KeyedCollection<TKey, TItemlike this:
public class EmployeeCensus
{
private List<Employeem_Employees;
public EmployeeCensus(IEnumerable<Employeeemployees)
{
m_Employees = new List<Employee>(employees);
}
public ReadOnlyCollection<EmployeeEmployees { get { return m_Employees.AsReadOnly();
} }
}
It's a shame that these classes were included in a different namespace (System.Collections.ObjectModel)
because many developers haven't discovered them yet or realize that they
should be using them.
Best Regards,
Dustin Campbell
Developer Express Inc.
