2

Say I have:

class foo
{
  private List<T> bar;
  public IEnumerable<T> GetBar();
}

where GetBar() should return that very same List, without exposing internals (i.e. no other class should be able to remove elements from the internal list, reorder it etc.) and avoiding excessive copying. What is the standard way to do this?

J Fabian Meier
  • 33,516
  • 10
  • 64
  • 142
  • 1
    Well obviously you can just return the list itself under the guise of `IEnumerable`, but I presume you mean you want to not allow the caller to explicitly cast to your list? I'm not sure if there is a "standard" way, but you could simply: `foreach (var b in bar) yield return b;`. There are also the Immutable Collections (via NuGet) that will allow you to provide a non-modifiable list that has a bit more support than `IEnumerable`. Personally I'd just return the list as an `IEnumerable` and bugs-be-upon-them if they break your public contract by assuming it's always a `List` underneath. – Adam Houldsworth Dec 13 '13 at 12:58

3 Answers3

8

If you want to return an immutable list, return bar.AsReadOnly().

Typically the member that exposes this readonly wrapper would be of type IList<T>. I would only downcast to IEnumerable<T> if I wanted to indicate to the consumer that the implementation might use lazy enumeration.

Personally I'd make it a property, and return the same readonly wrapper on each invocation:

class Foo
{
    private List<T> bar;
    private IList<T> readonlyBar;

    public Foo()
    {
        bar = ...;
        readonlyBar = bar.AsReadOnly();
    }

    public IList<T> Bar
    {
        get { return readonlyBar; }
    }
}
Joe
  • 122,218
  • 32
  • 205
  • 338
  • Unless `bar` is immutable, `bar.AsReadOnly()` won't be either. Depending upon how `bar` is used, `bar.AsReadOnly` may represent a consistent read-only view of the state encapsulated by `bar`, or it may at some point become detached. Unfortunately, there's no standard way to distinguish immutable objects from read-only views. – supercat Dec 14 '13 at 01:11
  • @supercat, you're right, the property is a readonly wrapper, and the mutability of the underlying list is determined by the implementation of the containing class. Typically this class will either ensure it's immutable, or that it can only be mutated in well-defined and documented ways. – Joe Dec 14 '13 at 09:59
  • I don't know that I'd say "well-defined and well-documented". Many read-only objects are rather vague as to whether they promise to be attached or detached from the source object (e.g. `IDictionary.Keys`). I wish Microsoft would introduce e.g. a `ISnapshot`, so that an `IEnumerable.Snapshot()` extension method could return `ISnapshot>.Snapshot()` for types that implement it, or return ToArray() for other types. Immutable types could implement their enumerable-snapshot method to return themselves. – supercat Dec 14 '13 at 17:41
  • To be sure, anyone could easily define `ISnapshot` themselves, but the biggest benefits to such an interface would come from having it standardized so that classes by different authors could take advantage of each others' `ISnapshot<>` implementations. Incidentally, another couple types I wish Microsoft would offer would be a read-only variation of `ArraySegment` which did not expose the underlying array, but offered a `CopyTo` method which could copy a range of items to an outside array, and an `IExtendedList` with features to copy a range of items to/from another `IList. – supercat Dec 14 '13 at 17:45
  • Presently, the only way to copy a range of items from one `IList` to another is to either do all the items individually, or else copy everything to an intermediate array. A type which encapsulates its data in multiple arrays (e.g. of 4096 elements each to avoid LOH allocations) would need a method to pass multiple `ReadOnlyArraySegment` to the other, but that would still be more efficient than copying individual items. – supercat Dec 14 '13 at 17:50
  • @supercat, I see where you're coming from, though I'm not sure that your proposed ISnapshot is useful enough to make it into the Framework. After all, the overhead of the existing ToList is rarely a bottleneck. As for IDictionary.Keys, it's clear that an interface can't make any promises, though the documentation for Dictionary.Keys is explicit enough. – Joe Dec 14 '13 at 19:00
  • In many cases the overhead of `ToList` is reasonably slight, but there are other cases where it may be huge, and others where `ToList()` may destabilize the system. Given an `IEnumerable`, I know of no way to take a snapshot without imposing an arbitrary size limit or risking destablizing the system. I'm not sure why you say interfaces can't make promises; the ``Keys` and `Values` collections of `IDictionary` are "guaranteed" to contain corresponding items in matching sequence; what is unclear is what is allowed to happen to those sequences if the underlying dictionary is modified. – supercat Dec 14 '13 at 20:16
  • I guess my thought is that when there is a common operation which could be done reasonably well by a type-agnostic method on most things which implement an interface, but the type-agnostic method would in some cases perform far worse than could a type-specific method, or could kill the system while trying to do something impossible, that's a pretty good sign that there should be a means by which types which know how to do things better could do so. Having a `Snapshot` and `SnapshotList` extension methods on `IEnumerable` which look for interfaces and use them if they exist... – supercat Dec 14 '13 at 20:33
  • ...would be pretty cheap, and allowing iterators to specify via attribute whether the iterator object's "snapshot" method should `return this;`, `return this.ToList()`, or `throw new NotSupportedException(...)` wouldn't seem overly expensive either. – supercat Dec 14 '13 at 20:36
7

You can return an instance of ReadOnlyCollection from your method:

return new ReadOnlyCollection(items);
Ufuk Hacıoğulları
  • 37,978
  • 12
  • 114
  • 156
0

A property which encapsulates part of an object's mutable state may, if the type is a mutable class, represent any of these:

  1. A live mutable view of the property's state
  2. A mutable view of a detached copy of the property's state
  3. A mutable view of a state that may or may not remain attached to the property's state

If the type is read-only, but not guaranteed to be immutable, it may represent any of these:

  1. A guaranteed-"live" read-only view of the property's state
  2. An immutable copy of the state as of the time it was called
  3. A view of a state that may start out being attached to the property's state, but may or may not remain attached.

While the type of the determined object may be used to distinguish the first set of three behaviors from the latter set, I know of no standard convention to indicate, in the absence of a suitable guaranteed-immutable return type, which of the behaviors within the appropriate set applies. Since there is no guaranteed-immutable implementation of IEnumerable<T> that would seem applicable here, your best bet is probably to use Joe's code, but also document your behavior as #3. Unless you can think of some reason clients would benefit from #2, documenting your behavior as #3 will give you the most flexibility to change your implementation in future should that be necessary. For example, if thread-safe access ends up being necessary in the future, your class could implement an internal method which locks the collection long enough to copy it to an array and returns that array (possibly wrapped in ReadOnlyCollection<T>, though it probably wouldn't matter) cast as IEnumerable<T>. Such an approach wouldn't work if the property was supposed to represent a live view.

supercat
  • 77,689
  • 9
  • 166
  • 211