0

I have the following pattern:

public class Test
{
    public ICollection<string> Stuff { get; private set; }

    public Test()
    {
        Stuff = new List<string>();
        Stuff.Add("Initial Item");
    }
}

FxCop is complaining with CA2227:CollectionPropertiesShouldBeReadOnly. Why? The setter is private so it's no different to having a private field, except it's a shorter and neater syntax.

This is a very common pattern for me, and I really don't want to have to Suppress each warning individually or replace it with a field + property pattern.

Is there a way around this?

Richard Payne
  • 81
  • 1
  • 12
  • I cannot reproduce the problem. Which version of FxCop and/or Visual Studio are you using? Which version of the .NET Framework does is your assembly built against? – Nicole Calinoiu Apr 11 '14 at 17:21
  • VS 2010 Express, FxCop 10 and .Net 4.0 I think. – Richard Payne Apr 12 '14 at 06:54
  • Hmmm... I tried this with FxCop 10.0 against an assembly built in VS 2010 and targeting .NET 4.0, and there was no CA2227 violation. Do you see a violation for the exact `Test` class you posted? – Nicole Calinoiu Apr 12 '14 at 19:33
  • no, and it's suddenly started working on my main project too. I have no idea what was going on there. :( – Richard Payne Apr 15 '14 at 08:37

2 Answers2

4

EDIT:

Yes making a private set should be like readonly, but I guess the FxCorp doesn't consider that. You can ignore the rule or you can have a backing private field with readonly. I think this is some of the case where you can safely ignore the FxCorp rule. They are more like a guidelines anyway.

Another way could be to have private ICollection<string> and then expose your own method for adding and removing items from the collection. This will remove the warning from FxCorp.

Old Answer:


By having a private set, you are preventing it to be initialized / set to a new value from outside the class, but you are not preventing anyone to add anything in the collection. From outside the class you can still do:

Test t = new Test();
t.Stuff.Add("Something");

Thus your collection is not read-only.

You can also see this post from Jon Skeet about exposing a collection as a property

Community
  • 1
  • 1
Habib
  • 219,104
  • 29
  • 407
  • 436
  • That's not what the rule relates to: "Properties that return collections should be read-only so that users cannot entirely replace the backing store. Users can still modify the contents of the collection by calling relevant methods on the collection." – Richard Payne Apr 11 '14 at 14:00
  • @RichardPayne, what exactly is the FxCorp Error and its number ? – Habib Apr 11 '14 at 14:01
  • The error description is in the title. The number is CA2227. – Richard Payne Apr 11 '14 at 14:21
  • @RichardPayne, I see now, yes making a private `set` should be like `readonly`, but I guess the FxCorp doesn't consider that. You can ignore the rule or you can have a backing private field with `readonly`. I think this is some of the case where you can safely ignore the FxCorp rule. They are more like a guidelines anyway. – Habib Apr 11 '14 at 14:31
  • Yeah, I know they're guidelines, but generally they seem to be good guidelines so I've been trying to make sure it comes up clean, either by fixing the code of suppressing the rules on an individual instance basis. This is the first one I've come across that just seems wrong, which is a shame. – Richard Payne Apr 12 '14 at 06:53
-3

Issue seems to have resolved itself.

Richard Payne
  • 81
  • 1
  • 12
  • This cannot be the accepted answer. How does it improve the knowledge of the community? Better delete it. – Miro J. Oct 29 '19 at 14:12
  • What do you want me to delete? The answer, or the question? – Richard Payne Feb 17 '20 at 11:50
  • This answer has no value because no one will know how to replicate it to solve a similar problem. – Miro J. Feb 18 '20 at 13:31
  • It had the value of letting people know not to bother investigating further since the problem stopped of its own accord. Since I don't know how to replicate it, I'm not sure what else I should have said instead? – Richard Payne Feb 18 '20 at 13:35