What Getters and Setters should and shouldn't do.

Question

Possible Duplicate:
Convention question: When do you use a Getter/Setter function rather than using a Property

I've run into a lot of differing opinions on Getters and Setters lately, so I figured I should make it into it's own question.

A previous question of mine received an immediate comment (later deleted) that stated setters shouldn't have any side effects, and a SetProperty method would be a better choice.

Indeed, this seems to be Microsoft's opinion as well. However, their properties often raise events, such as Resized when a form's Width or Height property is set. OwenP also states "you shouldn't let a property throw exceptions, properties shouldn't have side effects, order shouldn't matter, and properties should return relatively quickly."

Yet Michael Stum states that exceptions should be thrown while validating data within a setter. If your setter doesn't throw an exception, how could you effectively validate data, as so many of the answers to this question suggest?

What about when you need to raise an event, like nearly all of Microsoft's Control's do? Aren't you then at the mercy of whomever subscribed to your event? If their handler performs a massive amount of information, or throws an error itself, what happens to your setter?

Finally, what about lazy loading within the getter? This too could violate the previous guidelines.

What is acceptable to place in a getter or setter, and what should be kept in only accessor methods?

Edit:

From another article in the MSDN:

The get and set methods are generally no different from other methods. They can perform any program logic, throw exceptions, be overridden, and be declared with any modifiers allowed by the programming language. Note, however, that properties can also be static. If a property is static, there are limitations on what the get and set methods can do. See your programming language reference for details.

Answer 1

My view:

1. If a setter or getter is expected to be expensive, don't make it a property, make it a method.

2. If setting a property triggers events due to changes, this is fine. How else would you allow listeners to be notified of changes? However, you may want to offer a BeginInit/EndInit pair to suppress events until all changes are made. Normally, it is the responsibility of the event handler to return promptly, but if you really can't trust it to do so, then you may wish to signal the event in another thread.

3. If setting a property throws exceptions on invalid values, it's also fine. This is a reasonable way to signal the problem when the value is completely wrong. In other cases, you set a bunch of properties and then call a method that uses them to do something, such as make a connection. This would allow holding off validation and error-handling until the properties are used, so the properties would not need to throw anything.

4. Accessing a property may have side-effects so long as they aren't unexpected and don't matter. This means a JIT instantiation in a getter is fine. Likewise, setting a dirty flag for the instance whenever a change is made is just fine, as it setting a related property, such as a different format for the same value.

5. If it does something as opposed to just accessing a value, it should be a method. Method are verbs, so creating a connection would be done by the OpenConnection() method, not a Connection property. A Connection property would be used to retrieve the connection in use, or to bind the instance to another connection.

edit - added 5, changed 2 and 3

Answer 2

I agree with the idea that getters/settings shouldn't have side effects, but I would say that they shouldn't have non-obvious side effects.

As far as throwing exceptions, if you are setting a property to an invalid value (in a very basic sense), then validation exceptions are fine. However, if the setter is running whole series of complicated business rule validation, or trying to go off and update other objects, or any other thing that may cause an exception, then that is bad. But this problem is not really an issue with the exception itself, but rather that the setter is going off and secretly performing a lot of functionlity that the caller would not (or should not) expect.

The same with events. If a setter is throwing an event saying that "this property changed", then it's OK, because that's an obvious side effect. But if it's firing off some other custom event so cause some hidden chuck of code to execute in another part of a system, it's bad.

This is the same reason that I avoid lazy-loading in getters. Indeed, they can make things easier a lot of the time, but they can make things a more confusing some of the time, because there always ends up being convoluted logic around exactly when you want the child objects loaded. It's usually just one more line of code to explicitly load the child objects when you are populating the parent object, and it can avoid a lot of confusion about the object state. But this aspect can get very subjective, and a lot of it depends on the situation.

Answer 3

I've always found the conservative approach to be best, when working in C# anyway. Because properties are syntactically the same as fields, they should work like fields: no exceptions, no validation, no funny business. (Indeed, most of my properties start out as simple fields, and don't become properties until absolutely necessary.) The idea is that if you see something that looks like it's getting or setting a field set, then it IS something like getting or setting a field, in terms of functionality (there is no exception thrown), overall efficiency (setting variables doesn't trigger a cascade of delegate calls, for example) and effect on program's state (setting a variable sets that variable, and doesn't call lots of delegates that could do just about anything).

Sensible things for a property set to do include setting a flag to indicate that there's been a change:

set {
    if(this.value!=value) {
        this.changed=true;
        this.value=value;
    }
}

Perhaps actually set a value on another object, e.g.:

set { this.otherObject.value=value; }

Maybe disentangle the input a bit, to simplify the class's internal code:

set {
    this.isValid=(value&Flags.IsValid)!=0;
    this.setting=value&Flags.SettingMask;
}

(Of course, in these latter two cases, the get function might well do the opposite.)

If anything more complicated needs to happen, in particular calling delegates, or performing validation, or throwing exceptions, then my view is that a function is better. (Quite often, my fields turn into properties with get and set, and then end up as a get property and a set function.) Similarly for the getters; if you're returning a reference to something, it's no problem, but if you're creating a whole new large object and filling it in each time the property is read -- not so hot.