Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: ReadOnlySet<T> #100113

Closed
stephentoub opened this issue Mar 22, 2024 · 4 comments · Fixed by #103306
Closed

[API Proposal]: ReadOnlySet<T> #100113

stephentoub opened this issue Mar 22, 2024 · 4 comments · Fixed by #103306
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Collections in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Mar 22, 2024

Background and motivation

If someone wants to return an IList<T> via a read-only wrapper that prevents the list from being mutated via that reference, they can wrap it in a ReadOnlyCollection<T>.

If they have an IDictionary<TKey, TValue>, they can wrap it in a ReadOnlyDictionary<TKey, TValue>.

But if they have an ISet<T>, there's no ReadOnlySet<T> that can be used to easily wrap the set. The dev needs to either write their own wrapper type, and will instead often end up making a copy with an ImmutableHashSet<T> or FrozenSet<T>.

Related:
#76028
#29387
#2293

API Proposal

namespace System.Collections.ObjectModel;

[DebuggerDisplay("Count = {Count}")]
public class ReadOnlySet<T> : IReadOnlySet<T>, ISet<T>, ICollection
{
    public ReadOnlySet(ISet<T> set);

    public static ReadOnlySet<T> Empty { get; }

    protected ISet<T> Set { get; }

    public int Count { get; }
    public IEnumerator<T> GetEnumerator();

    public bool Contains(T item);
    public bool IsProperSubsetOf(IEnumerable<T> other);
    public bool IsProperSupersetOf(IEnumerable<T> other);
    public bool IsSubsetOf(IEnumerable<T> other);
    public bool IsSupersetOf(IEnumerable<T> other);
    public bool Overlaps(IEnumerable<T> other);
    public bool SetEquals(IEnumerable<T> other);

    ... // explicit interface implementations of all other interface methods, throwing from mutating members
}

API Usage

HashSet<int> set = ...;
var ros = new ReadOnlySet<int>(set);

Alternative Designs

  • ISet<T> exposes CopyTo while IReadOnlySet<T> does not. ReadOnlySet<T> will implement it, but should it be implicit or explicitly implemented? (Alternatively/separately, should we consider adding CopyTo to IReadOnlyCollection<T> and thus implicitly to IReadOnlySet<T>?)
  • Do we also want an ReadOnlySet<T> AsReadOnly() on HashSet<T>? List<T> has ReadOnlyCollection<T> AsReadOnly() and there's a static ReadOnlyCollection<T> AsReadOnly(T[]) on Array, but Dictionary<> doesn't have an AsReadOnly.
  • Do we also want an extension method ReadOnlySet<T> AsReadOnly(this ISet<T>)? Such an extension exists for both IList<T> and IDictionary<>.
  • What reference assembly should this be exposed from? ReadOnlyCollection/Dictionary are both implemented in CoreLib and exposed via System.Runtime. Should this live there, too? Or in System.Collections.dll?

Risks

n/a

@stephentoub stephentoub added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Collections labels Mar 22, 2024
@stephentoub stephentoub added this to the 9.0.0 milestone Mar 22, 2024
Copy link
Contributor

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

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 22, 2024
@N-Olbert
Copy link

N-Olbert commented Mar 26, 2024

Should there be an AsReadOnly-(extension)method for HashSet and OrderedSet, like there is one for List?

public ReadOnlySet<T> AsReadOnly()
            => new ReadOnlySet<T>(this);

// Or as Extension, probbaly somewhere within System.Linq
public static ReadOnlySet<T> AsReadOnly(this ISet<T> set)
            => new ReadOnlySet<T>(set);

@stephentoub stephentoub self-assigned this Apr 11, 2024
@terrajobst
Copy link
Member

terrajobst commented Jun 11, 2024

Video

  • Looks good as proposed
namespace System.Collections.ObjectModel;

[DebuggerDisplay("Count = {Count}")]
public class ReadOnlySet<T> : IReadOnlySet<T>, ISet<T>, ICollection
{
    public ReadOnlySet(ISet<T> set);
    public static ReadOnlySet<T> Empty { get; }
    protected ISet<T> Set { get; }
    public int Count { get; }
    public IEnumerator<T> GetEnumerator();
    public bool Contains(T item);
    public bool IsProperSubsetOf(IEnumerable<T> other);
    public bool IsProperSupersetOf(IEnumerable<T> other);
    public bool IsSubsetOf(IEnumerable<T> other);
    public bool IsSupersetOf(IEnumerable<T> other);
    public bool Overlaps(IEnumerable<T> other);
    public bool SetEquals(IEnumerable<T> other);
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 11, 2024
@stephentoub stephentoub added the in-pr There is an active PR which will close this issue when it is merged label Jun 11, 2024
@KennethHoff
Copy link

KennethHoff commented Jun 14, 2024

#29387 was closed because there didn't exist a ReadOnlySet at the time. Now that it exists, will that also come in .Net 9? I didn't see it in the closing PR (#103306).

@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Collections in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants