Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

ICollection<T> doesn't have AddRange but List<T> does, is Casting Bad

Tags:

c#

So in my class I have this private readonly member ICollection<IMusicItem> playlist. I would prefer to use the interface ICollection<T>.

I would like to use the List<T>.AddRange(IEnumerable<T> items). In my method would it be dangerous to cast the ICollection to a List<T> even if I instantiate the ICollection<T> as a new List<T>() in the constructor.

Is this bad practice, is there a better way of doing this?

Or is it just better to have a List<T>

like image 740
Callum Linington Avatar asked Dec 19 '22 02:12

Callum Linington


2 Answers

It's bad practice, because it breaks encapsulation. Using an interface is good, but it's pointless if you have to cast the object back to a concrete type. Act as if you didn't know the concrete type, or it's a future bug waiting to happen if you decide to switch to another type later on.

Use an extension method instead:

public static void AddRange<T>(this ICollection<T> collection, IEnumerable<T> items)
{
    foreach (var item in items)
        collection.Add(item);
}

Note: It's better to expose interfaces in a public API so you're free to change the implementing object later, but it's a matter of style whether to do it on private fields. You may as well use the concrete class.

like image 109
Lucas Trzesniewski Avatar answered Feb 15 '23 22:02

Lucas Trzesniewski


It's not dangerous (when done right), just pointless.

if (playlist is IList<IMusicItem>)
{
  (playList as IList<IMusicItem>).AddRange(items);
} 
else
{
   // still need a foreach here
}

The real issue is "I would prefer to use the interface ICollection<T>".

Why exactly? Your question suggests it always is a List, so why not expose it as such?

Exposing it as a more general ICollection<> only makes sense when other implementations than List might exist, and then the casting is useless.

like image 35
Henk Holterman Avatar answered Feb 16 '23 00:02

Henk Holterman