I have spent quite a few hours pondering the subject of exposing list members. In a similar question to mine, Jon Skeet gave an excellent answer. Please feel free to have a look.
ReadOnlyCollection or IEnumerable for exposing member collections?
I am usually quite paranoid to exposing lists, especially if you are developing an API.
I have always used IEnumerable for exposing lists, as it is quite safe, and it gives that much flexibility. Let me use an example here:
public class Activity
{
private readonly IList<WorkItem> workItems = new List<WorkItem>();
public string Name { get; set; }
public IEnumerable<WorkItem> WorkItems
{
get
{
return this.workItems;
}
}
public void AddWorkItem(WorkItem workItem)
{
this.workItems.Add(workItem);
}
}
Anyone who codes against an IEnumerable is quite safe here. If I later decide to use an ordered list or something, none of their code breaks and it is still nice. The downside of this is IEnumerable can be cast back to a list outside of this class.
For this reason, a lot of developers use ReadOnlyCollection for exposing a member. This is quite safe since it can never be cast back to a list. For me I prefer IEnumerable since it provides more flexibility, should I ever want to implement something different than a list.
I have come up with a new idea I like better. Using IReadOnlyCollection:
public class Activity
{
private readonly IList<WorkItem> workItems = new List<WorkItem>();
public string Name { get; set; }
public IReadOnlyCollection<WorkItem> WorkItems
{
get
{
return new ReadOnlyCollection<WorkItem>(this.workItems);
}
}
public void AddWorkItem(WorkItem workItem)
{
this.workItems.Add(workItem);
}
}
I feel this retains some of the flexibility of IEnumerable and is encapsulated quite nicely.
I posted this question to get some input on my idea. Do you prefer this solution to IEnumerable? Do you think it is better to use a concrete return value of ReadOnlyCollection? This is quite a debate and I want to try and see what are the advantages/disadvantages that we all can come up with.
EDIT
First of all thank you all for contributing so much to the discussion here. I have certainly learned a ton from each and every one and would like to thank you sincerely.
I am adding some extra scenarios and info.
There are some common pitfalls with IReadOnlyCollection and IEnumerable.
Consider the example below:
public IReadOnlyCollection<WorkItem> WorkItems
{
get
{
return this.workItems;
}
}
The above example can be casted back to a list and mutated, even though the interface is readonly. The interface, despite it's namesake does not guarantee immutability. It is up to you to provide an immutable solution, therefore you should return a new ReadOnlyCollection. By creating a new list (a copy essentially), the state of your object is safe and sound.
Richiban says it best in his comment: a interface only guarantees what something can do, not what it cannot do.
See below for an example:
public IEnumerable<WorkItem> WorkItems
{
get
{
return new List<WorkItem>(this.workItems);
}
}
The above can be casted and mutated, but your object is still immutable.
Another outside the box statement would be collection classes. Consider the following:
public class Bar : IEnumerable<string>
{
private List<string> foo;
public Bar()
{
this.foo = new List<string> { "123", "456" };
}
public IEnumerator<string> GetEnumerator()
{
return this.foo.GetEnumerator();
}
IEnumerator IEnumerable.GetEnumerator()
{
return this.GetEnumerator();
}
}
The class above can have methods for mutating foo the way you want it to be, but your object can never be casted to a list of any sort and mutated.
Carsten Führmann makes a fantastic point about yield return statements in IEnumerables.
