Question

Seems like every C# static analyzer wants to complain when it sees a public field. But why? Surely there are cases where a public (or internal) field is enough, and there is no point in having a property with its get_ and set_ methods? What if I know for sure that I won't be redefining the field or adding to it (side effects are bad, right?) - shouldn't a simple field suffice?

Was it helpful?

Solution

Because it breaks encapsulation -- this is why most people use accessors heavily. However, if you think it's the right solution for your task, ignore it (meaning the strict encapsulation complaints) and do what's right for your project. Don't let the OO nazis tell you otherwise.

OTHER TIPS

It's really about future-proofing your code. When you say (emphasis mine):

What if I know for sure that I won't be redefining the field or adding to it (side effects are bad, right?) - shouldn't a simple field suffice?

That's an absolute statement, and as we know (as well as most static analyzers), there are only two absolutes in life.

It's just trying to protect you from that. If it is an issue, you should be able to tell the analyzer to ignore it (through attributes that are dependent on the analysis tool you are using).

Given the fact that current C# 3.0 allows for automatic properties whose syntax is like:

public int Property {get; set;}

the extra work required for using Properties over public fields is almost zero. The thing is you can never be completely sure a field won't be used differently or the accessor won't ever change and given the trade off in work there's no reason not to implement a property.

Anyway, the analyzer complains about things that in a high percentage (in this case like 99.99% of the cases) are bad programming practices... but anyway it is just complaining. Fields can be made public and there are some extreme cases where its direct use may be justified. As ever, use your common sense... but keep in mind the elemental rule for best programming practices ... Is there a really good reason to break the convention? If there's then go ahead, if not or if the answer is "it involves more work" then stick to the practice...

Because changing public fields later to have get/set accessors will break code. See this answer for more information

In general, it's a good idea to hide fields behind properties, even if you "know for sure" that you won't be redefining the field. All too often, what you "know for sure" today changes tomorrow. And, making a property to refer to a field is just a little bit of trouble.

That said, static analyzers are no substitute for thought. If you're happy with your design and in your judgement the analyzer is wrong, then ignore or (if possible) suppress that warning in that circumstance.

I think the point is that generally you don't know for sure that you won't be redefining the field or adding to it later. The whole point of encapsulating and hiding the data is that you are then free to do these things without changing the public interface and subsequently breaking dependent classes. If your property accessors are just simple get/sets then they'll be compiled down to that anyway, so ther are no performance concerns - given this your question should be is there any good reason not to use them?

One other benefit properties bring to the table is when doing Reflection. When you reflect over your class, you can get all the properties in one shot, rather than having to get the properties AND the fields.

And let's not forget that accessors give you flexibility when working with multiple threads.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top