Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
9  
Properties should not perform a lot of work, or that code will confuse whoever is reading it. Methods should do that, plus the code needs to be well-organized. stackoverflow.com/questions/2784934/… –  Job Jul 15 '11 at 16:05
    
Right. I always forget something significant. But... What's your definition of "property"? Something with a backing variable? So, a data member named mBar and an accessor getBar() is a property, while a function getBlurgh() with no backing variable and a complex calculation whose result is cached (memoized) is not a property? And, if we later change the code to delete mBar and turn getBar() into a computation, it's no longer a property? How is "method" distinguished from "property"? –  JohnL4 Jul 15 '11 at 16:15
    
That kinda depends on the language. In Java, getters and setters are really methods (prefixed with Get and Set) masquerading as properties, but the principle is the same. Generally, all methods prefixed with Get or Set are assumed to be "properties." In C#, properties are first-class citizens of the language. –  Robert Harvey Jul 15 '11 at 16:18
    
@JohnL4, this depends on a language. C# helps to make a clear distinction at a syntax level. There you can have an auto-property, such as public int XYZCount { get; private set; } which is very cool (albeit there exists a bug there somewhere(which will be fixed soon)). So, in java, you would have to perform some extra work and add a backing variable yourself. Getters should not have side effects. Setters should not do much either. What Robert said ... –  Job Jul 15 '11 at 16:20
    
Thanks, y'all. Just to clarify: I know language syntaxes. Property-vs-method is not a syntax question, but a higher-level sorta thang. Hmm, I wonder how tell-don't-ask factors into this. –  JohnL4 Jul 15 '11 at 16:43
show 2 more comments

4 Answers 4

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.

share|improve this answer
    
I don't usually expect a setter to return with a value... –  glenatron Jul 15 '11 at 16:22
    
@glenatron: Fixed. –  Robert Harvey Jul 15 '11 at 16:24
2  
I kinda disagree about the setter. Setters are, generally, about changing state. I never assume changing state is cheap. –  ElGringoGrande Jul 15 '11 at 16:48
    
@ElGringoGrande: I originally had a statement in my answer about ORM's. Since ORM's talk to a database, it is generally expected that it will take some time to not only save state in the database, but also retrieve it. This is generally true of all remote communication scenarios. But if you're writing an ordinary class with properties, I would expect getters and setters to return almost instantaneously, and if there's some really heavy lifting to be done, I would expect that to be behind a method. –  Robert Harvey Jul 15 '11 at 16:54
add comment

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.

share|improve this answer
    
Whoops, my bad. Didn't make clear that it's a VB.NET property with setter, not a simple public data member. What, you couldn't read my mind? All my questions about setters now apply to `obj.SetTabOrdering(3);' -- do we prefer complexity in SetTabOrdering(), or we prefer it elsewhere, such as (for wild example) GetTabOrdering() or IsTabInForeground()? –  JohnL4 Jul 15 '11 at 17:22
    
@JohnL4: I understood that it was a property with a setter. Don't expose state, expose the intention through a method instead. As for where the complexity should go, that depends on what you're doing and why - please edit your question with more details on the mechanism and usage. Complexity does not belong in getters and setters, and setters do not belong in the public interface if avoidable ;-) –  Steven A. Lowe Jul 15 '11 at 20:48
add comment

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.

share|improve this answer
    
Would that it were so simple. It's essentially an index into an XML tree of questions to ask the user. My eye just slid over it naturally, because it looked like a simple member variable or property set. Furthermore, it's in the middle of a simulated user click, because the programmer who came before me wanted to achieve results by pretending the user had clicked on a button rather than by refactoring code out of the click handler. And anyway, this is all in the middle of a huge (different) click handler call tree, so, by the time we're done, the GUI will definitely be updated. –  JohnL4 Jul 15 '11 at 17:39
    
The question is more about managing complexity than performance, I was just anticipating a response I never got. :) (...is why I brought caching into it.) –  JohnL4 Jul 15 '11 at 17:39
    
Re: managing complexity, I still think that handling the logics in events is the way to go. =] –  dlras2 Jul 15 '11 at 18:00
    
10-4, thx. I was enamored of event-driven stuff (in the Java world, PropertyChangeListeners and all that) a few years ago, but my beef is that it's really hard to statically know which handlers will get run. But... I'll bear it in mind. :) –  JohnL4 Jul 15 '11 at 18:14
add comment

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...

share|improve this answer
    
So are you basically saying (in so many words) that side effects should only occur in the setter, and never in the getter? –  Robert Harvey Jul 15 '11 at 16:34
    
I general, but I rarely subscribe to universals - for example there may be an auditing functionality that requires a record of who has accessed certain information to be recorded and that could be tied to a getter, although it would be a bit scrappy. I would say a member being set is more likely to trigger further actions than a member being retrieved in most cases. –  glenatron Jul 15 '11 at 16:44
    
I think that is what he is saying and if so that is very correct. Calling a getter shouldn't affect state. OTH I do all my lazy loading in the getters and that is the way I see it handled in most cases. But lazy loading isn't logically affecting state, it is just retrieving the state from an expensive place. –  ElGringoGrande Jul 15 '11 at 16:45
    
On a separate thread, no less. I think he's assuming an app in which the state change/computation is really heavyweight and it's ok for the old state to hang around a while. Case in point: Amazon shopping cart. "Add to cart" is sort of a setter which fires off a cart update, "check cart" is the getter which the user expects to return quickly. So, (a) we precompute cart on a separate thread, for responsiveness, and (b) it's not the end of the world if the cart doesn't get updated w/in a couple of seconds, because a user checking his or her cart that fast is the exceptional case. –  JohnL4 Jul 15 '11 at 16:50
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.