The getter, the setter, the thinking and the box
This is not a post about Actions, but bare with me.
So…
Actions. There’s always a point in the life of a project which makes use of some sort of Action Based web framework when you come face to face with that dreaded Action that has to populate an odd one million parameters to be rendered into an HTML template, objects that need to be retrieved from one million different entities residing all over the codebase, hopefully well confined in the domain space and becoming available to the action through some well thought out Service interface. You know the one. It’s the one whose execute() method is more than 10 lines long. The one whose unit test has a mocked out Service (if some decent design is anywhere to be seen, that is) with a silly number of expectations set for.
Things become even more hairy when you want to test different parts of the Action’s execution outcome, such as passing the correct values that it gets from each Service call. This involves setting the same expectations on the same ol’ mock numerous times, almost the same every time, just different enough to disallow some sort of common abstraction, diverting the attention from the intent of each particular test. I used to blame the fact that the execute() method is an Action’s sole point of activity, the main weak point of the whole Action abstraction in my opinion.
Until today.
A few hours ago, Paul Ingles came up with a suggestion so obvious, that I deserve to be shot for not having thought of it all this time. Being worried about the ugliness and size of some unit tests dealing with a situation similar to the one I just described above, I passed on my great theory about how Actions suck because you have to do everything in execute(). Well, no maan, you don’t.
If you’re lucky enough to be using Webwork, you’re probably used to the convenient way in which your getters and setters magically populate/get populated with request parameters. Of course, the standard way of doing that is the StandardJavaWay, which means one private attribute and two public methods, to set or return it.
The breakthrough suggestion was: Why don’t we put the specific service calls inside each getter? (It was a ‘View’ type of Action, by the way). In code, please, Jack:
public Thing[] getThings() {
if (thingsHasNotYetBeenSet()) {
//Just in case the getter is called more than once
this.things = thingService.getThings();
}
return this.things;
}
We suddenly have a lean, decomposed action, and nice, concrete unit tests that state their intent clearly and test one as opposed to fifty different things at the same time.
But THIS POST ISN’T ABOUT ACTIONS
What I find really alarming with this whole story is how tightly bound inside a tiny Java box our mind is sometimes. I was so used to seeing and writing those anemic getters and setters that it never crossed my mind that they could do something useful for a change in the context of the above problem. And I deserve even more blame, because it’s been a long time now since I started complaining about declaring private fields with public getters and setters.
Another thing that drives me nuts is how we can’t get away from the getter and setter nonsense even if we want to, because the most useful tools in our armory (call me Hibernate, Webwork, etc…) are build around them. I don’t remember how we came up with the whole JavaBean concept, I don’t even want to think about how we all fell for it, but the sheer fact that everyone’s still using it is beyond my kern.
What is even more interesting, Paul, the guy whose suggestion inspired this post, in case you forgot, is primarily a .Net dude. So, .Net devs can be clever, too. Sometimes…. Heheh….
Digg the box
June 29th, 2006 at 7:27 am
Sometimes yes, myself in general, no
June 29th, 2006 at 3:26 pm
I’ve almost always felt pain when doing fancy stuff in getters and setters. (And that’s actually a reason that I’m not too opposed to public fields, especially for non-exported code which you can auto-refactor into getters and setters if needed.) No examples off the top of my head. If I needed to decompose an execute() method, I’d probably prefer to do just that. Make helper methods or classes that do subtasks.
I prefer to have all the data ready and then start working with it rather than try to get too fancy with half-finished data or whatever. But I guess, as you said, the post isn’t about actions.
I just prefer anemic domain models most of the time.
June 29th, 2006 at 11:15 pm
Hey, I agree the whole requirment of get/set stinks. FYI, in hibernate, you do not actually need them. Just use access=”field” instead of property.
One down, 3 frameworks to go though.
One thing my teams do since we hate struts actions, is
1. use dispatch action
2. move almost all logic into the form- typical actions can then be about 3-4 lines:
1. cast form
2. have form do something
3. choose view
Cheers
James
June 30th, 2006 at 12:49 pm
Isn’t it “wrong” to use a getter to execute logic code more complex than a simple fetching of a property value?
June 30th, 2006 at 1:20 pm
Although I do not use Java (I use PHP or all my web work) I decided not t use getters and setters from day 1. Rather than inserting or etracting values one field at a time I do it en mass via an associative array. This means I have have generic methods such as:
$array = $object->getData($where);
$array = $object->insertRecord($array);
$array = $object->updateRecord($array);
$array = $object->deleteRecord($where);
All these calls are inside page controllers, and as no object names or field names are hard-coded I do not need a separate customised controller for each model. This means I can make use of a standard set of reusable controllers where the class name(s) from which the objects are instantiated are passed in as parameters.
I would not e able to achieve the same level of code resuse with getters and setters, therefore getters and setters are evil.
June 30th, 2006 at 2:12 pm
It is not “wrong”, but it may have unexpected side-effects if you use such classes with tools, such as Hibernate, which expect getters and setters (and constructors) to be plain and simple. I often put some extra code in those methods, because I find it convenient, but quite often funny stuff started happening afterwards, e.g. wrong values being read/saved to the database or getters/setters being executed at inappropriate times, like before the aplication was fully initialized.
If you only use those classes yourself, in a controled way, then it’s ok to have more complex getters/setters/constructors. If they are used by 3rd party bean handling tools, then you can still make them complex, but you have to be careful with what you do. You have to be aware that those tools expect these methods to be simple and that they can call them anyhow, at any time.
June 30th, 2006 at 5:22 pm
Eddy,
Possibly, and as my Economics master used to suggest I answer every question with: It depends.
In this instance, all we’re talking about is the loading of data. There’s no funky monkey-business with “if this child object doesn’t exist yet, create it, and then return it”.
It’s a slightly different use of the Lazy Initialisation pattern - where we’re not really attempting to delay database access, rather instead to improve our test code. The benefit was a JUnit TestCase that we didn’t need to edit every other testXxxx every time we changed our action.
However, I have seen strange behaviour through too much logic in getters (primarily from my years of .NET work). In this instance though, I don’t think it’s doing too much more than what a standard getter would be doing.
June 30th, 2006 at 5:48 pm
It isn’t wrong to use more complex logic in getters and setters, because the very purpose of using a getter or setter rather than a public property is encapsulation: to allow the object implementation with its private data to change without breaking code that relies on it! Even if a getter or setter starts out as braindead, in the future the object can put any code there that should be executed when the property value changes or gets accessed. If it was just a public property and not a getter/setter pair, that code would have to be duplicated everywhere that the object’s property is manipulated–outside the object–and that would be awful to maintain.
July 14th, 2006 at 2:09 pm
http://c2.com/cgi/wiki?TellDontAsk
Tell, don’t ask. Single-responsibility methods do things, and maybe take parameters to help them do them. (Don’t combine setters and doers, though.)