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

4

14

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?

Linguistician answered 20/9, 2010 at 5:41 Comment(0)
G
19

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.

Grant answered 20/9, 2010 at 5:44 Comment(2)
Right, however a small doubt. I have multiple projects like DAL and BLL, Entities and PL. Entities is where I define the classes like Employees and then use them in other projects. The idea is Each project can be worked upon independent of each other. Is there a possibility that a developer may use IList<Employee> in BLL and another developer may use Collection<Employee> in DAL, which may cause issues. The point define an implementation in Entities and let others use it wherever they want it.Linguistician
@sassyboy - since Collection<T> implements IList<T>, that isn't usually an issue. Ultimately you aren't really defining a proper implementation by subclassing List<T>, since that does nothing to allow T-specific logic.Grant
F
11

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.

Fabianfabianism answered 20/9, 2010 at 5:55 Comment(0)
P
4

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

Perfectible answered 20/9, 2010 at 6:10 Comment(0)
O
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.

Overshine answered 20/9, 2010 at 6:30 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.