Add an `ISet<T>.AsReadOnly()` extension method

[vanillajonathan]

The List<> class have a AsReadOnly method to turn the collection into a ReadOnlyCollection, but HashSet<> does not have any such method.

Rationale and usage

Allows Entity Framework Core to expose a collection navigation property as a IReadOnlyCollection when backing it with a private backing field declared as a HashSet.

[Table("Blogs")]
public class Blog
{
    private HashSet<Post> _posts;
 
    public int BlogId { get; set; }
 
    public IReadOnlyCollection<Post> Posts => _posts?.AsReadOnly();
 
    public void AddPost(Post post)
    {
        // Do some business logic here...
        _posts.Add(post);
    }
}

Proposed API

namespace System.Collections.Generic;

public class CollectionExtensions
{
      public ReadOnlyCollection<T> AsReadOnly(this IList<T> list);
      public ReadOnlyDictionary<T> AsReadOnly(this IDictionary<T> dictionary);
+     public ReadOnlySet<T> AsReadOnly(this ISet<T> set);
}

[safern]

Thanks @vanillajonathan I think this makes sense. I've marked the issue as api ready for review.

[omariom]

Returning ReadOnlyCollection means AsReadOnly will have to make a full copy of the HashSet.

What about a new type ReadOnlySet than can wrap any ISet<T>?
Then we will have readonly wrappers for all "ObjectModel" collections: IList, IDictionary, ObservableCollection and ISet.

[GrabYourPitchforks]

Feedback - what are you actually trying to do here?

HashSet<T> already implements IReadOnlyCollection<T>, so you can cast it directly: IReadOnlyCollection<T> collection = myHashSet;.

If you want to create a ReadOnlyCollection<T>, it's one line: ReadOnlyCollection<T> collection = new ReadOnlyCollection<T>(myHashSet.ToArray());.

[vanillajonathan]

I want to create a entity in Entity Framework Core. EF Core internally treats collections as hash sets but they are commonly exposed as ICollection properties in the DbSet.

I intend to employ domain-driven design (DDD), so I do not want to expose it as a ICollection since that would allow the collection to be modified.

Since HashSet (as you pointed out) implements IReadOnlyCollection<T> I can expose it as a IReadOnlyCollection (instead of a ICollection) but since that is just exposing it as a different interface it is still possible to typecast it back to a ICollection and modify it.

[GrabYourPitchforks]

In your scenario, accessing the Posts property will return a brand new collection instance every single time. I'd instead recommend using the existing List<T> and ReadOnlyCollection<T> types, especially since it seems HashSet<T> might not be a good fit for the Post type.

Example:

public class Blog
{
    private List<Post> _posts = new List<Post>();
    private ReadOnlyCollection<Post> _postsAsReadOnlyCollection = new ReadOnlyCollection<Post>(_posts);

    public IReadOnlyCollection<Post> Posts => _postsAsReadOnlyCollection;

    public void AddPost(Post post)
    {
        // business logic here
        _posts.Add(post);
    }
}

Or, if you really want to use HashSet<Post>:

public class Blog
{
    private HashSet<Post> _posts = new HashSet<Post>();
    private ReadOnlyCollection<Post> _cachedROC = null;

    public IReadOnlyCollection<Post> Posts => _cachedROC ?? (_cachedROC = new ReadOnlyCollection<T>(_posts.ToArray()));

    public void AddPost(Post post)
    {
        // business logic
        _posts.Add(post);
        _cachedROC = null; // clear cached value if it exists
    }
}

[vanillajonathan]

I was under the impression that ToList() would create a brand new collection instance every single time, and that AsReadOnly would not do so.

In the first of your examples I would need two private and one public instead of just one public or one private setter. Also it uses List while EF Core internally uses HashSet so the performance may differ.

In the second of your examples, it introduces caching into the business entity.

Both of these are undesirable. Could HashSet have a AsReadOnly method, and would that solve this in a clean, efficient manner?

[GrabYourPitchforks]

I was under the impression that ToList() would create a brand new collection instance every single time, and that AsReadOnly would not do so.

Yes, it must create a brand new collection every time. The reason for this is that ReadOnlyCollection<T> requires the underlying collection to implement IList<T>, which HashSet<T> does not do on its own. This means that a AsReadOnly() method which returns a ReadOnlyCollection<T> necessarily needs to make a copy of the contents of the HashSet<T> instance.

[vanillajonathan]

The reason for this is that ReadOnlyCollection<T> requires the underlying collection to implement IList<T>, which HashSet<T> does not do on its own.

This raises the question, should HashSet<T> implement IList<T>?

[GrabYourPitchforks]

This raises the question, should HashSet implement IList?

No. HashSet<T> is an unordered no-duplicate collection. IList<T> is an ordered list, potentially with duplicates. They represent two different collection concepts.

[vanillajonathan]

This raises the question, should HashSet implement IList?

No. HashSet<T> is an unordered no-duplicate collection. IList<T> is an ordered list, potentially with duplicates. They represent two different collection concepts.

Good point. Could HashSet<T> have a AsReadOnly method (like List<T> does, but without implementing IList<T>)?

With a List<T> you can cleanly do things like:

public class Blog
{
    private List<Post> _posts;
    public IReadOnlyCollection<Post> Posts => _posts?.AsReadOnly();
}

Which you cannot do as cleanly when using a HashSet<T> instead of a List<T>.

[GrabYourPitchforks]

Which brings us back to the original response on the thread... given your scenario it really would just be easier to have a property that directly casts the HashSet<T> to an IReadOnlyCollection<T>.

Since HashSet (as you pointed out) implements IReadOnlyCollection I can expose it as a IReadOnlyCollection (instead of a ICollection) but since that is just exposing it as a different interface it is still possible to typecast it back to a ICollection and modify it.

So don't cast it back? :)

[vanillajonathan]

Which brings us back to the original response on the thread... given your scenario it really would just be easier to have a property that directly casts the HashSet<T> to an IReadOnlyCollection<T>.

Since HashSet (as you pointed out) implements IReadOnlyCollection I can expose it as a IReadOnlyCollection (instead of a ICollection) but since that is just exposing it as a different interface it is still possible to typecast it back to a ICollection and modify it.

So don't cast it back? :)

I wouldn't typecast it back to a ICollection<T>, but if I exposed a IReadOnlyCollection<T> then I want that to be read-only, but someone using my library could typecast it back a ICollection<T> and modify it.
With a List<T> this is much easier to prevent since it has a AsReadOnly method.

[GrabYourPitchforks]

So what is your concrete proposal then? Are you proposing a new type ReadOnlyCollectionWrapper<T> whose ctor takes an IReadOnlyCollection<T> instead of an IList<T>? Is this a scenario that is common enough and that would be burdensome enough for somebody to do themselves that such a type belongs in the Framework proper? Presumably if you need to be unblocked immediately you've already created such a wrapper.

[vanillajonathan]

Well I don't know how feasible it is, but I expected the HashSet<T> to have a AsReadOnly method, so I would propose the addition of a AsReadOnly method to the HashSet<T>.

[GrabYourPitchforks]

And what would be the return type of HashSet<T>.AsReadOnly()? As discussed it can't be ReadOnlyCollection<T>.