views:

71

answers:

5

I would like to get your opinion on as how far to go with side-effect-free setters.

Consider following example

Activity activity;
activity.Start    = "2010-01-01";
activity.Duration = "10 days";   // sets Finish property to "2010-01-10"

Note that values for date and duration are shown only for indicative purposes.

So using setter for any of the properties Start, Finish and Duration will consequently change other properties and thus cannot be considered side-effect-free. Same applies for instance to Rectangle class, where setter for X is changing the values of Top and Bottom and so on.

The question is where would you draw a line between using setters, which have side-effect of changing values of logically related properties and using methods, which couldn't be much more descriptive anyway. I.e. defining method called SetDurationTo(Duration duration) also doesn't reflect that either Start or Finish will be changed.

I suppose it is more of personal preference and theoretical discussion, but hey, why not asking what other people think about it and approach this issue.

Your opinions are appreciated!

A: 

I have always worked with the general rule of not allowing public setters on properties that are not side-effect free since callers of your public setters can't be certain of what might happen, but of course, people that modify the assembly itself should have a pretty good idea as they can see the code.

Of course, there are always times where you have to break the rule for the sake of either readability, to make your object model logical, or just to make things work right. Like you said, really a matter of preference in general.

Steve Danner
+1  A: 

Personally, I think it makes sense to have a side-effect to maintain a consistent state. Like you said, it makes sense to change logically-related values. In a sense, the side-effect is expected. But the important thing is to make that point clear. That is, it should be evident that the task the method is performing has some sort of side-effect. So instead of SetDurationTo you could call your function ChangeDurationTo, which implies something else is going on. You could also do this another way by having a function/method that adjusts the duration AdjustDurationTo and pass in a delta value. It would help if you document the function as having a side-effect.

I think another way to look at it is to see if a side-effect is expected. In your example of a Rectangle, I would expect it to change the values of top or bottom to maintain an internally-consistent state. I don't know if this is subjective; it just seems to make sense to me. As always, I think documentation wins out. If there is a side-effect, document it really well. Preferably by the name of the method and through supporting documentation.

Vivin Paliath
A: 

I think it's mostly a matter of common-sense.

In this particular example, my problem is not so much that you've got properties that adjust "related" properties, it's that you've got properties taking string values that you're then internaly parsing into DateTime (or whatever) values.

I would much rather see something like this:

Activity activity;
activity.Start    = DateTime.Parse("2010-01-01");
activity.Duration = Duration.Parse("10 days");

That is, you explicity note that you're doing parsing of strings. Allow the programmer to specify strong-typed objects when that is appropriate as well.

Dean Harding
As I have mentioned "Note that values for date and duration are shown only for indicative purposes." So your comment really doesn't address the question asked, but thank you anyway...I knew someone will point at this ;-)
Martin
A: 

One option is to make your class immutable and have methods create and return new instances of the class which have all appropriate values changed. Then there are no side effects or setters. Think of something like DateTime where you can call things like AddDays and AddHours which will return a new DateTime instance with the change applied.

Sam
+4  A: 

I think you're misunderstanding the term "side-effect" as it applies to program design. Setting a property is a side effect, no matter how much or how little internal state it changes, as long as it changes some sort of state. A "side-effect-free setter" would not be very useful.

Side-effects are something you want to avoid on property getters. Reading the value of a property is something that the caller does not expect to change any state (i.e. cause side-effects), so if it does, it's usually wrong or at least questionable (there are exceptions, such as lazy loading). But getters and setters alike are just wrappers for methods anyway. The Duration property, as far as the CLR is concerned, is just syntactic sugar for a set_Duration method.

This is exactly what abstractions such as classes are meant for - providing coarse-grained operations while keeping a consistent internal state. If you deliberately try to avoid having multiple side-effects in a single property assignment then your classes end up being not much more than dumb data containers.

So, answering the question directly: Where do I draw the line? Nowhere, as long as the method/property actually does what its name implies. If setting the Duration also changed the ActivityName, that might be a problem. If it changes the Finish property, that ought to be obvious; it should be impossible to change the Duration and have both the Start and Finish stay the same. The basic premise of OOP is that objects are intelligent enough to manage these operations by themselves.

If this bothers you at a conceptual level then don't have mutator properties at all - use an immutable data structure with read-only properties where all of the necessary arguments are supplied in the constructor. Then have two overloads, one that takes a Start/Duration and another that takes a Start/Finish. Or make only one of the properties writable - let's say Finish to keep it consistent with Start - and then make Duration read-only. Use the appropriate combination of mutable and immutable properties to ensure that there is only one way to change a certain state.

Otherwise, don't worry so much about this. Properties (and methods) shouldn't have unintended or undocumented side effects, but that's about the only guideline I would use.

Aaronaught
Thanks, this is what I was looking for and it never occur to me the "side-effect-free setter" is in reality not possible as it will change the state. And as you pointed out the CLR will translate to method anyway.
Martin
Yes, the side effect of a setter is to set *the member variable it refers to*. But an additional side effect is to modify *other* member variables.
Loadmaster
I was just thinking about the same and I would even say that setting of the member variable is not a side effect, but desired effect of the setter. Changing of other member variables is a side effect.
Martin
You *might* say that setting the member field is not a side-effect, but you would be incorrect. The term has a very specific definition; *side effect* means that the operation changes state, period. It may not affect any fields at all - maybe it saves to disk or sends data out a socket. That is why I made the distinction of *unintended* or *undocumented* (or perhaps *undesired* side effects). It then becomes a simple question: Is this *additional* side-effect expected or not? It's not the *number* of things changing that matters, it's *what* those things are.
Aaronaught
Ok, have done my homework and do agree with your statement. Thanks again for your valuable input.
Martin