dotnet / runtime

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

New API proposal; new collection interfacing, new read-only models – read-only & alter-also design pattern #31573

Closed bravequickcleverfibreyarn closed 3 years ago

bravequickcleverfibreyarn commented 4 years ago

Abstract

Recently I went through ReadOnlyCollection<T> (1, 2) finding it generaly failing by design terms.

I reviewed the origin (root cause) and I found out that some collection designs are not as good as they could.

Nowadays any movement toward protectivity and declarativity is beyond nice-to-have. Today readonly fields, structs, parameters (in) are part of programers’ day. (Let mention there ReadOnlySpan<T>, ReadOnlyMemory<T>.)

The same is missing on the collection level in framework in a way we get used to feel such presence. There exists in System.Collections.ObjectModel mentioned ReadOnlyCollection<T> that is declared to be base type for collection of kind. Must be noted that dedicated base class throws 12 times NotImplementedException. 🙅‍♂️

If one review interface like IReadOnlyList<T> or IReadOnlyCollection<T> surely will find that these interfaces are capable of little making them inapplicable for any reasonable read-only collection designing on framework level since for any advanced work one have to implement other interface or find out other workaround (like acting classic type as read-only one). In any place no collection alteration will ever come the read-only type could be used – idea for resource management.

As it was noted actual trend confirms read-only tendecies and wide popularity for it. For a such new trends new collections could become. I find that making design in let name it read-only & alter-also pairs fits this trend and demands. Beside popularity of readonlyness one can find out that such designs are really proper in opposite to actual ones.

That is observable at ReadOnlyCollection<T>. There is no general tendecy about its usage although commonly we declare readonly fields (they are stil less efective than classic fields since defensive copying), use in paramater modifiers etc. This is likely due its bad design and aside this also by fact that it is sited in some “top-level” designers’ namespace. Worth noting that regardless its naming it exposes indexer – like lists do. I found this far away from proper. It is just some dirty sufficient solution to readonlyness for case where it must be. More proper is to go with as simple type as it is needed like it is aforementioned but with such read-only collectioning framework avoids this.

Concrete

I prepared some design samples for demonstration of such read-only & alter-also patterning.

This pattern is about building “classic” types on top of their read-only counterparts.

Sample for non-generics:

namespace System.Fine.Collection.Collection.Interface
{
  /// <remarks>
  ///  Completely replaces System.Collections.ICollection that does not exists at all.
  /// </remarks>
  public interface IReadOnlyCollection : System.Collections.IEnumerable
  {
    void CopyTo(Array array, int index);

    int Count { get; }

    Object SyncRoot { get; }

    bool IsSynchronized { get; }
  }
}

namespace System.Fine.Collection.Collection.Interface
{
  interface IReadOnlyList : IReadOnlyCollection
  {    
    Object this[int index] { get; }

    bool Contains(Object value);

    int IndexOf(Object value);
  }
}

namespace System.Fine.Collection.Collection.Interface
{
  interface IList : IReadOnlyList
  {
    new Object this[int index] { get; set; }

    int Add(Object value);

    void Clear();

    bool IsFixedSize { get; }

    void Insert(int index, Object value);

    void Remove(Object value);

    void RemoveAt(int index);
  }
}

Sample for generics:

namespace System.Fine.Collection.Generic.Interface
{
  public interface IReadOnlyCollection<T> : System.Collections.Generic.IEnumerable<T>
  {  
    int Count { get; }

    bool Contains(T item);

    void CopyTo(T[] array, int arrayIndex);
  }
}

namespace System.Fine.Collection.Generic.Interface
{
  public interface ICollection<T> : IReadOnlyCollection<T>
  {        
    void Add(T item);

    void Clear();

    bool Remove(T item);
  }
}

namespace System.Fine.Collection.Generic.Interface
{
  public interface IReadOnlyList<T> : IReadOnlyCollection<T>
  {
    T this[int index] { get; }
  }
}

namespace System.Fine.Collection.Generic.Interface
{
  public interface IList<T> : IReadOnlyList<T>, ICollection<T>
  {
    new T this[int index] { get; set; }

    int IndexOf(T item);

    void Insert(int index, T item);

    void RemoveAt(int index);
  }
}

namespace System.Fine.Collection.Generic.Model
{
  public class ReadOnlyCollection<T> :
    Interface.IReadOnlyCollection<T>,
    Collection.Interface.IReadOnlyCollection
  {
    // Implementation.
  }
}

namespace System.Fine.Collection.Generic.Model
{
  public class Collection<T> :
    ReadOnlyCollection<T>, 
    Interface.ICollection<T>
  {
    // Implementation.
  }
}

namespace System.Fine.Collection.Generic.Model
{
  public class ReadOnlyList<T> :
    ReadOnlyCollection<T>, 
    Interface.IReadOnlyList<T>,
    Collection.Interface.IReadOnlyList
  {
    // Implementation.
  }
}

namespace System.Fine.Collection.Generic.Model
{
  public class List<T> : 
    ReadOnlyList<T>, 
    Interface.IList<T>,
    Collections.IList
  {
    // Implementation.
  }
}

Going along with this approach the benifits are:

  1. Straight hierarchy logic where all non-altering executions belongs to read-only type/interface.
  2. Consumer is provided with read-only types that are not lame brothers of their counterparts while alter-also types are fully capable.
  3. Resource efficiency. Since alter-also types are bigger in bytes, consumer can go on with lightweight but capable read-only types.
  4. Brings popular readonlyness to consumers instead of code magicians.

Conclusion:

I am totally aware about actual count of types in framework build on actual collection types and interfaces but still any move forward is not move backward or stiff attitude.

Also I know that such redesign is not just about to copy paste because time advanced and other changes would be included also in bunch.

It is not possible to look readonlyness popularity over and the same should be seen on framework collections. Not such stain like System.Collections.ObjectModel.ReadOnlyCollection<T>. (No any of read-only kind of the proper ones.)

This read-only & alter-also desing results in ReadOnlyList<T> that never throws for any member of any interfaces that it contracts to implement. That is far better than 12 throws.

Since actual framework collection robustness I suppose that such new designs should come as separated bunch in dedicated namespace like System.Collections.Fine. From there they can leak out.

scalablecory commented 4 years ago

I think if collections were designed today, we might have similar considerations.

I am totally aware about actual count of types in framework build-on actual collection types and interfaces

I think this will be your biggest hurdle. I don't feel the current system is harmful enough to warrant splitting functionality into new types.

bravequickcleverfibreyarn commented 4 years ago

I was thinking about that but forgot to mention it out there. It is fixed now.

Since actual framework collection robustness I suppose that such new designs should come as separated bunch in dedicated namespace like System.Collections.Fine. From there they can leak out.

huoyaoyuan commented 4 years ago

The language and framework teams have considered to implement a "better" collection abstraction via new language features.

Using existing syntax to provide a new series of interfaces benifits very little. With the new language syntaxes, every collection that has already implemented IList<T> will get most functionalities without change.

bravequickcleverfibreyarn commented 4 years ago

The language and framework teams have considered to implement a "better" collection abstraction via new language features…

Is there some reading like specification draft or some MSDN blog article about it? I understand little by you description.

I noted that to prevent breaking changes new collections should reside in new namespace.

(Immutability in this context expresses the same as readonlyness as I see.)

Also I proposed new types not just abstraction. I found such read-only types handy. If there are provided immutable types over read-only types, it means another design intentions – for instance immutable types are heavier and perhaps can be misused instead of classic mutable types because end consumer gets desired results anyway.

huoyaoyuan commented 4 years ago

Is there some reading like specification draft or some MSDN blog article about it?

dotnet/runtime#31001

The majority of effort is to let old collections and old methods get them by free. A new namespace that requires manual implementation won't be valuable.

bravequickcleverfibreyarn commented 4 years ago

The majority of effort is to let old collections and old methods get them by free…

I went through that dotnet/runtime#31001. It really touches the same area but it just makes another bypass to existing bad designs. Beside such form of default implementation is not natural IMO and rather seems more as work around than a solution IMHO it solves nothing really specific about consumer read-only collection types sited at framework level. It just lets bad read-only interfaces be implemented by bad altering interfaces.

Next-gen collections based on actual needs and trends is far better design modeling than rigid abiding on existing collections even if there are minor useless changes.

huoyaoyuan commented 4 years ago

Next-gen collections based on actual needs and trends is far better design modeling

If we start a new series of interface from now, everyone can get perfect design very quickly. But the ecosystem doesn't allow us to do so.

And also, it increases implementation overhead of both collection side and util method side.

It just lets bad read-only interfaces be implemented by bad altering interfaces.

Just to point out that your design is bad too. SyncRoot isn't well designed.

Given the ecosystem has been this shape, all further improvements must be something that can be transformed from current shape without modification of implementations.

This read-only & alter-also desing results in ReadOnlyList that never throws for any member of any interfaces that it contracts to implement. That is far better than 12 throws.

Why does they throw? Because such interfaces are strongly needed. For example, WPF data binding requires IList.

bravequickcleverfibreyarn commented 4 years ago

And also, it increases implementation overhead of both collection side and util method side.

You mean because of 2 collection bunches, right?

But the ecosystem doesn't allow us to do so.

I see there have to be either provided the same API twice. One for each of collection bunches. Or there have to be conversions. First is okay but not natural, second is not okay. One more option would be some compiler magic allowing to use new collections in place of old – that should not be impossible, I guess. ✌️

In either way I would find that beneficial although it would not worth it.

Just to point out that your design is bad too. SyncRoot isn't well designed.

You are rather right but let keep it there for simplicity of example. Otherwise there have to be 1 more interface like ISyncableCollection.

Why does they throw? Because such interfaces are strongly needed. For example, WPF data binding requires IList.

I am aware of actual implementation concepts. On the other hand read-only collection of such shape will never be popular. It is not common that collection throws in opposite for instance to streams. Basically it is useless wherever one cannot use its class type.

Likely the benifits could not equilibrate the demands so at this branch framework will stay behind the times but it is not any notable harm as you pointed out.

eiriktsarpalis commented 3 years ago

What would be the motivation for making this proposal? Experience shows that such additions while well-intentioned only function to increase the concept count of your typical codebase.

In order for the new interfaces to be useful, both runtime libraries and nuget ecosystem alike would need to implement them on their types. Do note however that there is a performance penalty when appending new interface implementations to types, so this is not pay-for-play.

While we do acknowledge the shortcomings of existing designs, these exist partly due to the fact that they weren't designed in one go, rather they are incremental additions reflecting evolving attitudes toward software, throughout the 20-year-old history of .NET. Consequently, even if we were to come up with the perfect design today, it's unlikely it would cover all requirements 20 years from now.

The problem with runtime libraries is that as soon as we add an API it crosses the threshold of immortality. Once it's there we need to maintain and ship it with the sdk, forever. Which is why we tend to favour conservative changes that improve the users' quality of life even though it might give the impression of design inconsistency. At the same time we strongly advocate for the community building their own versions of collections on NuGet.

All in all, I find it unlikely we would make a change of this magnitude, so I'm going to close this issue. Thank you for taking the time to contribute.