tags:

views:

782

answers:

9

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?

+17  A: 

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.

Cody Brocious
Um, wtf? Are you saying that accessors provide encapsulation? o_O
That's their aim. Accessors encapsulate the inner workings of the object from the outside.
Cody Brocious
Encapsulation is information hiding. With a public field you can do foo.bar=baz, with a public property you can do foo.bar=baz. Seeing that there's no difference in syntax and in contract, accessors do not hide information.
So if you have a property transparently fetch/store data in a database, that isn't encapsulation just because it's accessible from the outside? I think you need to recheck your definition of encapsulation...
Cody Brocious
Ignore Iraimbilanja comment please, Cody is right.
pmlarocque
Shouldn't a database fetch be a *method*, not a *property get*?
Dmitri Nesteruk
Dmitri, Not necessarily, but it was just an example. For what it's worth, property gets/sets are really just methods, and although it may not be a good idea to do queries in them in most cases, there's no reason this is an inherently Bad Thing (TM).
Cody Brocious
Cody, accessors that do more than handle a member field clearly do hide information. That doesn't apply to the vast majority of accessors though - they are not in any way encapsulation, they merely model the machine's limitations. Encapsulating accessors are the exception rather than the rule.
Iraimbilanja, even ignoring that many accessors simply backend to a field, saying they don't provide encapsulation is nonsense. If you want to change how the data is handled internally, accessors allow you to do that. How is that not encapsulation?
Cody Brocious
Accessors provide a level of indirection. Most times you'll just see get; set;, but there are times that you really need to hide internal details. If you use Accessors first, you don't have to add a crapload of code. If you don't, you're going to have to, or something will break.
George Stocker
Point taken. When taking into account future maintenance and how it requires to have implementation information, accessors do provide encapsulation. I'm only unsure where people who read your answer interpreted it correctly.
Encapsulation is not hiding information. Encapsulation is hiding *implementation*. The idea is that as a client of the object you don't want to care how its properties are stored, may them be via backing fields, via access to other aggregated objects, via direct access to the database (no matter how absurd this might seem). Properties provide precisely that abstraction layer. They also allow very practical things such as validations.
Rui Craveiro
+15  A: 

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).

casperOne
+5  A: 

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

Binary Worrier
+2  A: 

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.

Jim Mischel
+1  A: 

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?

Stu Mackellar
+11  A: 

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...

Jorge Córdoba
I've actually run into one case where it was expedient to make a public field: exposing a counter for Interlocked.Increment, which needs a genuine ref. Even then, I could have instead exposed my own Increment method, so it wasn't absolutely necessary.
Steven Sudit
+1  A: 

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.

BFree
If I have only fields, well, that works too.
Dmitri Nesteruk
A: 

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

Peter LaComb Jr.
A: 

I also think that sometimes a public/internal field is enough. As far as I know, properties are not "inlined" when compiling x64 applications. Isn't that more than a reason for using public fields in .NET critical applications?

Leo Arnt