Inheriting List<T> to implement collections a bad idea?

Question

Hi All,

I once read an article by Imaar Spaanjars on how to build 3 tier applications. (http://imar.spaanjaars.com/416/building-layered-web-applications-with-microsoft-aspnet-20-part-1) which has formed the basis of my coding for a while now.

Thus I implement collections as he has done, by inheriting a List<T>. So if I have a class named Employee,to implement a collection I will also have a class Employees as below.

class Employee
{
   int EmpID {get;set;}
   string EmpName {get;set;}  

}

class Employees : List<Employee>
{
   public Employees(){}
}

I never really questioned this as it did the work for me. But now that I started trying out a few things I am not sure if this is the correct approach.

e.g. if I want to get a subset from Employees, such as

 Employees newEmployees = (Employees) AllEmployees.FindAll(emp => emp.JoiningDate > DateTime.Now);

This throws a System.InvalidCastException . However, if I use the following then there is no Issue.

List<Employee> newEmployees = AllEmployees.FindAll(emp => emp.JoiningDate > DateTime.Now);

So how do I implement Employees so that I dont have to explicitly use List<Employee> in my DAL or BLL? Or maybe how do I get rid of the InvalidCastexception?

Answer 1

I wouldn't inherit from List<T> - it introduces issues like these, and doesn't really help (since there are no virtual methods to override). I would either use List<T> (or the more abstract IList<T>), or to introduce polymorphism Collection<T> has virtual methods.

As a note; re things like FindAll, you may also find the LINQ options (like .Where()) useful counterparts; most notably, they will work for any IList<T> (or IEnumerable<T>), not just List<T> and subclasses.

Answer 2

What benefit are you getting by sub-classing List<Employee>? If it adds nothing then it's unnecessary code-bloat. If you're not showing methods or are enforcing contracts through a constructor that you haven't shown here then there that might be a valid reason for sub-classing.

In terms of resolving your casting problem you can't downcast from a List<Employee> to an Employees as List<Employee> doesn't inherit from Employees. If you need to return an Employees with your criteria then you're best to encapsulate the call and insert the returned list items into your own Employees object. This seems like a waste of time to me unless, like I said above, you've got a good reason to sub-class List<Employee>.

Personally, I'd try and use IList<T> where ever possible and don't create subclasses unless they have a reason to exist.

Answer 3

I would personally store the list as a member of a container object, and if necesery provide get accessor to the list itself.

class MyCollection<T> 
{
    List<T> _table;
    public List<T> Table
    {
        get
        {
            return _table;
        }
    }
    // ... other access/utility functions common for all tables 
    //     (CRUD for example)
}

class Employees : MyCollection<Employee>
{
    // ... yet more methods 
}

I must admin that I did that to provide myself with methods like:

T FindById(int id);
void Add(T entity);
void Remove(T entity);

on base class and:

T[] GetEmployeesBy(your_filter_here);

I (still) use .net 2.0 so I have no linq - maybe that's the reason for it here.

Answer 4

A simple rule of thumb is to start with COMPOSITION (e.g. wrap Employees around a generic collection ) and NOT INHERITANCE. Starting off with inheritance based design is painting yourself into a corner. Composition is more flexible and modifiable.