views:

317

answers:

2

In my VS2008 SP1, .NET 3.5 SP1 project, I have different classes that contain different properties. I use C#3.0 auto properties a lot.

Some of these properties need to be collections. Since I want to make it simple, I use ReadOnlyCollection<T> for these properties.

I don't want to use IEnumerable<T> since I want random access to the elements.

I use Code Analysis (FxCop rules) and I get the CA2227 warning.

I don't understand why does ReadOnlyCollection<T> should have a set method while it can't be changed... The set method can only do exactly what the property can do.

Example:

using System.Collections.ObjectModel;

namespace CA2227
{
    public class MyClass
    {
        public ReadOnlyCollection<int> SomeNumbers { get; set; }
    }
}

CA2227 : Microsoft.Usage : Change 'MyClass.SomeNumbers' to be read-only by removing the property setter. C:\Users...\Visual Studio 2008\Projects\CA2227\MyClass.cs 7 CA2227

A: 

A ReadOnlyCollection cannot be changed, but there's no reason why a property with a setter that is of type ReadOnlyCollection can't be changed to refer to a different ReadOnlyCollection. If you want the SomeNumbers property to be immutable, then it needs to be both of a read-only type, and also have a non-public setter.

EDIT

If you're convinced in what you want, then although FxCop is correct to warn you, you are happy with the warning. If you want to get rid of it, then include a SuppressMessage attribute at that point - as long as you also define a CODE_ANALYSIS constant in the project properties before you build, FxCop will honour that attribute and just not issue that particular warning on that particular occasion.

David M
I want to be able to change the value of the property.I don't want to get the CA2227 warning.I also want to understand why I'm getting the CA2227 warning and whether should I write the code differently.
brickner
Well, at the end of the day it's a warning that what you are doing is unusual and may be incorrect. If you're convinced you're right - see my edit...
David M
I'm not convinced.I want to understand why do I get this warning for ReadOnlyCollection<T> and not for IEnumerable<T>.
brickner
A: 

It's rather odd to block changes to the contents of the collection without also blocking changes to the collection itself. If you want to be able to set the collection from within your class while conserving the use of automatic properties, you could use a private setter. e.g.:

public ReadOnlyCollection<int> SomeNumbers { get; private set; }
Nicole Calinoiu
Why is this so odd?Why is setting an IEnumerable<T> (which also can't be modified) isn't odd and doesn't give CA2227?I want the setter to be public. I love using object initializers - which don't work with private setters.
brickner
It's also unusual to allow setting of an IEnumerable<T> property, but at least the intent is fairly clear when one does so, and the class implementer should presumably be aware of the implications.I suppose one could argue the same regarding a ReadOnlyCollection<>, but the FxCop rule as it is currently written does not examine which ICollection or ICollection<> implementation you are exposing.[continued in next comment]
Nicole Calinoiu
I suspect that your code is probably not safe for truly "public" use. Unless you're willing to invest in checking that allowing setter invocation at any time will not cause unexpected side effects, you may wish to reduce the setter visibility to internal, which should get rid of the FxCop violation in most cases.
Nicole Calinoiu
I'm pretty sure that setting a ReadOnlyCollection or will not cause unexpected side effects that can't be caused by setting an IEnumerable.
brickner