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

Do something with IReadOnlyCollection and ICollection. We needs good abstractions hierarchy. #219

Closed
dmitriyse opened this issue Mar 2, 2017 · 10 comments

Comments

@dmitriyse
Copy link

Just drop IReadOnly****, mark them obsolete.
We can't do anything with backward compatibility here.
If we inherit ICollection from IReadOnlyCollection we will broke existing code.
See: http://stackoverflow.com/questions/12622539/why-doesnt-generic-icollection-implement-ireadonlycollection-in-net-4-5

But may be sometime will be good point do do this breaking change, provide tools for migration automation.

And also IReadOnly*** is not clear solution, we needs IReadable*** and IImmutable****

This is not-null reference types like problem. But it's second after nullability.
Let's start/continue to solve this problem.

@dmitriyse dmitriyse changed the title Do something with IReadOnlyCollection and ICollection. We needs good abstraction hierarchy. Do something with IReadOnlyCollection and ICollection. We needs good abstractions hierarchy. Mar 2, 2017
@jnm2
Copy link
Contributor

jnm2 commented Mar 2, 2017

I like this except that so many .NET libraries, not just the BCL, have accepted that IReadOnly == IReadable and follow that convention. I don't see any wins in fixing it in the BCL after it's sunk in and spread so far. I choose ReadOnly over Readable because that's what it means to everyone now and it communicates precisely what I intend. Yes it's illogical but like the English language, we have no problem making ourselves understood.

@dmitriyse
Copy link
Author

dmitriyse commented Mar 2, 2017

If we can't do anything in CLR, We can use not-null reference approach. ICollection annotated with ImmutableAttribute or ReadableAttribute should be handled by compiler and DevTools to block from writing to a collection.

public void SomeReadOnlyConsumer(IReadable<T> c){}

or

public void SomeReadOnlyConsumer(readable ICollection<T> c){}

can be treated by CLR as

public void SomeReadOnlyConsumer([Readable]ICollection<T> c){}

This approach even better fits to immutability semantic. IFreezable is best example. The same ICollection can be mutable and immutable after Freeze call. So it' very very close to well-designed nullability solution.

Toolset will save us!

@dmitriyse
Copy link
Author

IReadOnly == IReadable, Yes i agree it's not a bit trouble. We just needs IImmutable and some way to annotate semantic of each variable/argument/field/property.

@HaloFour
Copy link
Contributor

HaloFour commented Mar 2, 2017

This has nothing to do with C# the language. I'd suggest proposing this at the CoreFX repository.

Although this argument has already happened here. In short, too late, that ship has long since sailed.

@dmitriyse
Copy link
Author

dmitriyse commented Mar 2, 2017

Yes I will duplicate. But I disagree without that nothing to do with C#. Non-Null reference is also "too late" feature. But it will be finally appear in C# 8.0.
THE SAME APPROACH POSSIBLE.

public void MyMethod(readable ICollection<T> c)
where T: new
{
     c.Add(new T()) // Compiller Error/Warning.
}

It's toolset/C# languate related solution method.

@dmitriyse
Copy link
Author

dmitriyse commented Mar 2, 2017

public void MyMethod(readable ICollection<T> c)
where T: new
{
     c.Add(new T()) // Compiller Error/Warning.
}

public void MyMethod(immutable ICollection<T> c)
where T: new
{
     c.Add(new T()) // Compiller Error/Warning.
}

This would be killing feature, that can greatly improve compile-time checking.

@dmitriyse
Copy link
Author

dmitriyse commented Mar 2, 2017

public class MyClass
{
    // here why readable but not readonly keyword.
    private readable readonly ISet<strings> _myStrings;
    public MyClass(IEnumerable<string> strings)
    {
         _myStrings = new HashSet<string>(strings);
    }
    public readable ISet<string> MyStrings {} => _myStrings;
}

public void Test()
{
    var a = new MyClass(new[] {"dog", "cat"});
    a.Contains("dog"); // Good usage.
    a.MyStrings.Add("mouse"); // Compiler error/warning.
}

@dmitriyse
Copy link
Author

There is more suitable solution for this general problem:
#222
https://github.com/dotnet/corefx/issues/16660
https://github.com/dotnet/corefx/issues/16661
https://github.com/dotnet/corefx/issues/1973

So I am closing this issue on C# Language repository.

@svick
Copy link
Contributor

svick commented Mar 3, 2017

Not sure if you've arrived at a similar idea independently, or if you were inspired by it, but the approach of marking variables as readable and immutable sounds very similar to what @joeduffy describes Midori (experimental OS based on C#/.Net) did. Though their system was much more comprehensive and complicated and was mostly about concurrency safety.

@dmitriyse
Copy link
Author

Thank you for this link, I will inspect their work and result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants