Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
2  
Have a look at this article about this preview of new immutable collections for .NET 4.5: blogs.msdn.com/b/bclteam/archive/2012/12/18/… You can already get them via NuGet. –  Kristof Claes Jan 29 '13 at 15:08
add comment

3 Answers

up vote 6 down vote accepted

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.

share|improve this answer
add comment

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.

share|improve this answer
4  
+1, using an IEnumerable<string> is exactly what one wants here. –  Doc Brown Jan 29 '13 at 15:28
    
+1, LINQ make it very easy to work with IEnumerable, so they can be used without fear most of the time. –  Laurent Bourgault-Roy Jan 29 '13 at 15:31
    
Thank you, I did not realize that IEnumerable<> is immutable. Your solution is very good, however the question was aimed at general approach to objects (not just collections). That is why I think Doc Brown answer is better. –  NeverStopLearning Jan 29 '13 at 16:14
2  
In .Net 4.5, you can also use IReadOnlyList<T>. –  svick Jan 29 '13 at 16:24
2  
Note for other interested parties. When considering the property accessor: if you do just return IEnumerable<string> instead of List<string> by doing return _list (+implicit cast to IEnumerable) then this list can be cast back into List<string> and changed and the changes will propagate into the main object. You can do return new List<string>( _list ) (+subsequent implicit cast to IEnumerable) which will protect the internal list. More on this can be found here: stackoverflow.com/questions/4056013/… –  NeverStopLearning Jan 29 '13 at 16:37
show 1 more comment

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

share|improve this answer
1  
Problem with this is, that its possible to cast it back to mutable version. And its violiation of LSP, bacause state observable through base class's (in this case interface's) methods can be changed by new methods from subclass. But it at least comunicates intent, even if it does not enforce it. –  user470365 Jan 29 '13 at 15:25
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.