Amazon.com Widgets

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? 


 

 

Published 04 February 04 11:16 by BradA

Comments

# RichB said on February 4, 2004 11:54 PM:
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.
# Alan McBee said on February 5, 2004 12:36 AM:
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.
# notbrainsurgery said on February 5, 2004 1:01 AM:
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).

# Lee said on February 5, 2004 1:42 AM:
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.
# Dominic Morris said on February 5, 2004 2:43 AM:
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.
# Olivier Le Pichon said on February 5, 2004 3:25 AM:
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.
# Michael said on February 5, 2004 3:58 AM:
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.
# Ben said on February 5, 2004 5:55 AM:
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.
# Ben said on February 5, 2004 6:11 AM:
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.
# Marc Scheuner said on February 5, 2004 6:37 AM:
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...
# Jim Argeropoulos said on February 5, 2004 6:59 AM:
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.
# Patrick Cauldwell said on February 5, 2004 9:13 AM:
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.
# Keith Patrick said on February 5, 2004 11:14 AM:
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++)
# Nicolai Kollner said on February 5, 2004 4:34 PM:
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.
# James Roe-Smith said on February 5, 2004 6:58 PM:
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
# Jeff Key said on February 5, 2004 11:41 PM:
# Jeff Key said on February 5, 2004 11:41 PM:
# Ben S. Stahlhood II said on February 7, 2004 8:26 AM:
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.
# Hackward and Foreword said on April 16, 2005 12:06 PM:
# Hackward and Foreword said on May 10, 2005 9:50 PM:
New Comments to this post are disabled

Search

This Blog



These postings are provided "AS IS" with no warranties, and confer no rights.



Ads by Lake Quincy Media

Syndication

Page view tracker