I have “best practices” question about OOP in C# (but it sort-of applies to all languages).
Consider having library class with object that is to be exposed to public, say via property accessor,
but we do not want the public (people using this library class) to change it.
class A
{
// Note: List is just example, I am interested in objects in general.
private List<string> _items = new List<string>() { "hello" }
public List<string> Items
{
get
{
// Option A (not read-only), can be modified from outside:
return _items;
// Option B (sort-of read-only):
return new List<string>( _items );
// Option C, must change return type to ReadOnlyCollection<string>
return new ReadOnlyCollection<string>( _items );
}
}
}
Obviously the best approach is “option C”, but very few objects have ReadOnly variant (and certainly no user-defined classes have it).
If you were the user of class A, would you expect changes to
List<string> someList = ( new A() ).Items;
propagate to original (A) object?
Or is it okay to return a clone provided that it was written so in comments/documentation?
I think that this clone approach might lead to quite difficult-to-track bugs.
I remember that in C++ we could return const objects and you could only call
methods marked as const on it. I guess there is no such feature/pattern in the C#? If not why would they not include it? I believe it is called const correctness.
But then again my main question is about “what would you expect” or
how to handle Option A vs B.
1
Besides @Jesse’s answer, which is the best solution for collections: the general idea is to design your public interface of A in a way so that it does return only
-
value types (like int, double, struct)
-
immutable objects (like “string”, or user defined classes which offer only read-only methods, just like your class A)
-
(only if you must): object copies, like your “Option B” demonstrates
IEnumerable<immutable_type_here> is immutable by itself, so this is just a special case of the above.
Also note you should be programming to interfaces, and by and large, what you want to return from that property is an IEnumerable<string>. A List<string> is a bit of a no-no because it is generally mutable and I’ll go so far to say that ReadOnlyCollection<string> as a implementor of ICollection<string> feels like it violates the Liskov Substitution Principle.
9
I encountered a similar problem a while back and below was my solution, but it really only works if you control the library too:
Start with your class A
public class A
{
private int _n;
public int N
{
get { return _n; }
set { _n = value; }
}
}
Then derive an interface from this with only the get portion of the properties (and any reading methods) and have A implement that
public interface IReadOnlyA
{
public int N { get; }
}
public class A : IReadOnlyA
{
private int _n;
public int N
{
get { return _n; }
set { _n = value; }
}
}
Then of course when you want to use the read only version, a simple cast is sufficient. This is just what you solution C is, really, but I thought I’d offer the method with which I achieved it
1
For anyone who finds this question and still is interested in the answer – now there is another option which is exposing ImmutableArray<T>. These are few points why I think it’s better then IEnumerable<T> or ReadOnlyCollection<T>:
- it clearly states that it is immutable (expressive API)
- it clearly states that collection is already formed (in other words it’s not initialised lazily and it’s OK to iterate throught it multiple times)
- if the count of items is known, it does not hide that knowlegde like
IEnumerable<T>does - since client doesn’t know what is the implementation of
IEnumerableorIReadOnlyCollecthe/she cannot assume that it never changes (in other words there is no guarantee that items of a collection have not been changed while reference to that collection stays unchanged)
Also if APi needs to notify client about changes in the collection I would expose an event as a separate member.
Note that I’m not saying that IEnumerable<T> is bad in general. I’d still use it when passing collection as an argument or returning lazily initialised collection (in this particular case it makes sense to hide item count because it’s not known yet).