src/Common/src/CoreLib/System/Collections/ObjectModel/ReadOnlyCollection.cs design failures

[deep-outcome]

1. Since ReadOnlyCollection<T> exposes indexer, its naming should be ReadOnlyList<T> to stuck with current approach.
2. Implementing ICollection<T>, IList<T>, IList is also debatable. Since .NET Core is brand new, one could have been observing more changes to collection designs:
   • Saying that ReadOnlyCollection<T> is implementing these is as well designed as design IReadOnlyList<T> in manner where public indexer public T this[int index] { get; } will have setter part supposed to throwing NotImplementedException.
   • Proper way is to redesign read-only interfaces in a way that they are providing non-altering methods like Contains(T), IndexOf(T) and remove these methods from their alter-also counterparts. So for instance ICollection<T>.Contains(T) should be sited to IReadOnlyCollection<T>. Alike IList<T>.IndexOf<T> should be in IReadOnlyList<T>. Then altering interface should implement read-only interface in natural logic way of inheritance design.

Then there can be ReadOnlyList<T> not implementing many methods that throws exception. 👍

[stephentoub]

Thank you for sharing your concerns. But what is the purpose of this issue? This type was released publicly years ago. Whether or not one agrees with the design choices made them, such changes will not be made now, as they'd all be very breaking.

[deep-outcome]

I knew what will be the answer but still. Recently I went through these implementations finding out that such type designs are out of date.

One can say there is not direct benefit of this but this could be said to every design problem where work around exists. Design is not about direct benefits. It is about patterns, paradigms, performance, security, clarity, to-human-softness. Such read-only and alter-also interface couples and throwing read-only collections that are read-only lists are not this.

I see no issue in moving toward proper designs including introducing “doubling” types. Just it would be for instance in case of System.Collections.Generic, System.Collections.ObjectModel with System.Collections.New.Generic, System.Collections.New.ObjectModel. There exists some approach to backward compatiblity but this does not disallow to bring new well designed types.

System.Collections.ObjectModel namespace ensued as result of some decision. Now this namespace seems like pushed away child (personal opinion) more than anything else. I mean if there were not VB conflict would ever this namespace exists? I guess not. So now there in dedicated namespace for object modeling exists type clearly declared as intentional base type that throws in 12 implementing members. That’s a mess and also quite a joke. (See end.)

I do not know exact history but I guess that read-only interfaces came after alter-also ones but this is no barrier to distribute their members properly. Copy-paste implementation to new types in new namespace is not difficult work at all.

Nowadays one can find on List<T> declared bool ICollection<T>.IsReadOnly { get { return false; } } while IReadOnlyList<out T> does not expose such property (and any base interface up the tree neither). It is awkward, it seems like contradiction and more over does not make any sense at all.

Let make conclusion:

1. ReadOnlyCollection<T> is list in fact, it is encouraged to be used as base type although it throws really often and it is also placed somewhere out of sight so nobody will ever tend to use it other way than object base class.
2. These little sense read-only torso interfaces with their over-loaded alter-also counterparts made any read-only collection designing using framework interfaces out of the scheme. That is not proper. One can use read-only type in places where no alteration is comming. For that reason particular read-only interface implementation could be choosen. Like not throwing ReadOnlyList<T> or ReadOnlyCollection<T>.
3. Back to design guideliness again. Simpler objects consume less memory so beside let say proper hammer for several nail approach resource management is common design consideration – read-only types can be choosen where appropriate.

So purpose of this issue is to state that those designs are not good – especially in ReadOnlyCollection<T> this emanates evidently – and that is not bad to move toward clean implementations. The more so they are clear at sight. I think any ignition toward changes to better is fine.

Now the joke – I imagine something like tableau where some man telling me, “Hey guy take this rattletrap. It shall surely take you there.”.

(I name non-read-only interfaces alter-also while it is not exact name at present but for reading me well this naming is proper enough.)

[danmoseley]

Thanks @Uzivatel919 . As @stephentoub mentions, all your suggestions seem to be breaking changes. If you'd like to propose non breaking changes (eg., additive changes) please open a new issue. I'll close this one.

[deep-outcome]

@danmosemsft As far as I could say new types cannot be breaking change if of course I am not missing something.

[danmoseley]

Yes, if you propose to add a new API that is generally not a breaking change. You can do this by opening an issue specific to the change you propose. Please see
https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

[deep-outcome]

I am not finding there anything about breaking changes. Also I cannot imagine how new bunch of consumer types could break any existing API but I am about to go with your decision. As I noticed some point from that page I can come back with some proposal skeletons. Perhaps I will do it later on.

[danmoseley]

As I said " if you propose to add a new API that is generally not a breaking change" (although I had a typo)

You are welcome to propose new API, but please in a new issue in the format specified in the document linked.

[deep-outcome]

Ah, great. Now I understand.

[deep-outcome]

See https://github.com/dotnet/corefx/issues/42729.