Using a SetProperty method to prevent accidental changes to a property

Question

Is it good/bad practice to do the following:

public class MyClass {
  public MyType MyProperty { get; private set; }

  public void SetMyProperty(MyType myProperty) {
    MyProperty = myProperty;
  }
}

My intention is to prevent MyProperty from being accidentally changed outside the class. However, it can be changed if needed.

Also, if MyType is a reference type, then will the private set prevent, for example:

instanceOfMyClass.MyProperty.SomeField = 2;

In other words, will it make MyProperty and all its members readonly, or just the reference to MyProperty?

Sorry if this seems an unintelligent question; I'm afraid I'm a beginner.

Answer 1

My intention is to prevent MyProperty from being accidentally changed outside the class. However, it can be changed if needed.

I'm not seeing the point of doing this, to be honest. Calling

myClass.MyProperty = new MyType();

is not any less intentional than calling

myClass.SetMyProperty(new MyType());

In terms of readability, I think you're far better off going with the more intuitive setter approach instead of a separate method. And if you don't want to allow anyone to change the value of MyProperty, then don't allow it either via a setter or a method.

In other words, will it make MyProperty and all its fields readonly, or just the reference to MyProperty?

Just the reference to MyProperty. MyType has to be an immutable type if you want to prevent people from changing its properties.

Answer 2

Having a private setter is a perfectly valid practice and I do this often myself. This is part of encapsulation, which is one of the foundations of OOP.

However, providing a public method for setting is rather pointless - if you want to be able to set the value, use a public setter.

As you asked, having a private setter will not allow the value of the underlying field to change outside of the class (the compiler generates one for you - there is still an underlying field). This is unless you provide public functions that will cause changes to this value.

Answer 3

If I wanted to use "SetProperty" methods all day, I'd program in java.

Additionally, if you have a public get property, even if that property has a private set, the property's value's members can be changed. Some classes provide "read only" wrappers to solve this concern, such as the many of the array class: http://msdn.microsoft.com/en-us/library/53kysx7b.aspx

Answer 4

Providing a getter still allows the properties of the class to be changed. I would clone the instance if you'd want to avoid that

private MyType _myProperty;
public MyType MyProperty { get { return new MyType(_myProperty); } set { _myProperty = value; } }

Answer 5

Although there is not much reason to do this for a single property, but I do think there is a reason to do this for setting multiple properties at the same time.

public class MyClass {
  public MyType MyProperty1 { get; private set; }
  public MyType MyProperty2 { get; private set; }

  public void SetMyProperties(MyType myProperty1, MyType myProperty2) {
    MyProperty1 = myProperty1;
    MyProperty2 = myProperty2;
    // other business logic
  }
}

This is used when you want to force MyProperty1 and MyProperty2 to be set at the same time, and properly run some business logic checks for it.

Answer 6

If something would behave as a read-write property, it should generally be implemented using a read-write property. On the other hand, the fact that the value of a property can be changed does not mean it will behave like a read-write property. I would suggest that in general, something should only be implemented as a read-write property if it acts as a "clean" property (described below), though I would note that many classes in .net use read-write properties in ways which do not conform to what I suggest. I would conjecture that in many cases, this stems from time pressure to get the framework implemented before the full philosophical background was completely worked out.

I would regard a read-write property as clean if:

1. Reading it will never have any side effect and will always yield the same value in the absence of a method call or write to that property.
2. Reading it and then writing back the value read will have no effect.
3. A read which follows a write without intervening method call will always yield the value written.
4. Writing the property more than once without any intervening method calls will have the same effect as performing only the last write.
5. In the absence of intervening method calls, the effect of setting a clean property to X and then setting some other property to Y will be the same as setting the other property to Y and then setting the clean one to X.

Note that it is perfectly permissible for a write to a "clean" property to affect the state of other read-only properties. The effect of setting multiple properties should depend upon the values written, but not the sequence in which the writes occur. If myRect is initialized to (X=1,Y=2,Width=3,Height=4), the effect of MyRect.X=2; MyRect.Top = MyRect.Right; should be the same as MyRect.X = 2; MyRect.Top = 5; or MyRect.Top = 5; MyRect.X = 2;, even though it wouldn't be the same as MyRect.Top = MyRect.Right; MyRect.X = 2;.

Many .net classes use properties in ways which are by these definitions "unclean". Using these definitions, for example, StringBuilder.Length should probably be a read-only property, but Text could probably be a read-write property (reading Text would be equivalent to ToString(), while writing it would be equivalent to clearing it and doing Append).

There are variety of situations in which one may be able to change properties of an object, but they should not be considered "clean read-write properties". Among them:

1. A rectangle which stores (X,Y,Width,Height) and exposes them as read-write properties may have a `Right` property, but attempting to set it would affect one of the others.
2. An object may hold a reference to an `IDisposable` object and be expected to possibly call `Dispose` on it when it's no longer needed. A method `SetXXAndTakeOwnership()` would offer clearer semantics (call `Dispose` the old object before setting the property) would offer much clearer semantics than a read-write property.
3. Some objects may have properties which can only be set to certain combinations of values; having a method to set all such properties at once may be much nicer than having to set the properties individually and worry about sequencing. `ProgressBar` is particularly icky in this regard, since its properties must be written in an order which depends upon old and new values; if a `ProgressBar` is presently indicating 7/9 done, changing it to 3/5 requires writing `Value` before `Maximum`, but setting it to 14/19 requires writing one must update `Maximum` first. A `SetFraction` method which accepted parameters for both value and maximum, and updated both simultaneously, would have been much better.
4. Some properties like the time remaining on a timer may change between the time they're written and the time they're read. Such things may be perfectly fine as read-only properties (reading them would take time, and the act of taking time might affect future readings, but such side effect would be no different from any other code the system might execute) but they would violate the rule that a clean property should return the last value read.

If any of those conditions (or any of many others--the list is hardly exhaustive) apply, use a SetXX method rather than a read-write property.

Answer 7

The only time I use this (very rarely) is when I have two properties that can be changed but not independently. So one non-private setter method that changes two properties simultaneously. It’s rare.

But how would I use an assignment like setter by accident, and a setter that looks like a method not? Don’t get it.