How can I make a read-only ObservableCollection property?

Question

I'd like to expose a property on a view model that contains a list of objects (from database).

I need this collection to be read-only. That is, I want to prevent Add/Remove, etc. But allow the foreach and indexers to work. My intent is to declare a private field holding the editable collection and reference it with a read-only Public Property. As follows

public ObservableCollection<foo> CollectionOfFoo { 
     get { 
         return _CollectionOfFoo;
     }
}

However, that syntax just prevents changing the reference to the collection. It doesn't prevent add/remove, etc.

What is the right way to accomplish this?

Solution 1

The [previously] accepted answer will actually return a different ReadOnlyObservableCollection every time ReadOnlyFoo is accessed. This is wasteful and can lead to subtle bugs.

A preferable solution is:

public class Source
{
    Source()
    {
        m_collection = new ObservableCollection<int>();
        m_collectionReadOnly = new ReadOnlyObservableCollection<int>(m_collection);
    }
 
    public ReadOnlyObservableCollection<int> Items
    {
        get { return m_collectionReadOnly; }
    }
 
    readonly ObservableCollection<int> m_collection;
    readonly ReadOnlyObservableCollection<int> m_collectionReadOnly;
}

See ReadOnlyObservableCollection anti-pattern for a full discussion.

Solution 2

I don't like using ReadOnlyObservableCollection<T> as it seems like a mistake / broken class; I prefer a contract based approach instead.

Here is what I use that allows for covarience:

public interface INotifyCollection<T> 
       : ICollection<T>, 
         INotifyCollectionChanged
{}

public interface IReadOnlyNotifyCollection<out T> 
       : IReadOnlyCollection<T>, 
         INotifyCollectionChanged
{}

public class NotifyCollection<T> 
       : ObservableCollection<T>, 
         INotifyCollection<T>, 
         IReadOnlyNotifyCollection<T>
{}

public class Program
{
    private static void Main(string[] args)
    {
        var full = new NotifyCollection<string>();
        var readOnlyAccess = (IReadOnlyCollection<string>) full;
        var readOnlyNotifyOfChange = (IReadOnlyNotifyCollection<string>) full;


        //Covarience
        var readOnlyListWithChanges = 
            new List<IReadOnlyNotifyCollection<object>>()
                {
                    new NotifyCollection<object>(),
                    new NotifyCollection<string>(),
                };
    }
}

Comments

[Charlie Salts]

Remember, though, if CollectionOfFoo is say, a List<foo>, and the caller knows this, it can cast CollectionOfFoo back to List<foo>, and do add/remove operations. This works, but its not bullet-proof.

[DVA]

Note thought that INotifyCollectionChanged changed of ReadOnlyObservableCollection<T> is protected, and thus to access the event hookup you need to cast explicitly to INotifyCollectionChanged. An alternative would be to extend the ROOC to expose that event publicly (which it should have in the first place imho) and file a MSDN bug report on the current implementation. :)

[Oskar]

@GaussZ Yes nice catch, why is it not public? It makes no sense at all to me. I might extract this into a new SO question to see if anyone else has some insight on this.

[Bradley Grainger]

As @Eric J. has pointed out, returning a new ReadOnlyObservableCollection every time the property is accessed makes it difficult for clients to correctly unsubscribe from events (which is presumably why it's a ReadOnlyObservableCollection, and not just a ReadOnlyCollection).

[Jake Berger]

Binding to an ItemsSource. This answer works, but should be avoided.

[Søren Boisen]

Indeed the very existance of ReadOnlyObservableCollection is a giant design smell. ObservableCollection should have implemented a read-only combined view interface from the get-go.

[tehDorf]

You could remove the need for m_collectionReadOnly by making Items an auto-property with a private setter, and just set it once in the constructor. Then you can have code that is a little cleaner and you don't have to worry about creating a new instance each time it is accessed.

[Eric J.]

Now you can, though I'm not sure that private setters for automatic properties were available when I wrote the answer. However, using readonly enforces an even stricter contract than what you suggest. If you have a private setter, other class methods can still theoretically change the value over the lifetime of the object. While this may seem unlikely in a simple class you are writing today, keep in mind that code often lives on for many years, with many maintainers. For that reason, I favor the least permissive contract.

[LuckyLikey]

@SørenBoisen i agree ReadOnlyObservableCollection should be some kind of IReadOnlyObservableCollection somewhere in the interitance hierarchy

[LuckyLikey]

@jberger link is broken

[Jake Berger]

@LuckyLikey try ItemsSource Examples section

[Peter Duniho]

Very nice, elegant. I like this much better than using the ReadOnlyObservableCollection<T> class. That said, it is worth noting that one advantage of using ReadOnlyObservableCollection<T> is that the type itself simply does not support modification. With this solution, you are not protected from careless coders who simply cast the instance back to a writeable type (i.e. NotifyCollection<T>, INotifyCollection<T>, IList<T>, etc.). And trust me, in any large enough team, there's at least one person who would do that if they can.

[Dhir Pratap]

This is an abuse of inheritance and should be avoided (liskov substitution).

[Joep Beusenberg]

Using C#6 you can now also use implicit readonly setters ...{ get; }, only assignable from within the constructor. Then you have best of both.