public fields

jrh

Dec 6, 2016 16:59

I was reading Code Complete and I noticed that the author said (I'm going off of memory here)

Prefer properties over [non-private] fields because properties give you the ability to change what the property does without effecting the interface.

. I've read this before in many places; I tend to agree for inheritable classes, but if the class is final, is there any problem with just using a field? Isn't adding additional validation (and exceptions) changing the "interface", silently?

Also, using ListViewItem in Winforms as an example. Let's say ListViewItem.Text did not originally redraw the ListView when an item's Text property was set, and a later version did, it seems like a (semi) reasonable thing for the ListViewItem.Text property to do but it could still break things and be a jarring change.

ratchet freak

Dec 6, 2016 17:15

very often you want to be able to substitute a new implementation for the old one in a way that old code doesn't need to be altered. Then it is very good when the old interface allows the new one to get substituted in.

Robert Harvey

@AwokeKnowing: I rolled back your edit. There are very good reasons why we removed the request for a specific pattern name. We're not a dictionary; we'd prefer to focus on the design issues here, not refer you (and every other person encountering this post) somewhere else. If that results in a well-known pattern name, so much the better. — Robert Harvey 1 min ago

jrh

@ratchetfreak Do you have an example in mind of where the implementation changed without affecting old code? A real one if you can.

Robert Harvey

@jrh Note that, in C# (and probably in other languages as well), changing a public field to a property breaks backward compatibility. That's why we favor public properties from the start; if you add validation later, the binary contract is preserved.

ratchet freak

such changes don't tend to be really visible to user code

it's much more common to find times where the change broke old code and people had to spend time and money to fix it.

jrh

If it makes a difference, this is in an application, not a library, so the only client code is code within the same assembly

ratchet freak

Dec 6, 2016 17:20

but the runtime is a library which at runtime may not be the same you compiled against

Robert Harvey

Most programmers consider public fields a violation of encapsulation; you're exposing the class's implementation details. With a property, at least you're formalizing that exposure, even if it doesn't contain validation code.

jrh

@ratchetfreak sorry, I don't quite follow, are you saying the application is its own library, or are you referring to the .NET runtime?

ratchet freak

.NET runtime

Robert Harvey

For what it's worth, I don't think that whether it's a library or an application matters.

A class is a class.

jrh

@RobertHarvey What if the field isn't an implementation detail though, and any value of the field is valid (e.g., a bool)

Or are fields always considered implementation details?

Robert Harvey

Dec 6, 2016 17:24

If it's public, it's more semantically correct to make it a property; the programmer coming after you will divine your intent better.

ratchet freak

but what the implementation changes so that it's cheaper to recalc the bool as needed instead of keeping track of it and changing as needed

if you make it a field you are stuck in the "keep it synced" pattern

Robert Harvey

Yes, it's (almost) always better to separate the implementation from the external API.

There are edge cases, like game programming where every clock cycle matters, but those almost certainly don't apply here.

amon

@RobertHarvey It absolutely matters whether we have a library or application. Inside an application, I can update all references to that field when I change it to a property (simply recompile everything). If I publish a library, I'd force all my users to recompile. Though of course OOP made it popular to view internal parts of the application as an encapsulated library anyway.

My No 1. use case for properties is that I prefer mostly-immutable classes. If someone is going to set a field, it's going to be me. Properties allow me to me to make a field publicly readonly but privately writable.

ratchet freak

@RobertHarvey also keeping a variable synced to other state is error prone

jrh

I've done some A/B testing, some strictly following the advice of always using properties for non-private data, and some allowing public fields, I noticed that in the ones where I never exposed public fields, rather simple things like changing the Name of an entry in a Dictionary made me nervous that it was going to sync back to the container and update everything at a bad time, for example.

Whereas sometimes doing something like a dumb string Name field on the entry, and having an UpdateDictionary() method on the class that is responsible for maintaining that Dictionary seemed a lot simpler.

ratchet freak

Dec 6, 2016 17:33

that's the difference between a POD and a actual class

jrh

Also as far as "keep it synced" the only pattern that emerges is that the data in the field has to be validated before methods use it, which I'm still sort of on the fence about.

Uncle Bob seemed to recognize a need for POD, but other sources seem to condemn PODs as having no place in OOP.

ratchet freak

with a property it can be validated every time you access it

I've often wanted to use a quick pod for something in my stuff but in java there is quite a bit of friction against creating one

jrh

Getters and Setters typically shouldn't throw exceptions as per Microsoft's guidelines, and A/B testing has shown that it's easy to forget what's "Valid" unless the method does the validation itself, in some cases.

Though that sacrifices (sometimes) shorter methods for arguably easier to read methods with boilerplate.

I say sometimes because the .NET framework methods have a ton of boilerplate, usually.

From what I've been reading the friction against PODs was mostly due to the fact that in OOP objects should dictate what each method does, and abstract away data, though I'm interested as to why programmers chose to hold on to that in a language that deviated so far from something like Smalltalk (that's why I've been researching this from both a philosophical and practical perspective).

I have yet to find the original author who stated that PODs had no place in Java / C#.

amon

> The fundamental horror of this anti-pattern is that it's so contrary to the basic idea of object-oriented designing; which is to combine data and process them together. The anemic domain model is just a procedural style design, exactly the kind of thing that object bigots like me ... have been fighting since our early days in Smalltalk. What's worse, many people think that anemic objects are real objects, and thus completely miss the point of what object-oriented design is all about.

— Marting Fowler, cited via en.wikipedia.org/wiki/Anemic_domain_model

jrh

oh, I forgot about him, thanks. I'll look into his work.

It seems like databases are a popular topic in OOP, I've never really done anything with them,

The primary cost is the awkwardness of mapping to a database, which typically results in a whole layer of O/R mapping. This is worthwhile iff you use the powerful OO techniques to organize complex logic.

is he saying that each "column" in a database would be a property in a class?

amon

Dec 6, 2016 17:58

Yes, instance fields roughly correspond to database columns – they are both data you want to store in your system. In fact, it's quite easy to transform an ER diagram and a UML class diagram into each other.

The object-relational mismatch has two problems: (1) Objects are encapsulated data + behaviour, the database is just public data. This makes DB access inherently procedural. (2) You can extend objects with new fields by inheritance. You can't put that extra data into a DB table that doesn't have these columns.

(Of course there are relational databases that do offer table inheritance, and non-relational databases that don't require a specific data shape.)

jrh

Dec 6, 2016 18:42

I can definitely see why procedural access to a database like that could be a problem, I'll have to think about this more.

Robert Harvey

Dec 6, 2016 20:23

Lack of industry standards and patterns is a poor argument. I've seen systems that are decidedly non-standard and exotic that run circles around more orthodox systems. You have to figure out what the goals of the "others" are, and show them things that will meet those goals. If your goals don't align with theirs, that's a different problem altogether. — Robert Harvey 52 secs ago

enderland

@RobertHarvey but... standards!

Thomas Owens

Dec 6, 2016 20:36

Our charting library doesn't produce a standard compliant SVG.

It doesn't validate.

They only test against their conversion package (which is in beta) which apparently consumes invalid SVGs.

Robert Harvey

Bummer.

jrh

@amon is there any particular implementation of an object database API that you would recommend that I install and play around with, to get an idea of their design goals?

I'd guess that the "properties vs fields" argument might be clearer in the DB context since the schema can change in the DB but you may not want to update your code; since the schema is external and possibly controlled by a different department with different goals than code it might be a headache if any change in the DB caused the code to break.

Robert Harvey

@jrh Is this merely about using DTO's? Objects that are solely bags containing data, and have no other purpose?

jrh

sort of. So far I agree with Uncle Bob that DTOs have their place but some authors outright condemn DTOs, saying they don't belong in OOP.

The more extreme articles seem to focus on databases which is kind of a foreign topic for me

Robert Harvey

What programming language are you using?

jrh

Dec 6, 2016 20:43

C#, though if the guidelines are different for C++, Java, and others I'd be interested in the reasoning for that too.

I know Java and C++ don't have properties

Robert Harvey

What benefit do you get by exposing public fields instead of get; set; properties?

Other than the saving of a few copy/pasted characters?

jrh

A slight increase in clarity, it can never be changed to do anything else

Robert Harvey

But every C# programmer with a minimal level of experience understands get; set; properties.

I'd be more worried about their lambda knowledge.

jrh

True, but going back to the ListViewItem example, is it necessarily clear that ListViewItem.Text redraws the entire control?

Everyone knows get / set, sure, but would a programmer know what really happens in there, and how much would opinions differ?

Robert Harvey

Ah, that's probably not something you should be doing in a property.

jrh

Dec 6, 2016 20:45

And yet, there's at least 10 or 20 properties in .NET that do

Robert Harvey

That's a Redraw() method.

@jrh Examples, please.

Duga

Try posting to [email protected] — Thomas Matthews 7 secs ago

jrh

I will collect some and get back to you, ImageList is a big one

Robert Harvey

Just one.

jrh

TreeNode

TreeNode.Text, like ListViewItem.Text redraws

same with all of the icon indexes

Robert Harvey

Dec 6, 2016 20:47

Which property redraws?

jrh

Text, ImageIndex, Checked are the ones I recall in msdn.microsoft.com/en-us/library/…

the UX designer in me understands why this is convenient

but at the same time as a programmer I want to be able to batch update these without redrawing every time.

Robert Harvey

@jrh It redraws those on the Set, for reasons which should be obvious.

Batch redraws do, in fact, occur in this context. But if you're changing the text of a single control, it follows that that control will unconditionally need to be redrawn, and it's not all that expensive to redraw a single control.

Imagine if you had to call Redraw() or RedrawAsync() every time you updated a form control property.

jrh

It's obvious to me but it's because I've played around with the control and watched it visually update, I'm concerned that a less visible property wouldn't have such an obvious connection; if I didn't try it I may have been looking for a Redraw method.

Robert Harvey

Unless you've looked at the source code, you really don't know for sure what's happening under the hood. For all you know, you setting that property is marking the control dirty, and an event loop somewhere is periodically sweeping the control hierarchy looking for dirty things to redraw.

In any case, none of this has much to do with DTO's. Note that some things like WCF won't work properly unless your DTO's are structured properly. Reflection works differently on member variables than it does on properties.

Aaron Hall

Dec 6, 2016 21:28

Oh, boy, there's an SQL expert, 4 desks over, giving a very long and loud Socratic lecture to someone else. Open office floor plans... sigh.

Duga

This is a great programming exercise. But you might enjoy this cautionary tale about names. — Daniel Wagner 45 secs ago

MetaFight

Here, this will distract you:

Microsoft has decided to re-use their confusingly phallic image on their new Mobile Center site.

Robert Harvey

That would be a seriously dysfunctional phallus.

gnat

Dec 6, 2016 21:45

9

Q: Anyone else annoyed by this advertisement?

There's one advertisement that frequently appears in the right margin of Stack Overflow pages, that I find annoying and distracting. Does anyone else feel the same way? The advertisement in question is for a Microsoft Developer Camp. It has a man with his arms folded. To the left of the man, ...

jrh

@RobertHarvey Sure, I use properties frequently when it's needed for reflection, though I am still concerned about cases like ImageList.Image[], (which returns a copy of the image at a given index) vs PictureBox.Image (which stores the image itself, and if that image is freed, the PictureBox will stop working) -- how would the caller know that they need to free the Image they retrieved from ImageList?

this may be more of a semantics issue but I am not sure if a property is descriptive enough, whereas a hypothetical field and a Render() method might've been more obvious (note: I think PictureBox a pretty convenient class, ImageList I'm not as thrilled with...)

jrh

Dec 6, 2016 22:05

related: this is one thing that I haven't seen effectively addressed by various OOP texts, if IDisposable object A contains IDisposable object B, and it's needed to have a property to get A.B, should A return a copy of B, or A's (internal) instance of B? If A's instance of B is returned then the client code disposes B, that could cause A to stop working (e.g., this happens in PictureBox). If A returns a copy of B, then the client code must dispose its copy (ImageList).

Robert Harvey

Property Design Guidelines

jrh

Dec 6, 2016 22:40

@RobertHarvey Thanks, I've read that, quite a few other guides, a ton of blogs, some books... It was an interesting talk but I haven't really found the answer I'm looking for yet; I'm still hoping there's just some information I'm missing and I'll think properties are really great alternatives for fields but as of right now I haven't seen the benefits promised by having no fields (only properties) in the public / internal interface of a class.

The Whiteboard

Participants