dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

[API Proposal]: Add extension method AsReadOnly() for ICollection<T> #87330

Open ghost opened 1 year ago

ghost commented 1 year ago

Background and motivation

Like List<T>.AsReadOnly, I'd like to propose the same for ICollection<T>. If I'm not mistaken, currently to convert an instance of ICollection<T> to IReadOnlyCollection<T>, we need either:

API Proposal

namespace System.Collections.Generic
{
    public static class CollectionExtensions
    {
+       public static IReadOnlyCollection<T> AsReadOnly(this ICollection<T> collection);
    }
}

API Usage

ICollection<int> collection = new HashSet<int>() { 1, 2, 3 };
IReadOnlyCollection<int> readOnlyCollection = collection.AsReadOnly();

Alternative Designs

No response

Risks

Nothing I can see.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-collections See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation Like `List.AsReadOnly`, I'd like to propose the same for `ICollection`. If I'm not mistaken, currently to convert an instance of `ICollection` to `IReadOnlyCollection`, we need either: - Call `IEnumerable.ToList` then `List.AsReadOnly` but this will allocate a new `List`. - Create a custom extension method `AsReadOnly` on `ICollection` like: ```cs public static class CollectionExtensions { public static IReadOnlyCollection AsReadOnly(this ICollection collection) { return new ReadOnlyCollectionWrapper(collection); } private sealed class ReadOnlyCollectionWrapper : IReadOnlyCollection { private readonly ICollection _collection; public ReadOnlyCollectionWrapper(ICollection collection) { _collection = collection; } public int Count => _collection.Count; public IEnumerator GetEnumerator() { return _collection.GetEnumerator(); } IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } } } ``` - Probably other solution but nothing built-in or simple without additional memory allocation ### API Proposal ```diff namespace System.Collections.Generic { public static class CollectionExtensions { + public static IReadOnlyDictionary AsReadOnly(this ICollection collection); } } ``` ### API Usage ```cs ICollection collection = new HashSet() { 1, 2, 3 }; IReadOnlyCollection readonlyCollection = collection.AsReadOnly(); ``` ### Alternative Designs _No response_ ### Risks Nothing I can see.
Author: cakescience
Assignees: -
Labels: `api-suggestion`, `area-System.Collections`
Milestone: -
huoyaoyuan commented 1 year ago
  • Call IEnumerable.ToList<TSource> then List<T>.AsReadOnly but this will allocate a new List<T>.

No. This is semantically different, since it copies the content of the collection, instead of creating a read-only view.

ReadOnlyCollection<T> is in fact an "readonly list" wrapper. However, HashSet<T> implements IReadOnlyCollection<T> directly and there's no need to create a wrapper. You can't convert arbitrary ICollection<T> to IReadOnlyCollection<T> though, but can you give real-world business example? ICollection<T> itself is much less popular than IList<T>.

huoyaoyuan commented 1 year ago

I'm currently using ICollection<T> and there's no easy way to convert it to IReadOnlyCollection<T>.

Yes, this is a historical pain since the read-only interfaces came later. Is the collection implemented by yourself or the BCL? You can easily add IReadOnlyCollection<T> to your collections. All the BCL collections should have implemented it. If not, we should probably add it. Are you passing collections in ICollection<T> instead of its concrete type? This is a valid scenario. But if you can make sure the collection implements IReadOnlyCollection<T>, you can do a cast when passing to the third-party library as a workaround.

Is your recommendation to switch from ICollection<T> to IList<T>?

IReadOnlyCollection<T> gives literally little capacity (count only) thus it's not popular. Is there any reason to not use IList<T>?

eiriktsarpalis commented 1 year ago

Following the pattern of other read-only collections, shouldn't this expose a new collection type akin to ReadOnlyCollection<T> (that is actually a read-only list)?

huoyaoyuan commented 1 year ago

shouldn't this expose a new collection type akin to ReadOnlyCollection<T> (that is actually a read-only list)?

This is the expected approach, however the name is already taken.