Dave
Dave

Reputation: 8461

When to use Collection<T> or IEnumerable<T>

I've seen similar questions but I'm still lost in regards to the return type of Public, Internal and Private methods. When should my return type by Collection and when should it be IEnumerable

The MSDN guide says

X DO NOT use weakly typed collections in public APIs.

The type of all return values and parameters representing collection items should be the exact item type, not any of its base types (this applies only to public members of the collection).

This has totally confused me and implies that everything (public) should be Collection<T> or ReadOnlyCollection<T>. My confusion comes in when can we use List<T>, for example

public Collection<string> DoThisThing(int i)
{
     Collection<string> col = new Collection<string>();
     List<string> myList = null;
     if (i > 0)
         myList = GetListOfString();

     //col.AddRange(...); Can't AddRange as no method
     //return myList; Can't do this as it expects return type of Collection<string>

     //So, to ensure my return is Collection, I have to convert
     foreach(var item in myList)
     {
         col.Add(item);
     }

     return col;
}

In the above example, it's perfectly valid that List is a better choice, so, if I want to use AddRange, I could either

  1. Hope to have control of API so I can change the return type of GetListOfString to Collection (not always possible)
  2. Add an extension method to Control to support AddRange
  3. Type it out every time, as I do above

However, if I do option 1, then I don't see when I'd ever actually use ToList() (other than when I have to if an external API returns List) as all my return types would be either Collection<T>, ReadOnlyCollection<T>

Using option 2 seems more sensible but, now I'm lost as it feels as if I'm forcing it to do something it isn't intended to do.

Option 3 is repetitive

Whist I appreciate it will always depend on the situation, as a general rule, is the following accurate

Public - Should be very precise, for example IEnumerable<string> (note, not T), but not Collection<T>/Collection<string>

Internal and private - Should be more agnostic, so Collection<T> is better (and live with not really using List or if I do have to use, I will have to cast it into Collection<T>

Upvotes: 0

Views: 434

Answers (2)

pid
pid

Reputation: 11597

EDIT: added code at the end


You are confusing several different layers of the guide.

In your internal code you are free to do what you like, albeit there are guidelines for how to write code that is clean, readable and easy to maitain. Still, those are implementation details and they are tucked away behind your public methods, so you can do what you want (you may shoot in your own foot).

Artifacts you write to be used by other developers are one of these:

  • libraries;
  • frameworks;
  • modules of "some kind".

Now, in these cases you must decide:

  • the boundaries of your artifact;
  • how those developers use your artifact.

The boundaries are well defined by an assembly, if you choose to have one.

Sometimes namespaces may be considered "internal boundaries" and the consumer is yourself, in the future. Organizing your code into internal boundaries is a good practice but consider that classes already define boundaries so don't overdo it. Group 10-20 classes in a namespace by cohesion and keep those groups apart without interdependencies by low coupling. You cannot really understand what I mean here when I say bridge... but come back in a year or so and read this answer again :)

How to use the assembly is defined in these places:

  • manifest (generated automatically by the VS packaging);
  • documentation (which should accompany your assembly);
  • source code (if you expose it);
  • public constructs (enums, interfaces, classes, namespaces, package dependencies);
  • public and protected methods.

Now with all those tools you have a way to design that thing called an API. There is plenty of room to maneuver and make mistakes. Because of this, the good people at M$ decided to define a uniform and consistent way of designing the default APIs in the default assemblies and namespaces (such as System.*).

What you have touched in your question is a guideline to API designers (which you might or might not be). All they say is:

if you are designing an API such as a non-internal method of a public class in an accessible assembly, than do us the favor and return a tightly defined value, as concrete as possible.

So, if it is a List<T> don't return any of the ancestors such as IEnumerable<T> or whatever. This allows the consumers (other developers) of your assembly to leverage concrete properties of your return value.

On the other hand you should accept as general values as possible. If you just use a method of IEnumerable<T> in your method you should require just that:

public void MyMethod<T>(IEnumerable<T> e);

This signature of your method allows consumers to pass in many different concrete collections. The only thing you require is that it is (derives from) an IEnumerable<T>.

These design decisions are also bound to many important OO principles of SOLID. They postulate that:

  • you should depend on abstractions => so expose interfaces if possible and keep your API abstract;
  • you should hide implementation details so that changes in your assembly don't propagate to consumer code;
  • you should avoid changing the API by allowing extension mechanism.

To be more precise I take your code and try to be as concrete as possible:

public List<string> DoThisThing(int i) // use list to return a more concrete type
{
    List<string> list = new List<string>(); // you really want a *LIST* so why be abstract in your own code innards?
    // List<string> myList = null; <-- superfluous
    if (i > 0)
    {
        list = GetListOfString();
    } // ALWAYS (!) use code blocks, see the book "Clean Code"

    // this problem is solved: in your internal code don't be abstract if you don't need it
    //col.AddRange(...); Can't AddRange as no method

    return list; // Can't do this as it expects return type of Collection<string>

    // you're done...

    // So, to ensure my return is Collection, I have to convert
    // foreach(var item in myList)
    // {
    //     col.Add(item);
    // }

    // return col;
}

Some help about the theory behind all this: The SOLID principles.

What you really want to understand is how and why you need abstractions. When you really grasp that, you will also understand when you DO NOT need them and so decide in a reasoned manner when to use IEnumerable<T> or List<string> without negative repercussions on the rest of your code. The most important principles of SOLID are in this case OCP and DIP.

Upvotes: 3

O. R. Mapper
O. R. Mapper

Reputation: 20732

Your question seems to be thinking into the "wrong" direction, starting from the initial MSDN quotation:

X DO NOT use weakly typed collections in public APIs.

The type of all return values and parameters representing collection items should be the exact item type, not any of its base types (this applies only to public members of the collection).

This refers to the items of collections, not the collection types.

This guideline does not refer to the type of the collection itself at all. Whether you return an enumerable, a collection, a list, or any other collection type, entirely depends on how you intend your collection to behave and what guarantees you want to make about the set of items you store.

The guideline you cited, on the other hand, indicates that you should strongly type the items stored in your collection. That is, if you have a list of string values, do not declare the collection item type as object (e.g. by typing the collection to IEnumerable<object> or IList<object>), but do use string as the item type (e.g. IEnumerable<string>, ICollection<string>, IList<string>).

Upvotes: 3

Related Questions