Reputation: 8461
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
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
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:
Now, in these cases you must decide:
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:
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:
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
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