-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Proposal: Add IsImmutable property to IReadOnlyCollection<T> and IReadOnlyDictionary<TKey,TValue> #20418
Comments
Second approach I described in this issue https://github.com/dotnet/corefx/issues/16661 |
Could you explain what those cases are? The one I can think of is when you're caching some collection and want to know if it's safe to cache it directly, or if you have to create a copy of it.
That's not what var list = new List<int>();
ICollection<int> ro = new ReadOnlyCollection<int>(list);
Console.WriteLine(ro.IsReadOnly);
Console.WriteLine(ro.Count);
list.Add(1);
Console.WriteLine(ro.Count); The output shows that even though
The documentation could be clearer about this. |
This is very bad example because ReadOnlyCollection is a special case. Main purpose of this class is to save some collection from changes but allow to read collection content. public void ReadMyCollection(IReadOnlyCollection<int> c)
{
var mutableCollection = (ICollection<T>)c;
// Make diversion
c.Add(34);
} IsReadOnly looks like have the problem with convention. IsReadOnly is ReadOnly or Immutable. |
Probably the only solution would be adding new IsImmutable property. And finally write strict convention into all documentation. |
@svick do you have documentation change proposal? We can ask docs folks to update it ... @dmitriyse I still don't get your differentiation between IsReadOnly and IsImmutable, sorry. Do you have a small sample to demonstrate the difference? (something like @svick used above?) |
I updated topic and there an answer. |
OK, so |
The motivation was discussed here: https://github.com/dotnet/corefx/issues/16661#issuecomment-284587410
Of course, let's open only API proposals which do NOT depend on not-yet-implemented C# features - in such case we should wait (see https://github.com/dotnet/corefx/issues/16818#issuecomment-284790069). Closing as dupe for now, please let me know if you disagree. |
When dotnet/csharplang#52 feature will be added to C#/CLR (at least in restricted form), we will have ability to extend base BCL contracts.
This proposal rely on this suggestion and tries to solve ambiguous semantic of
ICollection<T>.IsReadOnly
property.We needs a new property.
Let suppose we have two different collections both inherited from ICollection<T>
First example is when library allows to access some internal collection by the interface.
this is ReadOnly meaning of IsReadOnly property.
Second collection can be well known ImmutableList
Probably "it's too late" and we have tons of code one collections uses ReadOnly semantic
another Collections uses ReadOnly+Immutable semantic.
But now we can do the next improvement in the documentation:
IsReadOnly says that only:
all mutation method will return NotSupportedException or InvalidOperationException. And no any more guaranties.
The text was updated successfully, but these errors were encountered: