Readonlycollection or Ienumerable for Exposing Member Collections

ReadOnlyCollection or IEnumerable for exposing member collections?

More modern solution

Unless you need the internal collection to be mutable, you could use the System.Collections.Immutable package, change your field type to be an immutable collection, and then expose that directly - assuming Foo itself is immutable, of course.

Updated answer to address the question more directly

Is there any reason to expose an internal collection as a ReadOnlyCollection rather than an IEnumerable if the calling code only iterates over the collection?

It depends on how much you trust the calling code. If you're in complete control over everything that will ever call this member and you guarantee that no code will ever use:

ICollection<Foo> evil = (ICollection<Foo>) bar.Foos;
evil.Add(...);

then sure, no harm will be done if you just return the collection directly. I generally try to be a bit more paranoid than that though.

Likewise, as you say: if you only need IEnumerable<T>, then why tie yourself to anything stronger?

Original answer

If you're using .NET 3.5, you can avoid making a copy and avoid the simple cast by using a simple call to Skip:

public IEnumerable<Foo> Foos {
get { return foos.Skip(0); }
}

(There are plenty of other options for wrapping trivially - the nice thing about Skip over Select/Where is that there's no delegate to execute pointlessly for each iteration.)

If you're not using .NET 3.5 you can write a very simple wrapper to do the same thing:

public static IEnumerable<T> Wrapper<T>(IEnumerable<T> source)
{
foreach (T element in source)
{
yield return element;
}
}

IEnumerable vs IReadonlyCollection vs ReadonlyCollection for exposing a list member

Talking about class libraries, I think IReadOnly* is really useful, and I think you're doing it right :)

It's all about immutable collection... Before there were just immutables and to enlarge arrays was a huge task, so .net decided to include in the framework something different, mutable collection, that implement the ugly stuff for you, but IMHO they didn't give you a proper direction for immutable that are extremely useful, especially in a high concurrency scenario where sharing mutable stuff is always a PITA.

If you check other today languages, such as objective-c, you will see that in fact the rules are completely inverted! They quite always exchange immutable collection between different classes, in other words the interface expose just immutable, and internally they use mutable collection (yes, they have it of course), instead they expose proper methods if they want let the outsiders change the collection (if the class is a stateful class).

So this little experience that I've got with other languages pushes me to think that .net list are so powerful, but the immutable collection were there for some reason :)

In this case is not a matter of helping the caller of an interface, to avoid him to change all the code if you're changing internal implementation, like it is with IList vs List, but with IReadOnly* you're protecting yourself, your class, to being used in not a proper way, to avoid useless protection code, code that sometimes you couldn't also write (in the past in some piece of code I had to return a clone of the complete list to avoid this problem).

ReadOnlyCollection IEnumerable

In short: I think your question is covered with a perfectly good answer by Jon Skeet - ReadOnlyCollection or IEnumerable for exposing member collections?

In addition:
You can just emulate AsReadOnly():

public ReadOnlyCollection<Abc> List
{
get { return new ReadOnlyCollection(list); }
}

UPDATE:

This doesn't create a copy of list. ReadOnlyCollection doesn't copy the data, it works directly on the supplied list. See documentation:

A collection that is read-only is simply a collection with a wrapper that prevents modifying the collection; therefore, if changes are made to the underlying collection, the read-only collection reflects those changes.

This constructor is an O(1) operation.

Expose IList T or ReadOnlyCollection T for Read-Only Property?

You can also use IEnumerable<T> - it will make your code as loosely coupled as possible in this case. It also provides random access with the ElementAt<T> method, which will work with O(1) complexity if the actual type implements IList<T>:

If the type of source implements IList<T>, that implementation is used to obtain the element at the specified index. Otherwise, this method obtains the specified element.

Exposing IList<T> or ReadOnlyCollection<T> could be useful if you really wanted to stress the fact that random access with constant-time complexity is available. Deciding which one really depends on a particular scenario.

Is an IEnumerable now more water tight than an IReadOnlyList?

I'd say just do:

public IReadOnlyList<Order> Orders => _orders.AsReadOnly();

It wraps your List into read-only wrapper, so "evil" code like this:

ICollection<Order> evil = (ICollection<Order>)customer.Orders;
evil.Add(order3);

Will just throw an exception that colleciton is read-only. Using IEnumerable has both performance and usability implications for this scenario, without providing much benefits (at all I'd say).

For example I guess it's a common operation to get a number of customer orders. If you return IEnumerable - that operation is both less perfomant compared to IReadOnlyList and less convenient for the caller.

Less perfomant is because with IEnumerable the only way to get count of orders is Count() LINQ method, and this method, in your implementation with yield return, will have to go through the whole collection and count how much items are there. With IReadOnlyList there is Count property which just contains that information.

Less convenient, because multiple enumerations of single enumerable is discouraged and VS will warn user of your api about that. Suppose user of your api has code like this:

IEnumerable<Order> orders = customer2.Orders;
// just an example, there is actually no need to check for count
// before enumerating
if (orders.Count() > 0) {
foreach (var order in orders) {

}
}

On foreach line, VS will warn him about "Possible multiple enumerations of IEnumerable". Only reasonable thing caller can do with IEnumerable is, well, enumerate it. Once. So to avoid this warning, he will have to write some ugly code, or do var orders = customer.Orders.ToList(), creating unnecessary copy of your list and working with that.

There is no reason to limit user of your api like that, unless enumerating just once is the only correct way of using that api.

What ReadOnlyCollection type should methods return?

My original question was not super well-defined; there are a number of considerations that should be taken into account:

  • Does the user only need to enumerate through the code? If so, IEnumerable is probably enough.
  • Should the user know that the collection is intended to be read-only? If so, IReadOnlyCollection is probably fine.
  • Do you want to prevent the user from casting your read-only collection to a mutable type? If so, use an immutable collection from System.Collections.Immutable.

Some useful answers are in these questions:

  • ReadOnlyCollection or IEnumerable for exposing member collections?
  • IEnumerable vs IReadonlyCollection vs ReadonlyCollection for exposing a list member

Best way to expose ReadOnlyCollection of ReadOnlyCollection while still being able to modify it inside class

ReadOnlyCollection cannot do this, at least not by itself. If you add a new list to _List, any user of RList will need to get a chance to see a new ReadOnlyCollection linked to that new list. ReadOnlyCollection simply doesn't support that. It is only a read-only view of the original items, and the original items are modifiable.

You can implement a custom class which behaves similarly to ReadOnlyCollection, except that it doesn't return the original items, but wraps them. ReadOnlyCollection's implementation is trivial: pretty much all of ReadOnlyCollection's methods can be implemented in a single line: they either throw an unconditional exception (e.g. IList.Add), return a constant value (e.g. IList.IsReadOnly), or forward to the proxied container (e.g. Count).

The change you would need to make to adapt it to your use involve the various methods that deal directly with items. Note that that is more than simply the this[int] indexer: you would also need to make sure two proxies of an identical list compare as equal, and provide a private/internal method to obtain the original list from a proxy, to make methods such as Contains work. You would also need to create a custom enumerator type to make sure GetEnumerator doesn't start returning the original items. You would need to create a custom CopyTo implementation. But even keeping those things in mind, it should be quite easy to do.


That said, perhaps there is a somewhat less clean but easier approach: if you create a custom class MyReadOnlyCollection<T> derived from ReadOnlyCollection<T>, you can provide an internal Items member (which forwards to ReadOnlyCollection<T>'s protected Items member).

You can then create a MyReadOnlyCollection<MyReadOnlyCollection<MyClass>>, and call RList[0].Items.Add from your own assembly, without worrying that external users will be able to call that too.


As noted, if the outer list actually never changes, it can be simpler: in that case, you can simply do something like

public ReadOnlyCollection<ReadOnlyCollection<MyType>> RList {
get {
return _List.ConvertAll(list => list.AsReadOnly()).AsReadOnly();
}
}

which doesn't bother to monitor _List for changes.

How to expose read-only collection another type of private collection?

The overhead is not so significant if you consider that ReadOnlyCollection is a wrapper around the list (i.e. it doesn't create a copy of all the items).

In other words, if your class looked like this:

class AnotherClass
{
private ReadOnlyCollection<string> _readonlyList;
public ReadOnlyCollection<string> ReadonlyList
{
get { return _readonlyList; }
}

private List<string> _list;
public List<string> List
{
get { return _list; }
}

public AnotherClass()
{
_list = new List<string>();
_readonlyList = new ReadOnlyCollection<string>(_list);
}
}

Then any change to the List property is reflected in the ReadOnlyList property:

class Program
{
static void Main(string[] args)
{
AnotherClass c = new AnotherClass();

c.List.Add("aaa");
Console.WriteLine(c.ReadonlyList[0]); // prints "aaa"

c.List.Add("bbb");
Console.WriteLine(c.ReadonlyList[1]); // prints "bbb"

Console.Read();
}
}

You may have issues with thread safety, but exposing IEnumerable is even worse for that matter.

Personally, I use a custom IIndexable<T> interface with several handy wrapper classes and extension method that I use all over my code for immutable lists. It allows random access to list elements, and does not expose any methods for modification:

public interface IIndexable<T> : IEnumerable<T>
{
T this[int index] { get; }
int Length { get; }
}

It also allows neat LINQ-like extension methods like Skip, Take and similar, which have better performance compared to LINQ due to the indexing capability.

In that case, you can implement a projection like this:

public class ProjectionIndexable<Tsrc, Ttarget> : IIndexable<Ttarget>
{
public ProjectionIndexable
(IIndexable<Tsrc> src, Func<Tsrc, Ttarget> projection)
{
_src = src;
_projection = projection;
}

#region IIndexable<Ttarget> Members

public Ttarget this[int index]
{
get { return _projection(_src[index]); }
}

public int Length
{
get { return _src.Length; }
}

#endregion

#region IEnumerable<Ttarget> Members

// create your own enumerator here

#endregion
}

And use it like this:

class AnotherClass
{
private IIndexable<string> _readonlyList;
public IIndexable<string> ReadonlyList
{
get { return _readonlyList; }
}

private List<SomeClass> _list;
public List<SomeClass> List
{
get { return _list; }
}

public AnotherClass()
{
_list = new List<SomeClass>();
_readonlyList = new ProjectionIndexable<SomeClass, string>
(_list.AsIndexable(), c => c.Age);
}
}

[Edit]

In the meantime, I posted an article describing such a collection on CodeProject. I saw you've implemented it yourself already, but you can check it out nevertheless and reuse parts of the code where you see fit.

Using IReadOnlyCollection T instead of IEnumerable T for parameters to avoid possible multiple enumeration

Having thought about this further, I have come to the conclusion, based on the article I mentioned in my Question, that it is indeed OK to use IReadOnlyCollection<T> as a parameter, but only in functions where it will definitely be enumerated. If enumeration is conditional based on other parameters, object state, or workflow, then it should still be passed in as IEnumerable<T> so that lazy evaluation is semantically ensured.



Related Topics



Leave a reply



Submit