Using ReadOnlyCollection and a Getter

Question

Yesterday I asked a question about deep cloning a list and I got a wonderful answer, which you can read here.

The issue I have is the answer uses ImmutableListand I've got no issue with it, it's just that if I wanted to use ReadOnlyCollection and ensure that I return a copy of my collection and that the one in the class cannot be modified.

I just wanted to know if the following is correct.

    private ReadOnlyCollection<Author> listofAuthors;   
    private List<Author> copyofAuthors;

    public Book(ICollection<Author> authors)
    {

        copyofAuthors = new List<Author>(authors);
        listofAuthors = new ReadOnlyCollection<Author>(new List<Author>(copyofAuthors));

    }

    public ICollection<Author> Authors
    {
        get
        {
            return new ReadOnlyCollection<Author>(new List<Author>(copyofAuthors));
        }
    }

According to MSDN documentation ReadOnlyCollection is just a wrapper for a underly mutable collection. So if any changes are made to the underlying collection it would be reflected in the ReadOnlyCollection. The above code the getter returns a new List as a readonly collection.

Question 1:

Given the above code, any other code calling it will get a copy of the private ReadOnly(new List()), correct? Any changes that the user makes will not be reflected in the ReadOnlyCollection inside the Book class, right?

Question 2:

I understand ImmutableList is more ideal, but if I needed to use ReadOnlyCollection<Authors> is what I've done in the Constructor/Getter correct? or can it be implemented in another/better way?

antiduh · Accepted Answer · 2016-03-08T20:26:20.787

Consider the following class:

public Book(ICollection<Author> origAuthors)
{
    // We only need to allocate the ReadOnlyCollection once.
    this.Authors = new ReadOnlyCollection<Author>( new List<Author>( origAuthors ) );

}

public ICollection<Author> Authors { get; private set; }

In this code, origAuthors stores a set of references to Author objects. That collection is modifiable by whoever provided it. I'm also assuming that Author is a class and not a struct, and thus has reference semantics.

This code makes a copy of the list of references to Author objects, wraps that list with a ReadOnlyCollection instance, and exposes that ReadOnlyInstance instance through our Book.Authors property.

Since the Book.Authors property is ultimately referring to a copy of the list of authors, if we remove an element from origAuthors, then Book.Authors will not reflect it; it will retain the set of Authors it had previously.

For instance:

void Test() {
    List<Author> mutableAuthors = new List<Author>();
    mutableAuthors.AddRange( ... add 5 authors ... );

    Book testBook = new Book( mutableAuthors );
    mutableAuthors.Clear();

    // Prints 5.
    Console.Writeline( testBook.Authors.Count );

    // Throws an exception from ReadOnlyCollection.
    testBook.Authors.Clear();
}

However, so far, all we've talked about are a set of lists that refer to Author objects; we've not talked to the Author objects themselves. Are they mutable?

If so, no matter how one retrieves a reference to an Author, it may be possible for them to modify the Author object itself. Using a ReadOnlyCollection<Author> only prevents you from modifying which items are in the list; it does not prevent you from modifying the items themselves.

Consider the following code:

void Test() {
    List<Author> mutableAuthors = new List<Author>();
    mutableAuthors.AddRange( ... add 5 authors ... );

    Book testBook = new Book( mutableAuthors );
    mutableAuthors.Clear();

    // Prints 5.
    Console.Writeline( testBook.Authors.Count );

    Author someAuthor = testBook.Authors[0];
    someAuthor.ChangeName("A new name");

    // Prints "A new name".
    Console.WriteLine( testBook.Authors[0] );
}

If you want to prevent this, then make sure your design for the Author class is immutable - once it's constructed, don't provide any means for consumers to modify it.

Just one question, provided that Authors is immutable, which it is, I can use the ReadOnlyCollection as you specified without worrying that the user can change the internal(Book) collection, right? Which the first part of your code seems to specify. — , Mar 08 '16 at 19:43
@Svetlana - Bingo. Since you make a copy of the list, your copy won't change when the original list is changed. Then you need to worry about making sure nobody modifies your copy of the list; you do that by exposing it through the `ReadOnlyList` wrapper class. Lastly, we need to make sure that the thing itself, not only the list of things, is immutable. But as you state, `Author` is immutable. So all of our bases are covered. — antiduh, Mar 08 '16 at 20:23
I would not recommend exposing concrete types (SOLID principles), instead change `ICollection` to `IReadOnlyCollection`. — Erik Philips, Mar 08 '16 at 20:29
@ErikPhilips - "I would not recommend exposing concrete types". Where do you see one such type? — antiduh, Mar 08 '16 at 21:17
@ErikPhilips - "change ICollection to IReadOnlyCollection". Sure, if you have .Net 4.5. I strived to change the asker's code as little as possible. — antiduh, Mar 08 '16 at 21:18
@ErikPhilips - Do you mean the ICollection in the getter or in the constructor? — , Mar 09 '16 at 12:30
@antiduh - Didn't you have IReadOnlyCollection in the getter, or am I not remembering correctly? if you did, please change it back, I like that approach better than ICollection. — , Mar 09 '16 at 12:48

Using ReadOnlyCollection and a Getter

1 Answers1