ToReadOnlyCollection

[bgrainger]

See Faithlife/FaithlifeUtility#5.

ToReadOnlyCollection has somewhat unpredictable behaviour, depending on the nature of the IEnumerable<T> it's called on:

• ReadOnlyCollection<T>: returns same object
• List<T>: returns an .AsReadOnly() wrapper around it; changes the caller makes to the list will be reflected in this wrapper, which is usually undesirable behaviour when the return value of this call is stored in a field by a constructor
• IEnumerable<T>: calls .ToList().AsReadOnly() and gets a new immutable list

Furthermore, this method returns ReadOnlyCollection<T>, which we would like to phase out.

Most usages of this method should be replaced with AsReadOnlyList(); some should change to .ToList().AsReadOnly().

N.B. enforcing this rule would be a huge breaking change. An intermediate step might be to enforce that .ToReadOnlyCollection() may not be used to initialize fields or properties in constructors and that .ToList().AsReadOnly() must be used instead. (This addresses the buggiest and least frequent usage.)

[RobertBolender]

Furthermore, this method returns ReadOnlyCollection, which we would like to phase out.

Could you elaborate on this?

[bgrainger]

https://stackoverflow.com/c/faithlife/a/54

[RobertBolender]

So we specifically want to phase out its use as a return value, correct?

[bgrainger]

Yes, phase it out in public APIs; there's nothing wrong with using it in an implementation to enforce read-only-ness. The suggestions above include using .ToList().AsReadOnly() which returns a ReadOnlyCollection<T>.