CODEWITHSUNDEEP
c#.netlinq
cogumel0
cogumel0

Reputation: 2671

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?

Upvotes: 15

Views: 9892

Answers (3)

Neo
Neo

Reputation: 4497

Microsoft's guidelines here state:

✓ DO use ReadOnlyCollection<T>, a subclass of ReadOnlyCollection<T>, or in rare cases IEnumerable<T> for properties or return values representing read-only collections.

So, basically, you should return ReadOnlyCollection<T>. It specifies interface IEnumerable in other cases, so if it intended interface IReadOnlyCollection<T>, it would have stated so.

Upvotes: 3

CodeCaster
CodeCaster

Reputation: 151594

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.

Upvotes: 10

Magnus
Magnus

Reputation: 46929

IReadOnlyCollection<T> can only be cast to List<T> if the underlying object is of that type. ReadOnlyCollection<T> for example also implements IReadOnlyCollection<T>.

So my recommendation, return IReadOnlyCollection<T> and if you are worried that caller would wrongly cast it to something it shouldn't, make sure the underlying type is ReadOnlyCollection<T>

public IReadOnlyCollection<User> GetUsers()
{
   return new ReadOnlyCollection<User>();
}


But returning IReadOnlyCollection<T> should be enough for caller of function to understand it is supposed to be read only.
Note that you can never completely secure your code with a ReadOnlyCollection<T>, caller can still use reflection to access the internal list and manipulate it.
The only option in that case would be to create a copy if the list and return that.

Upvotes: 14

Related Questions