views:

179

answers:

1

I've just started to work with FxCop to see how poorly my code does against its full set of rules. I'm starting off with the "Breaking" rules, and the first one I came across was CA2227, which basically says that you should make a collection property's setter readonly, so that you can't accidentally change the collection data.

Since I'm using MVVM, I've found it very convenient to use an ObservableCollection with get/set properties because it makes my GUI updates easy and concise in the code-behind. However, I can also see what FxCop is complaining about.

Another situation that I just ran into is with WF, where I need to set the parameters when creating the workflow, and I'd hate to have to write a wrapper class around the collection I'm using just to avoid this particular error message.

For example, here's a sample runtime error message that I get when I make properties readonly:

The activity 'MyWorkflow' has no public writable property named 'MyCollectionOfStuff'

What are you opinions on this? I could either ignore this particular error, but that's probably not good because I could conceivably violate this rule elsewhere in the code where MVVM doesn't apply (model only code, for example). I think I could also change it from a property to a class with methods to manipulate the underlying collection, and then raise the necessary notification from the setter method. I'm a little confused... can anyone shed some light on this?

+6  A: 

What this specific rule tells is that a collection property should be made read-only because you don't need to assign a whole collection to a property.

For instance, imagine a class like this:

public class Foo
{
   public ObservableCollection<int> Bar { get; set; }
}

What would happen if somewhere in the code I have the following line:

var f = new Foo();
f.Bar = new ObservableCollection<int>();
f.Bar.AddRange(new int[] { 1, 2, 3, 4 });
// ...
// Attaches and handlers to the collection events
// ...
f.Bar = new ObservableCollection<int>();
f.Bar.AddRange(new int[] { 5, 6, 7, 8 });

When the last two lines of code are executed the attached event handlers would not be fired, because the Bar property has a complete different object.

On the other hand, if the property were read-only the events would be fired and everything would behave as expected.

Paulo Santos
that's good to know, thank you for the explanation. However, this doesn't help in the Windows Workflow situation. I'm going to update my question with a specific runtime error message.
Dave
Why not simply ignore this reported item? You can use System.Diagnostics.CodeAnalysis.SuppressMessage attribute to ignore such false alarms.
Lex Li
@Lex: I'm doing that now for those specific instances. On a slightly related note, I've been having issues with SuppressMessage in a specific (global) case. Do you have any suggestions for that? http://stackoverflow.com/questions/2542883/globally-disabling-fxcop-errors-in-teamcity
Dave
I also came across something similar that could not be resolved. So finally I gave up and let it be. :)
Lex Li