How can I safely return List<T> from method/property declared as IEnumerable<T>?

Question

Let's say I have IEnumerable<int> property backed with List<int> field, so I can modify the collection from within the class, but it's publicly exposed as read-only.

public class Foo
{
    private List<int> _bar = new List<int>();

    public IEnumerable<int> Bar
    {
        get { return _bar; }
    }
}

But with code like that you can easily cast object retrieved from the property back to List<int> and modify it:

var foo = new Foo();
var bar = (List<int>)foo.Bar;
bar.Add(10);

Question is: what is the best (best readable, easiest to write, without performance loss) way to avoid that?

I can come up with at least 4 solutions, but non of them is perfect:

1. foreach and yield return:

   public IEnumerable<int> Bar
   {
       get
       {
           foreach (var item in _bar)
               yield return item;
       }
   }
   

   - really annoying to write and to read.

2. AsReadOnly():

   public IEnumerable<int> Bar
   {
       get { return _bar.AsReadOnly(); }
   }
   

   + will cause exception when someone tries to modify the returned collection
   + does not create a copy of the entire collection.

3. ToList()

   public IEnumerable<int> Bar
   {
       get { return _bar.ToList(); }
   }
   

   + User can still modify retrieved collection, but it's not the same collection we are modifying from within the class, so we shouldn't care.
   - creates a copy of entire collection what may cause problems when collection is big.

4. Custom wrapper class.

   public static class MyExtensions
   {
       private class MyEnumerable<T> : IEnumerable<T>
       {
           private ICollection<T> _source;
           public MyEnumerable(ICollection<T> source)
           {
               _source = source;
           }
   
           public IEnumerator<T> GetEnumerator()
           {
               return _source.GetEnumerator();
           }
   
           IEnumerator IEnumerable.GetEnumerator()
           {
               return ((IEnumerable)_source).GetEnumerator();
           }
       }
   
       public static IEnumerable<T> AsMyEnumerable<T>(this ICollection<T> source)
       {
           return new MyEnumerable<T>(source);
       }
   }
   

   usage:

   public IEnumerable<int> Bar
   {
       get
       {
           return _bar.AsMyEnumerable();
       }
   }
   

   + don't need to clone the collection
   - when you use it as LINQ queries source some methods won't use ICollection.Count, because you don't expose it.

---

Is there any better way to do that?

Answer 1

Question is: what is the best (best readable, easiest to write, without performance loss) way to avoid that?

In general, I don't try to avoid it. The consumer of my API should use the type I expose, and if they don't, any bugs resulting are their fault, not mine. As such, I don't really care if they cast the data that way - when I change my internal representation, and they get cast exceptions, that's their issue.

That being said, if there is a security concern, I would likely just use AsReadOnly. This is effectively self-documenting, and has no real downsides (apart from a small allocation for the wrapper, as there is no copy of the data, you do get meaningful exceptions on modification, etc). There is no real disadvantage to this vs. making your own custom wrapper, and a custom wrapper means more code to test and maintain.

In general, I personally try to avoid copying without reason. That would eliminate ToList() as an option in general. Using an iterator (your first option) is not as bad, though it does not really provide many advantages over a ReadOnlyCollection<T>.

Answer 2

In general I'm in the don't bother camp. If I give you an IEnumerable and you cast it to something else you are the one who broke the contract and its not my fault if something breaks. If I found myself in a position where I really needed to protect a client from corrupting my state I would create an extension method like:

static IEnumerable<T> AsSafeEnumerable<T>(this IEnumerable<T> obj)
{
    foreach (var item in obj)
    {
        yield return item;
    }
}

and never worry about it again.