Question regarding custom collections in C#

Question

I would like to know what is the best pattern when returning objects from custom collection classes. To illustrate my problem, here is an example:

I have a Customer class:

public class Customer
{
   //properties
   //methods
}

Then I have a customer collection class:

public class Customercollection: Collection<Customer>
{

  public Collection<Customer> FindCustomers()
   {
     //calls DAL and gets a Collection of customers
     Collection<Customer> customers = DAL.GetCustomers();

     return customers;
   }
}

Now, an alternative version of this method can be:

public class Customercollection: Collection<Customer>
{

  public Collection<Customer> FindCustomers()
   {
     //calls DAL and gets a Collection of customers
     Collection<Customer> customers = DAL.GetCustomers();
     foreach(Customer c in customers)
     this.Add(c);
     return this;
   }
}

I would like to discuss Which one is the better approach? And is there any other approach better than two above two?

Answer 1

I would propose a third approach:

Edit: I have updated this code example to reflect the OP's comments below.

public class Customer
{
    public static ICollection<Customer> FindCustomers()
    {
     Collection<Customer> customers = new Collection<Customer>();

     foreach (CustomerDTO dto in DAL.GetCustomers())
      customers.Add(new Customer(dto));  // Do what you need to to create the customer

     return customers;
    }
}

Most of the time a custom collection is not needed - I am assuming that this is one of those cases. Also you can add utility methods onto the type (in this case, the Customer type) as this aids developer discovery of these methods. (This point is more a matter of taste - since this is a static method you are free to put it in whatever type you wish CustomerUtility or CustomerHelper for example).

My final suggestion is to return an interface type from FindCustomers() to give you greater flexibility in the future for implementation changes. Obviously DAL.GetCustomers() would have to return some type that implemented IList<T> as well but then any API method (especially in a different tier like a data layer) should be returning interface types as well.

Answer 2

If you really want those methods on the CustomerCollection class then I would suggest

public static ICollection<Customer> GetAllCustomers()

or

public void FillWithAllCustomers()

However, I'd argue that your CustomerCollection class is redundant and consumers can go directly to the DAL if they want to get a collection of customer objects.

Answer 3

In my opinion both of them are a bit weird and confusing. When you extend the Collection class you kind of imply that your class IS a collection - so that it contains the data. I think when you make this method static in the first case it will make the most sense:

public class Customercollection: Collection<Customer>
{

  public static Collection<Customer> FindCustomers()
   {
     //calls DAL and gets a Collection of customers
     Collection<Customer> customers = DAL.GetCustomers();

     return customers;
   }
}

Answer 4

Yet another way would be:

public class Customercollection: Collection<Customer>
{
}

public class Customer
{
    public static CustomerCollection FindCustomers()
    {
        return DAL.GetCustomers();
    }
}

Answer 5

You extend Collection in your first example but you never use Customercollection to store any customers. Instead you are returning a Collection<Customer>. My suggestion is:

public static class Customers
{

  public static Collection<Customer> FindCustomers()
   {
     //calls DAL and gets a Collection of customers
     Collection<Customer> customers = DAL.GetCustomers();

     return customers;
   }
}

Used like this:

Collection<Customer> customers = Customers.FindCustomers();

Your second example is also a little weird, because if you call FindCustomers twice you will get each customer two times in the list.

Answer 6

I would put the FindCustomers method in the DAL class or create a Finder class to hold the method. Chances are you will need more finder methods later.

Answer 7

What if you do something like this:

public class CustomerCollection: Collection<Customer>
{
  public CustomerCollection: : base(new List<Customer>())
   {}

  public static IList<Customer> FindCustomers()
  {
   //return them from DAL
  }
}

Using List in your constructor will let you use useful's methods in List in your class without the need to write your own implementation.

Answer 8

I'd add this extension method to Andrew's suggestion:

public static Collection<T> ToCollection(this IEnumerable<T> seq) {
    return new Collection<T>(seq.ToList());
}

and use it like this:

public static Collection<Customer> FindCustomers() { 
    return DAL.GetCustomers().Select(dto => new Customer(dto)).ToCollection();
}

Or if you go with Andrew's advice about returning interface types,

public static IList<Customer> FindCustomers() { // or ICollection
    return DAL.GetCustomers().Select(dto => new Customer(dto)).ToList();
}