Design pattern for read-only vs. read-write collections


Interesting discussion over in WinFX land today, thought you might want to chime in:


What is better from the standpoint of the design guidelines:
1) Single class FooCollection with an IsReadOnly property and an AsReadOnly method returning a FooCollection
or
2) Two classes: FooCollection and ReadOnlyFooCollection


One suggestions, which I think is a pretty good one:


It depends. I would do #2 in commonly used API as it’s cleaner in terms of the Object Model. I would do #1 in to save one type is less commonly used API.


Of course with generics this gets a little easier…


What do you think? 



 


 

Comments (20)

  1. RichB says:

    I agree with the suggestion you posted. You could also consider placing a set of ReadOnly classes into a .ReadOnly namespace, much as System.Collections.Specialized classes have their own namespace.

  2. Alan McBee says:

    I don’t support *.ReadOnly namespace. It’s only a semantic difference, right? Namespaces are like UML packages to me; they serve to collect a group of related classes, not to distinguish operational differences between two otherwise identical classes.

    I’d much rather catch an illegal mod to a collection at compile time, rather than run time. I’m not crazy about putting ReadOnly* as the prefix — all the read-only collections will be grouped together alphabetically, when I’d rather each one be next to its read-write counterpart. Names like FooCollection/FooFixedCollection, or FooCollection/FooCollectionReadOnly, make more sense to me.

    That said, I must agree with the sensibility of using a single class name w/ IsReadOnly property for less commonly used APIs.

  3. Just for fun, here is a couple of crazy ideas:

    1. Control access to collection not by collection type but rather by accessor class. Like it is done in STL for iterator and const_iterator

    2. If you opt to introduce R/O flag, make it not boolean flag but predicate object which decides what of operations are allowed. (Kind of poor man MOP dispatcher).

    3. Itroduce "modification token" object. You need to pass this object as extra parameter to all methods which are allowed to modify the collection. This token is returned at the time the collection is created and is unique per instance. This way the creator could control to whom to gran this token. This will allow to move access control decision it logic of application code rather than hardcoding in the object type. For instance you can pass collection object as read-only along the chain of calls, but at some level it may get to somebody who have "modification token" and will be able to modify it (actually making it non-const).

  4. Lee says:

    I would prefer a single class. If the framework ever gets const support then I think it would be cleaner to convert in the future.

  5. It absolutely, in all circumstances, MUST BE A SINGLE CLASS.

    That is a no-brainer. It HAS TO BE A SINGLE CLASS.

    Can I shout any louder here?! 😉

    Consistency is key to an API: if half of the readonly collections are referred to by a different class name, and the other half are referred to invoking a method on a ReadWrite class, then that is just about the WORST THING THAT I CAN IMAGINE.

    That is a pit of despair, for users of the API anyway. Bad, bad, bad. Please do this with one class. It’s even bad OO, IMO: a readonly collection of widgets is not a different THING to a readwrite colleciton of widgets. The writeability of the collection is so clearly a property of a single object, that it hurts. Please don’t implement this as two classes.

  6. Olivier Le Pichon says:

    we really need:

    IReadOnlyKindOfCollection

    {

    // only getter stuff

    // no modifications on collection permited

    // (no Add, Remove but can permit to modify inner datas)

    }

    and

    IKindOfCollection

    {

    // return a read only view of the current datas in this IKindOfCollection (share and not copy)

    IReadOnlyKindOfCollection AsReadOnly

    }

    It can provide us a unified way to export a read only view of an internal (non read only) collection.

    And each kind of collection (Array, ArrayList, List, Hashtable, and so on) must have same pattern.

    This can be achieved by inheriting IKindOfCollection from IReadOnlyKindOfCollection and AsReadOnly can be as simple as returning this. It’s easy but not secure. It’s always possible to reinterprete a read only view with a single cast.

    A better implementation should return a new object implementing IReadOnlyKindOfCollection but using shared datas.

  7. Michael says:

    I think you should use an interface based approach:

    interface IReadOnlyFooCollection

    {

    // Readonly members

    }

    interface IFooCollection : IReadOnlyFooCollection

    {

    // Add, Remove…

    }

    And use an appropriate wrapper similar to ArrayList.Synchronized/ArrayList.ReadOnly to prevent casting to the non-readonly version.

    In my opinion there should only be a single-class implementation to prevent unnecessary copying operations at runtime.

  8. Ben says:

    I’m down with Michael’s idea about having a mutable thing inherit from the readonly thing. That way, APIs that don’t need to modify the instance can indicate that they are not by taking the readonly version. APIs that are going to mutate the instance take the mutable version. ie:

    There are problem with this, at least in the current version of c# (maybe they’re fixed in 2.0). Can I turn a read only property into a read/write property in a subclass in 2.0? ie:

    class Map<T> {

    public T this[int index] { get; }

    }

    class MutableMap<T> : Map<T> {

    public T this[int index] { get; set; }

    }

    ? If not, you have to do some goofy workarounds to get compile-time verification that you’re not trying to call a mutator on the instance.

    Would people rather have compile-time or run-time notification that they’re trying to mutate a readonly collection? Personally, I’d like compile-time.

  9. Ben says:

    Actually, looking at the code I just posted, I bet I can do that in 1.0 & 1.1 if I take out the generics and slap a new on the Mutable indexer. The real case I’m wondering about deals with straight properties:

    class Person {

    public virtual string Name { get; }

    }

    class MutablePerson : Person {

    public override string Name { get; set; }

    }

    Can I do that in 2.0? Or do I still have to do:

    class Person {

    protected string name;

    public Person(string name) { this.name = name; }

    public string Name { get { return "Ben"; } }

    }

    class MutablePerson : Person {

    public MutablePerson(string name) : base(name) {}

    public void SetName(string value) { this.name = value; }

    }

    Thanks.

  10. Marc Scheuner says:

    I would definitely vote for a ReadOnly flag.

    What if you need to have a collection that might be read/write at first, and then becomes read-only? If you have two distinct object types, you’re basically screwed, or copying around data like crazy.

    Also, what if I want to have a collection that’s read/write for some, read-only for other users? With a flag, I can easily change that behaviour based on the user – with distinct classes, I have to instantiate either a rw-coll or a ro-coll.

    I think a flag makes for a cleaner design, and more flexibility. After all, you can’t protect your code from *ALL* idiot programmers! :-) Someone will dream up a way to get around ANY scheme you think of to avoid getting at data…

  11. Jim Argeropoulos says:

    Read-only to who?

    You need to be able to fill it somehow. The question is really how do you pass the read-only semantics to others?

    I really think STL had this one nailed. I also think that too few under stood the possibilities available to them.

    If a function only needs the ability to iterate over the elements contained, you pass a range of iterators.

    If the ability to add items is needed you give an insert_iterator.

    If the ability to delete is needed you pass the container.

    Of course there is also the issue of the elements in the container are immutable.

  12. I’d vote for one class and a ReadOnly flag. I can understand the argument that a separate class might be apropriate, but I’d prefer to think of ReadOnly-ness as a state, rather than a behavior. If it’s a behavior then two classes make sense, but I think in this case it’s easier to model it as a state.

  13. Keith Patrick says:

    My ultimate preference (and I’m already ducking) is the C++ way, whereby params for a method can be marked const and forbids calling non-const methods of that parameter within the method, but that is a feature of the language as opposed to the framework. And to be honest, I’ve never tried using the const (or equivalent) modifier in a managed language, so maybe it even works (although I doubt it, given that I’ve never seen it done outside of C++)

  14. There can be no doubt about it… compile time checks makes the world a better place. My vote is on the pattern with two types, where one is a specialized version of the other.

  15. I’m going to opt for a single class, but with three private nested classes:

    1) Uses a factory class to return an initial ReadWrite collection.

    2) Can convert (by cloning) to and from read write/read only.

    3) Only downside is that the Add method is now a function that returns a collection reference.

    Public MustInherit Class TCollection

    Implements IEnumerable

    Public MustOverride Function GetEnumerator() As System.Collections.IEnumerator Implements System.Collections.IEnumerable.GetEnumerator

    Public MustOverride Function Add(ByVal item As Object) As TCollection

    Public MustOverride ReadOnly Property isReadOnly() As Boolean

    Private Sub New()

    End Sub

    Public Shared Function CreateInstance() As TCollection

    Return New TCollectionReadWrite

    End Function

    Private MustInherit Class TCollectionBase

    Inherits TCollection

    Protected al As New System.Collections.ArrayList

    Public Sub New()

    MyBase.New()

    End Sub

    Public Sub New(ByVal ts As TCollection)

    Me.New()

    For Each t As Object In ts

    al.Add(t)

    Next

    End Sub

    Public Overrides Function GetEnumerator() As System.Collections.IEnumerator

    Return al.GetEnumerator

    End Function

    End Class

    Private Class TCollectionReadOnly

    Inherits TCollectionBase

    Public Sub New(ByVal ts As TCollection)

    MyBase.New(ts)

    End Sub

    Public Sub New(ByVal ts As TCollection, ByVal newItem As Object)

    Me.New(ts)

    al.Add(newItem)

    End Sub

    Public Overrides Function Add(ByVal item As Object) As TCollection

    Return New TCollectionReadOnly(Me, item)

    End Function

    Public Overrides ReadOnly Property isReadOnly() As Boolean

    Get

    Return True

    End Get

    End Property

    Public ReadOnly Property GetReadWrite() As TCollection

    Get

    Return New TCollectionReadWrite(Me)

    End Get

    End Property

    End Class

    Private Class TCollectionReadWrite

    Inherits TCollectionBase

    Public Sub New()

    MyBase.New()

    End Sub

    Public Sub New(ByVal ts As TCollection)

    MyBase.New()

    End Sub

    Public Overrides Function Add(ByVal item As Object) As TCollection

    al.Add(item)

    Return Me

    End Function

    Public Overrides ReadOnly Property isReadOnly() As Boolean

    Get

    Return False

    End Get

    End Property

    Public ReadOnly Property GetReadOnly() As TCollection

    Get

    Return New TCollectionReadOnly(Me)

    End Get

    End Property

    End Class

    End Class

  16. Why not just default the Collection classes to readonly or immutable and have a method that returns a mutable copy? So you would have a Hashtable class that is specialized by a HashtableMutable class that has a MutableCopy method.

    Just a thought, probably makes no sense.