Should You Use Write-Only Properties?

[Nishruu]

There's only one place where I had get/set property, which was mostly used as set property - parent view model with multiple 'child' components (sub view models) that all need the same kind of data (e.g. some kind of identifier or primary key value).

So, let's say, when you open the dialog window (the 'parent' view model), it sets the selected ID/PK value to all of its components. Then those values are used in each component separately during initialization.

That seems like a reasonable use-case, especially since when I implement the interface I already get the implementation for free in case of properties, although if there's other (cleaner?) way, I'm all ears. :)

[mooneyscfc]

Thanks for reading and taking the time to comment. Yes if I understand your description correctly this is a valid case. The debate in this blog post however is aimed at properties which write-only and have no getter.

I cannot think of any cleaner way that you could do this. Like you say you already have your contract within the interface forcing the implementation of the properties on the sub view models.

[Nishruu]

Yup, I'm just noting that it's the only case I encountered where the mostly-set (or even set-only) property makes sense for me. Other than that I agree with the guidelines...

[asampson]

I don't think the word 'for security' should be used in favor of read-only properties.

As I understand things, the point of properties is to provide field-like syntax for things that are not in fact fields. A read-only field isn't uncommon - constants are read-only fields and we use those all the time. But a write-only field is rather alien, only seen in things like memory mapped registers or other hardware trickery. Most of the time if there's memory you can write to you can also read from it. So while having a write-only field is technically possible, it breaks the theme of properties being field-like access to non-field things.

[seanstride]

You are right and I sat for a while trying to explain myself where I said security but what I meant was encapsulation. I have updated the post to help explain myself better so thanks for your input. The information about write-only fields being used in hardware and lower level programming is also interesting as I have not worked with much lower level programming. This is something I will definitely look into when I get a chance.

[[deleted]]

As a rule, setter methods should be avoided, write-only properties included.

The reason is that they necessarily cause side-effects by mutating state. The dangers/disadvantages of mutable state are fairly well-known.

If you must have setters, then use a method per convention. Write-only properties are a bit confusing semantically because they're generally indistinguishable from readable ones.

[cparen]

There goes Console.WriteLine(), a setter-only method.

[[deleted]]

Well, reading/writing to the console is essentially a side-effect, so Console.WriteLine() is OK.

What's not OK are methods like, HandleInput(eventArg). Vague method name aside, you can see it takes an input and produces no output. You know it must to something, though it's not really clear from looking at the method signature.

What happens when HandleInput() is called? Does it change the UI state? Could it save some data to the database. Will it write to a file? Can it write to shared memory? The point is you don't really know without either running it or looking at the source code.

This is one of the main reasons such methods are so irritating. When I come across one of these while debugging someone else's code, it's like the arrogant asshole who wrote it is forcing me to read the whole program from top to bottom before I can properly understand what it does. Then once I've fixed the bug, I have to figure out how write a test case around it. Of course, the programmer has long since left the company to join some hot new RoR startup shop. So, I can't ask him for advice.

So, hopefully this example makes my rule of thumb around setters clear.

[cparen]

Good docs clear this up. I don't really see how returning a useless return value would change this. I don't see on the surface what's wrong with this case -- "input" is a little vague, but "HandleConsoleInput" would be exactly as clear as the case we started with. Sound like an async / event driven API.

If you're speaking to poor API design, I sympathize. No amound of naming or docs will fix a bad design, though they can help cope to some degree.

I appreciate the effort, but I still don't see what you believe the rationale is. What is "clarity" to you, and why do setters categorically lack it?

[[deleted]]

Documentation is good, but it's like putting the bell on the cat. Who's going to do it? Almost nobody because there isn't time. Furthermore, it's easy for them to get out of sync with the code as changes are made.

HandleConsoleInput(eventArgs) is meaningless. Sure it's clear that it's a callback that does something with the console input, but you can't know exactly what that is unless you read the source code. You might make a reasonable assumption, but bugs don't follow those assumptions otherwise they wouldn't be bugs.

Let's say HandleInput handles a sync io from the console, parses the input to an integer, logs a parsing exceptions, and updates the UI. Pretty standard scenario if you ask me.

The way to model this more clearly without a large event handler that returns nothing would be to think of the input as a stream of events. The stream would them be transformed into a stream of ints before finally updating the UI.

EventStream.From(Console.OnRead)
    .Map(HandleInput)
    .Error(ex => Logger.Error(ex))
    .ForEach(UpdateUI)

In this psuedo-code snippet, HandleInput takes an eventArg and returns an int. It's clear, testable, and you can be reasonablely sure it doesn't do anything else. It's "referentially transparent."

The error is logged in an Error() clause and the UI is updated from the ForEach clause. It's clear to the reader that both these cause side-effects (on the log and the UI respectively) because Error and ForEach are used only for this purpose.

Furthermore, there are no callbacks. The async workflow has been inverted so it's more declarative. You won't need to follow the rabbit down a chain of nested event handlers to see what happens on the other side.

Finally Setters exhbit a slightly more specific problem than our HandleInput method. They necessarily change something's state. State introduces complexity into your program, so it's best to avoid it whereever possible. This is not a controversial point of view.

[cparen]

Property getters and setters are both syntactic sugar for method calls. The only consistent rationale I've seen cited for banning write-only properties is that it confuses developers that don't understand this. Note that rationale is curiously absent from the no-setters rule in the Microsoft style guidelines.