There are things in life that you take for granted. That is, you are so accustomed to them that you never question them. What for? Accessors (member functions that directly manipulate the value of fields) in the Java world is one of them.
The use of getters and setters is one of the first lessons to learn when working with Java, probably because the JavaBeans specification mandates their use. They help to increase robustness, enforce logic and hide implementation details. What's not to love? Well, something.. but maybe Java 7 will finally bring us properties...
During the weekend I read an interesting article titled Dishonest Programming. I'll quote one of the paragraphs:
Everything was related to the following line of code (a line I've written a handful of times...):
The use of getters and setters is one of the first lessons to learn when working with Java, probably because the JavaBeans specification mandates their use. They help to increase robustness, enforce logic and hide implementation details. What's not to love? Well, something.. but maybe Java 7 will finally bring us properties...
During the weekend I read an interesting article titled Dishonest Programming. I'll quote one of the paragraphs:
Don't write a public accessor to a private member and tell me that member is still private. If an object is completely and utterly accessible from anywhere in the application, it's a global variable.
Everything was related to the following line of code (a line I've written a handful of times...):
setupModel.getOverrides().add(accountOverride);
Or a subtle variant:
for(Bean b: a.getBeans()) b.doC();
The Law of Demeter (Principle of Least Knowledge) clearly states that a method M of an object O may only invoke the methods of the following kinds of objects
- O itself
- M's parameters
- Any objects created/instantiated within M
- O's direct component objects
public class company {
private List<Address> addresses;
private Set<Account> accounts;
private Map<Long, Employee> employees;
...
}
private List<Address> addresses;
private Set<Account> accounts;
private Map<Long, Employee> employees;
...
}
Can you determine the number of methods to add to the interface if you pretend to comply with the LoD? At least a couple of dozens for the collections (just java.util.List packs thirty approximately) and some more for the types.. On the other hand, the reasons exposed by the author (here) seemed valid:
The first one is problematic though. Aren't public getters already breaking encapsulation? The moment they're available any other object can really interact with the supposedly private fields.
The second worries me greatly! This kind of code glues together objects that shouldn't be tied.
The fourth is problematic as well. PropertyChangeListeners already exist in Java but they're meant for peer-to-peer interactions not for parent/child invocations. If you're bypassing the object this way it's probably better to refactor.
At this stage, we have a problem. If the internal structure of an object is available (and so do the getters) it's difficult to avoid using it (or telling a programmer to avoid using it, if you prefer). I've come to the conclusion that creating proxy methods for cascade operations is the way to go, even if they are a lot!
There are two options then:
Finally, and I agree with the original author here, everything is a trade off and nothing should be written in stone. Just don't abuse the delegate strategy and try to conform to a best practice. We all know Java is verbose to the extreme and that this solution adds even more boilerplate (and that in 90% of the cases it won't matter) but if you stick to Java that's a nasty side effect you have to bear with. Just don't try to create shortcuts that punish good design.
- Violate encapsulation
- Tightly couple all three objects together
- Cripple modularity/type independence
- Deprive the parent class of the ability to guard, or even be aware of, the child data
The first one is problematic though. Aren't public getters already breaking encapsulation? The moment they're available any other object can really interact with the supposedly private fields.
The second worries me greatly! This kind of code glues together objects that shouldn't be tied.
The fourth is problematic as well. PropertyChangeListeners already exist in Java but they're meant for peer-to-peer interactions not for parent/child invocations. If you're bypassing the object this way it's probably better to refactor.
At this stage, we have a problem. If the internal structure of an object is available (and so do the getters) it's difficult to avoid using it (or telling a programmer to avoid using it, if you prefer). I've come to the conclusion that creating proxy methods for cascade operations is the way to go, even if they are a lot!
There are two options then:
- Defaulting to a protected visibility
- Returning immutable objects
Finally, and I agree with the original author here, everything is a trade off and nothing should be written in stone. Just don't abuse the delegate strategy and try to conform to a best practice. We all know Java is verbose to the extreme and that this solution adds even more boilerplate (and that in 90% of the cases it won't matter) but if you stick to Java that's a nasty side effect you have to bear with. Just don't try to create shortcuts that punish good design.
2 comentarios:
The immutable getter thing is one alternative that also occurred to my team. We tried it but it didn't work for us. On the doomed project I mentioned over on my blog, we tried to protect ourselves against the worst of it by changing all the list getters to return immutable collections. In the end we abandoned it because it added a LOT more boilerplate (everybody doing mutable/immutable conversions) and while it prevented callers from affecting the parent by modifying the child, it still didn't allow the parent to provide its own services on top of the child. In other words we reduced some of the drawback of tight coupling, but we did not gain any of the benefits of truly decoupling them at the object model level. The design was still "get B from A, then do something with B".
But that's when you need to re-examine the design and see if the parent really does need to add its own services. In most of our cases, we found it did, so we went ahead and proxied. In a few cases we found it didn't matter, so we put the mutable accessor back in. The greatest symptom to watch for here is that proxying becomes painful when you are forced to only have a conversation with the parent but you really do want to be having that conversation with the child.
If you find yourself proxying a lot, and you're using a Java-friendly text editor (i.e. one capable of churning out boilerplate for you from template) you will quickly discover a few proxy sets that you use over and over that you can just jam in and call it done. I can't remember the Eclipse syntax anymore but we had a collection proxy template that created add, remove, and getIterator methods for each proxied collection.
chalain comment hits the nail in the head.
To comply with the Law of Demeter, you don't need to delegate all the methods in the List, Set and Map interfaces. Think about which operations need to be performed:
* Add Address
* Add Account
* Add Employee
* List Addresses
* List Accounts
* List Employees
* Retrieve Employee
* (perhaps there are others)
Those are the only methods you need to implement in your Company class. And by returning unmodifiable implementations of the List, Set and Map, you guarantee that the only way to add an Address, Account or Employee to a company is through the Company class. Returning a mutable Employee, on the other hand, seems the right thing to do.
Rafael
http://tech.soronthar.com/
Publicar un comentario en la entrada