Search code examples
c#.netlinq

IReadOnlyCollection vs ReadOnlyCollection


There are a couple of similar questions already on SO, but none of the ones I found really touches on this particular topic, so here it goes...

My understanding is that one should always attempt to return an interface over a concrete class. Won't go into the reasons behind it, plenty of things on SO about that already.

However in the case of an IReadOnlyCollection vs a ReadOnlyCollection I'm not sure if that rule should be followed.

An IReadOnlyCollection can be easily cast into a List, which... well... breaks the ReadOnly aspect that the contract promises.

ReadOnlyCollection however cannot be cast into a List, but it means returning a concrete class.

In the long run, does it actually matter? It seems to me like in most cases a ReadOnly*/IReadOnly* object is only returned returned by either a method or a read-only property.

So even if the user decides to cast it to something else (in the case of a IReadOnly* object) or use LINQ to create a collection of some kind out of it (in the case of ReadOnly* object), there's really no way that the class exposing the ReadOnly*/IReadOnly* object is going to accept that back.

So what's the recommendation here, return an IReadOnly* interface or a concrete ReadOnly* class instance?


Solution

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