By Marcel-Is-Hier


2014-07-22 06:23:30 8 Comments

I have spent quite a few hours pondering the subject of exposing list members. In a similar question to mine, John 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.

Thanks in advance for your input.

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 garuantee immutability. It 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 : 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.

Thank you all once again.

3 comments

@Carsten Führmann 2015-09-18 11:10:59

One important aspect seems to be missing from the answers so far:

When an IEnumerable<T> is returned to the caller, they must consider the possibility that the returned object is a "lazy stream", e.g. a collection built with "yield return". That is, the performance penalty for producing the elements of the IEnumerable<T> may have to be paid by the caller, for each use of the IEnumerable. (The productivity tool "Resharper" actually points this out as a code smell.)

By contrast, an IReadOnlyCollection<T> signals to the caller that there will be no lazy evaluation. (The Count property, as opposed to the Count extension method of IEnumerable<T> (which is inherited by IReadOnlyCollection<T> so it has the method as well), signals non-lazyness. And so does the fact that there seem to be no lazy implementations of IReadOnlyCollection.)

This is also valid for input parameters, as requesting an IReadOnlyCollection<T> instead of IEnumerable<T> signals that the method needs to iterate several times over the collection. Sure the method could create its own list from the IEnumerable<T> and iterate over that, but as the caller may already have a loaded collection at hand it would make sense to take advantage of it whenever possible. If the caller only has an IEnumerable<T> at hand, he only needs to add .ToArray() or .ToList() to the parameter.

What IReadOnlyCollection does not do is prevent the caller to cast to some other collection type. For such protection, one would have to use the class ReadOnlyCollection<T>.

In summary, the only thing IReadOnlyCollection<T> does relative to IEnumerable<T> is add a Count property and thus signal that no lazyness is involved.

@supercat 2015-10-29 21:37:31

Unfortunately, nothing in IReadOnly* indicates whether an implementation claims to be immutable, or even read-only for that matter (a read-only object may be safely shared with code which must not be allowed to change an object, but would be allowed to see future changes to an object made by someone else who has a "special" reference related to it; many classes which implement IReadOnly* do not meet that criterion).

@Richiban 2016-03-29 14:11:46

@supercat Correct, it's the unfortunate situation we're in due to the fact that the base interfaces (IList, ISet etc) are from a time when immutable collections weren't the done thing... So "IReadOnly*" actually just means "no mutation methods are exposed here". You can see this from the fact that List(T) implements IReadOnlyList(T) even though it is clearly not readonly! But what else would you call that interface?

@supercat 2016-03-29 15:01:05

@Richiban: I would call it IReadableList<T>. I would reserve IReadOnlyXX<t> for things which guarantee that no legitimate implementations will expose means of mutation (implying that they may be safely shared with things that must not be allowed to modify the underlying collection) but would not promise that the underlying data store won't be modified in other ways.

@Richiban 2016-03-29 16:41:48

@supercat Yeah, IReadableList isn't bad... but IReadOnly* doesn't make any sense at all because an interface only guarantees what something can do, not what it cannot do.

@supercat 2016-03-29 16:56:00

@Richiban: The purpose of an interface is to define what recipients of an object can do with it. If an object legitimately implements IReadOnly* as I would have defined it, that would imply that recipients can safely expose it to outside it without having to wrap it first. Likewise, if an object implements IImmutable*, recipients can safely use a reference to the object as a means of encapsulating its contents, without having to make a defensive copy. In many cases, it may be helpful to have a sealed class which implements one of those interfaces inherit from a class...

@supercat 2016-03-29 16:59:48

...which is functionally identical but doesn't. While some people might balk at the idea of having an "immutable" interface, I would see great advantages in being able to have immutable collection types generate content on demand.

@AnorZaken 2016-04-04 03:12:38

Personally I think ReadOnly interfaces are fundamentally stupid because in 99% of cases the most proper and safe approach is thin value-type wrappers that contain only a single private readonly field of the wrapped type and exposes only the non-mutating methods and properties of the that type. a) Coding this is as easy as coding a similar class + overriding Equals to make a reference comparison on the private field, b) There are no "casting" issues. c) Creating instances is more performant, and d) implicit cast from the wrapped type to the read-only struct is perfectly safe!

@AnorZaken 2016-04-04 03:21:19

...having a mutable type implement a read-only interface is just plain bad and gets us back in that old "but the caller can cast it" pit. After all that is why using the ReadOnly-classes is encouraged. So then some might think "but what if I want to make my own implementation?" Yeah - no problem! Implement a private / internal class that inherits the same mutable interface that is stored in the desired readonly-struct - except you can leave all mutating methods unimplemented since they will never be called 100% guaranteed (unless you do something very wrong - i.e. an internal bug).

@AnorZaken 2016-04-04 03:28:55

What I meant by safe implicit casts is that generally speaking implicit casts is a source of bugs, but in this case you'd be hard pressed to make anything go wrong. Simple example: class MyClass { private int[] myNumbers; ReadOnlyArray<int> MyNumbers { get { return myNumbers; } } } where ReadOnlyArray<T> is a thin generic wrapper struct around an array (with an implicit cast from T[]). Such structs should be in included in .net by default - it would make all kinds of read-only things better imo.

@nawfal 2018-09-19 09:31:33

I too prefer to use IReadOnlyCollection<T> wherever possible to guarantee the least. IEnumerable<T> could be a lazy sequence, and ICollection<T>, though in-memory, has Add/Remove methods.. IReadOnlyCollection<T> has none of the problems - no order, no mutation methods or other guarantees but is loaded in memory (has Count property), brilliant! :)

@user11523568 2019-07-20 18:28:33

@nawfal Unfortunately, ICollection<T> and IReadOnlyCollection<T> both inherit from IEnumerable<T>, but not each other. They both provide a property Count, but here is the catch: LINQ's Count() extension method is optimized for ICollection<T>, not IReadOnlyCollection<T>. This is most unfortunate. github.com/microsoft/referencesource/blob/master/System.Core‌​/…

@Carsten Führmann 2019-07-20 21:27:23

@dfhwze Your remark is troubling news. I checked several of Microsoft's derivatives of IReadOnlyCollection<T>. Fortunately, all of them also implement ICollection or ICollection<T>. Still, Microsoft left a trap for people writing their own derivatives of IReadOnlyCollection<T>.

@user11523568 2019-07-20 21:39:35

@CarstenFührmann The only reason I can think of why they designed it this way, is that whoever implements IReadOnly.. should allow eager enumeration, rendering Count() almost as fast as Count. But I agree, they should have included both interfaces for optimization. Or why not Make ICollection<T> inherit IReadOnlyCollection<T> instead, C# team?

@Michael Denny 2014-07-22 07:26:10

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).

@Marcel-Is-Hier 2014-07-22 07:44:46

Very nice answer. The whole idea comes down to one word. Encapsulation. I am always paranoid when I see people exposing a list as a property with a get and set. So dangerous and guaranteed to be misused. Very interesting how other languages handle immutables

@Michael Denny 2014-07-22 08:18:43

Thanks! Just for record the mutable/immutable that I was speaking about in objective-c are respectively NSArray and NSMutableArray, the name also speak itself :)

@Dmitry Bychenko 2014-07-22 06:55:29

It seems that you can just return an appropriate interface:

...
    private readonly List<WorkItem> workItems = new List<WorkItem>();

    // Usually, there's no need the property to be virtual 
    public virtual IReadOnlyList<WorkItem> WorkItems {
      get {
        return workItems;
      }
    }
...

Since workItems field is in fact List<T> so the natural idea IMHO is to expose the most wide interface which is IReadOnlyList<T> in the case

@Marcel-Is-Hier 2014-07-22 07:02:32

I never knew of IReadonlyList. I copied this straight from my domain and changed it a bit. NHibernate needs these to be virtual though. I'll change that quickly thanks. I now need to dig a bit on that interface. Nice!!!!

@Rawling 2014-07-22 07:22:50

Can't you cast this back into a List<> like mentioned in the question for IEnumerable<>s?

@Justin J Stark 2014-10-23 15:17:35

@Rawling: Since ReadOnlyCollection inherits from List, casting it to a list will leave it as a ReadOnlyCollection. Doing this and then attempting to add an element will cause a NotSupportedException: Collection is read-only.

@Justin J Stark 2014-10-23 15:32:21

Another alternative is to return an IEnumerable using List.Skip(0) rather than casting to prevent the IEnumerable from being explicitly cast back to a List. See stackoverflow.com/a/491591/1025728

@Rawling 2014-10-23 16:02:32

@JustinJStark ReadOnlyCollection is not involved here. There is a List<T> hidden behind an IReadOnlyList<T> interface and, as the question and your second comment note, you can just cast that back to a List<T>.

@JG in SD 2016-05-17 16:59:04

Since the returned value can still be cast to List<T> and modified, I'd add call to AsReadOnly and return that. This will ensure the internal value isn't modified. get { return workItems.AsReadOnly(); }

Related Questions

Sponsored Content

28 Answered Questions

[SOLVED] How do I check if a list is empty?

  • 2008-09-10 06:20:11
  • Ray Vega
  • 2516064 View
  • 3235 Score
  • 28 Answer
  • Tags:   python list

45 Answered Questions

[SOLVED] How to make a flat list out of list of lists?

26 Answered Questions

[SOLVED] How do I concatenate two lists in Python?

29 Answered Questions

[SOLVED] Finding the index of an item given a list containing it in Python

  • 2008-10-07 01:39:38
  • Eugene M
  • 3501005 View
  • 2873 Score
  • 29 Answer
  • Tags:   python list indexing

20 Answered Questions

26 Answered Questions

[SOLVED] Why not inherit from List<T>?

15 Answered Questions

[SOLVED] How to clone or copy a list?

14 Answered Questions

[SOLVED] Returning IEnumerable<T> vs. IQueryable<T>

10 Answered Questions

[SOLVED] IEnumerable vs List - What to Use? How do they work?

12 Answered Questions

Sponsored Content