What kind of property is this? is it necessary?

Question

When creating a mapping, I am reading that your collection property should look like:

  public virtual ReadOnlyCollection<Product> Products
  {
           get { return new ReadOnlyCollection<Product>(new List<Product>(_products).AsReadOnly()); }
  }

Why does it have to be like this? it seems to be returning a new collection everytime it is referenced?

Answer 1

I believe it is recommending to build it like this so you can't modify the list in the calling code. Normally you'd be able to manipulate it (since it's going to be pass-by-reference) -- add items, delete items, etc. This ensures you have to use the setter to change the internal list.

I wouldn't say it has to be like that at all, unless you want your getters to return read-only lists.

Answer 2

It's returning a wrapper class instance that prevents callers from being able to directly modify the collection you are returning.

If you were to simply return the underlying list, any caller would be able to alter it in ways that may break the class that actually owns the list.

Even if you returned your list as a read-only interface (say IEnumerable, or ICollection), nothing prevents a caller from performing a run-time cast, and getting at the list.

By returning a wrapper object, you can prevent the caller from ever being able to alter the list. The wrapper does not expose any methods that allow the underlying list to be altered, and attempting to cast the wrapper object will fail. The wrapper does not duplicate the data - it merely keeps a reference to the list, and prevents write operations.

In the case of ORM mappings, this allows the object-model to control at which access point you can alter a relationship between objects.

Answer 3

It is specifically returning a new collection that cannot be modified. Seems a little goofy though. Unless I'm mistaken, it could be:

public virtual ReadOnlyCollection<Product> Products
{
    get
    {
        return new List<Product>(_products).AsReadOnly();
    }
}

or, if _products is some sort of List<Product> already:

public virtual ReadOnlyCollection<Product> Products
{
    get
    {
        return _products.AsReadOnly();
    }
}

Answer 4

This code seems redundant to me... why not just return _products.AsReadOnly() ? (assuming _products is a List<T>, array or any type that exposes an AsReadOnly method)

Answer 5

Your code looks a bit odd. First it creates a copy of _products, then it makes it read only, and then it wraps it in a ReadOnlyCollection again!

If you want to expose a collection that should be read-only, do something like this:

private List<Product> _products = new List<Product>();

private ReadOnlyCollection<Product> _readonlyProducts =
    new ReadOnlyCollection(_products);

public ReadOnlyCollection<Product> Products
{
    get
    {
        return _readonlyProducts;
    }
}

No need to recreate the ReadOnlyCollection each time (or copy or or double-wrap the collection).

Answer 6

Your collections don't have to always be read only. It depends on exactly what the list is for. If it is really just a list for referencing then you can even return IEnumerable rather than ReadOnlyCollection unless you explictly need a readonly collection.

To make it a readonly collection I would do:

private List<Product> products = new List<Product>();

public ReadOnlyCollection<Product> Products { get { return products.AsReadOnly(); } }

There is no need to wrap the AsReadOnly method with a new ReadOnlyCollection statement. Alternatively you could do:

public ReadOnlyCollection<Product> Products { get { return new ReadOnlyCollection<Product>(products); } }

However, I would just go with calling AsReadOnly as I think internally it will just be wrapping your list up for you anyway.

Answer 7

I suppose that _products is ICollection<Product> or IEnumerable<Product> - in this case I think it is enough to have new List<Product>(_products).AsReadOnly(). If _products is IList<Product> then new ReadOnlyCollection<Product>(_products) is enough. The decision whether to use this depends on the design of the class - in some cases it is even better to return collection adapter that hides every Product into a read-only ProductView or ProductDTO instance.