Logic in setters vs. getters?

Question

Okey dokey, next crappy-code-at-work-rant-turned-into-a-real-question (I hope):

So, I'm debugging some code, and we've got something like this:

objFoo.TabIndex = 5   'VB.NET property with setter implicitly called

and TabIndex is a property whose setter is the beginning of an insane call tree, with subroutines and XML and special cases depending on context (like: modes), and so on.

(Gack. You can see what working here is doing to me. I just prefixed Foo with "obj". :( )

Ok, granted, we need to manage state and perform computations and whatnot. Our state is either in the database (but may be expensive to pull out) or it's not yet ready to be committed to the database (e.g., it's only half-baked), so we can't just run out to the d/b every time we need some info, but...

Is it better to manage this state in setters or is it better to do it in getters?

I would think doing it in getters (i.e., lazily) would mean we would compute (and, presumably, cache) only what we need, while doing it in setters means we're essentially overcomputing, anticipating what getters may be called in the future and precomputing their results. (And then, there are the data members that don't have getters at all, they're just global variables in a 6,000-line class.)

Shouldn't we just, as a general rule, set the indicated data member, maybe set some flags indicating other data is now invalid (cache invalidation) and wait for the various getters to be called to recompute?

I can imagine one argument for doing this in setters is that our object is so complex that we really need to configure its state properly, using multiple complex data members. But isn't that a red flag in itself, indicating that a better approach would be to break the object up into smaller objects, not write a complex setter?

(Now, I know there are all sorts of exceptions which people will no doubt point out, but I'm looking for a general rule of thumb and the logic behind it, or the statement that no such general rule of thumb is possible, accompanied by a convincing argument.)

Thanks.

John.

Answer 1

Generally speaking, if the logic takes a non-trivial time to execute, it should be done in a method, not a getter or setter.

By convention, getters and setters are expected to return almost immediately, whereas methods are expected to take...a little longer.

Answer 2

Don't expose state

prefer

obj.SetTabOrdering(3);

for two reasons:

1. it hides the use of the TabIndex state variable, in case it changes in future

2. it implies (because it's a method) that additional processing may occur

In general:

• setters and getters for properties should not perform extensive computation
• setters and getters for properties should not be exposed in the public interface if avoidable

The latter helps ensure that internal state changes do not cause external interface changes.

Encapsulation + Interfaces: Hide your state. Expose your methods.

Answer 3

Regarding caching the properties, I would consider this on a case-by-case basis. In your example TabIndex property, does this refer to the tab displayed, or something else? If it's the displayed tab, obviously you need to update immediately, rather than waiting for something to query it's state and possibly leave your GUI outdated. Additionally, the use of caching may quicken your property set times, but now you may be doing all the cache updating in your property get calls, which is even less preferred.

I asked a similar question myself a while back, and what I've concluded is this: getters and setters should not perform extensive operations themselves, but may raise events which do perform a fair amount of logic. For example, updating TabIndex would raise TabIndexChanged, in which all the logic would be performed. This allows for a little more give and take in the program flow, and lets you separate your properties from your logic in a nice easy-to-understand way, which is very important.

Lastly, I like the idea that methods are verbs, properties are not. So don't set a Connect property to true to start a connection, call OpenConnection instead.

Answer 4

It depends on what the state affects and what affects the state, in my view.

Assuming the object state relates to more than the single member, then if by changing the value of that member value the state of the object is affected you might want to handle the state in the setter. Given that you may not want to leave the user hanging around in this case you might want to just fire an event to manage the state change in the setter, rather than waiting for all the related processing to go on.

If the member value, when returned, depends on the state, then you're going to have to check the state in the getter.

So really the connection between any given member variable and the overall object state should make it fairly clear when, if at all, you need to interact between the variable's value changing and being retrieved and the state. If it isn't then deciding whether to put that logic in the getter, the setter or somewhere else is nothing compared to your wider architectural concerns...