Ienumerable VS Ireadonlycollection VS Readonlycollection for Exposing a List Member

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 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;
}
}

IReadOnlyCollection vs ReadOnlyCollection

You definitely should attempt to make your public methods return interfaces.

If you're afraid that your class's callers are going to cast and modify your internal structures, such as in this example, where a class's internal queue shouldn't be touched from the outside:

public class QueueThing
{
private List<QueueItem> _cantTouchThis;

public IReadOnlyCollection<QueueItem> GetQueue()
{
return _cantTouchThis;
}
}

Then you could use AsReadOnly() to return a new ReadOnlyList<T>, sourced from the private List<T>:

public class QueueThing
{
private List<QueueItem> _cantTouchThis;

public IReadOnlyCollection<QueueItem> GetQueue()
{
return _cantTouchThis.AsReadOnly();
}
}

Now the caller can cast the returned value all they want, they won't be able to modify the _cantTouchThis member (except of course when they're going to use reflection, but then all bets are off anyway).

Given many types can implement an interface, a user of such a method should definitely not assume that it's safe to cast the return value of the method to any concrete type.

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.

Using IReadOnlyCollectionT instead of IEnumerableT 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.

Best practice for parameter: IEnumerable vs. IList vs. IReadOnlyCollection

You can take an IEnumerable<T> in the method, and use a CachedEnumerable similar to the one here to wrap it.

This class wraps an IEnumerable<T> and makes sure that it is only enumerated once. If you try to enumerate it again, it yield items from the cache.

Please note that such wrapper does not read all items from the wrapped enumerable immediately. It only enumerates individual items from the wrapped enumerable as you enumerate individual items from the wrapper, and it caches the individual items along the way.

This means that if you call Any on the wrapper, only a single item will be enumerated from the wrapped enumerable, and then such item will be cached.

If you then use the enumerable again, it will first yield the first item from the cache, and then continue enumerating the original enumerator from where it left.

You can do something like this to use it:

public IEnumerable<Data> RemoveHandledForDate(IEnumerable<Data> data, DateTime dateTime)
{
var dataWrapper = new CachedEnumerable(data);
...
}

Notice here that the method itself is wrapping the parameter data. This way, you don't force consumers of your method to do anything.

Expose IListT or ReadOnlyCollectionT 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.

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.

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


Related Topics



Leave a reply



Submit