1

I know how properties are used to expose fields, what are the benefits and so on. What I am trying to understand (and I've searched quite a bit) is:

Does every single field need to always have a wrapping property, even if it does NOT need to be exposed and is used only in internal logic?

I would assume that the answer is yes, because even if its modified in some internal logic, the new value would still need to be validated.

If the answer is yes, is this why its so common for a property to have public get and private set?

Edit: Thank you for all the answers that you gave me, but most of them have nothing to do with my question, most of you are talking about big and complex classes, my simple question is, should I have a wrapping property for a field thats used only inside its class and does not need to be exposed.

Edit 2: Some of the answers suggest something that can be accepted as a no answer, suggesting that a property should not be used if there is no need for logic

Edit 3: Thank you all, I reread most of the comments and it started making sense, just like always :)

Darkbound
  • 3,026
  • 7
  • 34
  • 72
  • Well, you can choose how you will use field in the every situation. If you have a few different internal logics for filed you can avoid to use property and add some validation' methods, of course, property is methods and you can wrap field to a few properties, but I think that it's redundantly and unclear – George Alexandria Sep 21 '17 at 22:34
  • 2
    No, don't make it a property unless you need to put some logic in the setter or getter (though the getter is less likely). If the class is so huge that nobody can keep track of it all and you need to hide parts from each other, it's too big and it's doing too much. Refactor it into two or more classes that are manageable. – 15ee8f99-57ff-4f92-890c-b56153 Sep 21 '17 at 22:51
  • Most of the times you'd want to use field when dealing with private data or constants used inside of the class and you'd want to use property if access to the field should be exposed outside of the class. – Fabjan Sep 21 '17 at 22:54
  • They're a few good approach when you should use property instead of method or shouldn't: [answer 1](https://stackoverflow.com/a/601648/5359302) and [answer 2](https://stackoverflow.com/a/1374291/5359302) – George Alexandria Sep 21 '17 at 22:54

3 Answers3

2

This isn't one of those issues where you can do a whole lot of damage getting it wrong.

That said, no, don't make it a property unless you need to put some logic in the setter or getter, and you probably don't need to. Don't add code that doesn't serve some purpose.

"Fields are bad" isn't a purpose. And in fact, it's nonsense. Fields exist for a reason.

If the class is so huge that nobody can keep track of it all and you need to hide parts from each other, it's too big and it's doing too much. Refactor it into two or more classes that are manageable.

Very often, as a large class grows (most often something that doesn't naturally have a very tight focus, like a major viewmodel, or Window or a Form in the bad old days), we find ourselves writing little "subsystems" within the class that have a field or two with logic in the set and/or get blocks. That's a candidate for refactoring into a separate small class with a clean interface, an easily reusable ball of logic and state that doesn't have its internals mixed in with your big class's internals. Give the big class a private copy of the little helper class. Drag and drop code is an example that springs to mind.

It's a classic progression:

  1. I'll just add a flag
  2. That flag I added yesterday has to be an enum
  3. With a flag
  4. Wait, I just copy and pasted a block of code. Put that in a setter on the flag.
  5. And a hel... two helper methods.
  6. Let's put a #region around that mess so I don't have to look at it.
  7. We're going to have to copy and paste that whole mess into another class. Good thing it's all in one place!
  8. It wasn't all in one place.
  9. What's my manager doing with that jacket with the funny sleeves?

If you ever reach Stage 7, it's past time for an intervention.

Private properties are just the first tiny whiff of what may turn into a noxious code smell over time. They're not evil, but they may be a sign that your code is showing a tendency to grow on the wrong axis.

Update

On internal vs external validation

Very often, a class needs to be able to break its own rules, and do so cleanly and straightforwardly. For example, validation of one property/field often depends on values of others (date ranges, etc.). Your classe's internals should be able to set them all in arbitrary order without any funny business (sometimes you see "_disableValidation" flags to work around this issue -- a code smell). Inside the class, validation should be voluntary, and the class should be kept simple enough so that isn't a problem. A class needs to permit itself to put itself in an invalid state temporarily, while forbidding or controlling the ways in which any external code can put it in an invalid state. Maybe if external code puts it in an invalid state, that's permitted by code, but it drives UI that hassles the user. You don't want to have weird flags to allow your constructor to do its job without popping up a message box or something.

Sometimes a class needs to run around nekkid in the privacy of its own home. Validation should be on inputs from code external to the class.

Community
  • 1
  • 1
  • Thank you, I have one more question that pretty much follows the previous one. If a field needs to be validated before a value is assigned to it, even if its from internal logic, should it go through a property even if it doesnt need to be exposed to the outside world, or should there be a logic in place where it is being assigned. Lets say that we have the x field and we want to change its value in the method ChangeXValue, should the validation logic (assuming its needed) go in that method, or in a X property? – Darkbound Sep 21 '17 at 23:19
  • @Darkbound Good question. Very often, a class needs to be able to break its own rules. For example, validation of one property/field often depends on values of others (date ranges, etc.). Your classes internals should be able to set them all in arbitrary order without any funny business (sometimes you see "_disableValidation" flags to work around this issue -- a code smell). Inside the class, validation should be voluntary, and the class should be kept simple enough so that isn't a problem. A class needs to permit itself to put itself in an invalid state temporarily, while... – 15ee8f99-57ff-4f92-890c-b56153 Sep 21 '17 at 23:24
  • @Darkbound ...forbidding or controlling the ways in which any external code can put it in an invalid state. Part of a programmer's job is to make it safe for a class to run around nekkid in the privacy of its own home. Validation should be on inputs from code external to the class. – 15ee8f99-57ff-4f92-890c-b56153 Sep 21 '17 at 23:25
  • If I understand this correctly, there is no need for internal validation, meaning that when working with internal logic we should work only with the fields and properties should not be part of the internal logic? – Darkbound Sep 22 '17 at 09:45
  • @Darkbound You can absolutely quote me! Thanks. I'd hesitate to say there is *never* a need for internal validation -- never say never. But every time I do it I regret it. – 15ee8f99-57ff-4f92-890c-b56153 Sep 22 '17 at 11:05
  • One last clarification, what about constructors? The data in this case is coming from an external source to the constructor (when we instantiate the class somewhere), should it go through a property in this case? I mean, when we assign the values inside the constructor, should it be constructor->property->field or constructor->field – Darkbound Sep 22 '17 at 18:54
  • @Darkbound Depends. A clean way to do stuff is: Have one method that does all the validation. Set all the private fields, whether or not they have public accessor properties. Then once you're in a complete consistent state and ready to go, then you call the validation method. If you write a class that can privately break its own rules when it needs to, you don't have to be so clever in writing the internals, which is an important goal. – 15ee8f99-57ff-4f92-890c-b56153 Sep 22 '17 at 20:10
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/155122/discussion-between-darkbound-and-ed-plunkett). – Darkbound Sep 22 '17 at 21:18
1

There's no need to do it but by using a property instead of a field you're more flexible because you can preprocess getting and setting a variable. Changing a field to a property while it's already in use in other code can be a breaking change. So if you don't know what to choose, go with a property to be on the safe and most flexible side.

Measurity
  • 1,316
  • 1
  • 12
  • 24
-1

YAGNI

if you don't use property - do not create it.
If you need change field some how or change logic how it created/updated you will be able to do it without property.

Because you do not expose it - you don't need to worry about breaking some other code.

if you have some "complex" logic which instantiate/update that field - create a private methods for it.

Putting some "extra" logic in the setter or even worth add some "side-effect" on the getter - can lead in some odd behavior or bugs, because when I initialize class

var order = new Order
{
    Id = 1001,
    Reference = "Reference one",
    CustomerId = 2001        
}

I do not expect that assigning CustomerId will cause in some database queries or execution of some complex logic. Instead

var order = new Order
{
    Id = 1001,
    Reference = "Reference one"      
};
order.SetCustomer(customerid);
Fabio
  • 31,528
  • 4
  • 33
  • 72