, Martin Fowler says: "... look through the methods of a class to find a method that seems to reference another object more than the object it lives on", and "by moving methods around, I can make the classes simpler and they end up being a more crisp implementation of a set of responsibilities".
For example, class Person does some stuff, including knowing the components of the person's address.
Class Envelope has a method in it that sends several messages to an instance of Person, something like:
^person name, person address, person city, person state, person zip
The responsibility to produce a name/address string just wants
to be on person. This method in Envelope sends to Person five times and never once to Envelope. Kent and Martin, in chapter 3, call this "FeatureEnvy
The responsibilities are better aligned, "a more crisp implementation", with this method moved to Person.
How do we see this? Where is it written what the responsibilities are? Well ... it's written only in the code. It doesn't matter what the documents say. In fact, if the document said: Person has the responsibility to hold address components. Envelope has the responsibility to append address components to make a printable address,
the document would be erroneous. The code knows.
This seems fine, but are there exceptions to the rule? For instance, the GOF visitor pattern seems to violate this rule. The visitor, at least in the times I have used it, is a lot more interested in the target object than itself. That is, the visitor makes a lot more calls to the target object than itself.
I'm not convinced about this example. It seems to me that Envelope has some specific things it needs to do with an Address, and that the AddressAsString?
method would end up doing something pretty specific to what Envelope wants.
For instance, where you put the zipcode/postcode on a printed envelope is determined by the country. It's different for Germany and the UK, for example. Why would Person want to know about that?
Perhaps I'm arguing for an Address class. More likely, I'm demonstrating my comparative OO ignorance in a public forum.
This is a good example of my comment above about visitor. Still seems that even with an address class that it would have to know how to format itself for each type of address (German, UK, US, etc.) and for each size of envelope.
Another way past this particular problem is to delegate to a Strategy object, which takes responsibility for rendering an Address in a form appropriate for a given country. E.g., when printing envelopes for Germany, plug in the printAddressForGermanyStrategy object. -- DaveSmith
This is a good example of a series of related problems. When we see a chain of object references like envelope -> person -> address, I've found that there are usually two possible explanations.
- That the code/object structure is weak. Straightforward refactorings can help here: a) have person return a const address that can be used directly by envelope, or b) move addressString to person, or c) move addressString to address. (If, as suggested, addressString requires locale specifics, you can still add a locale class and pass an instance to addressString regardless of which refactoring you choose.)
- That the objects in question are really just simple 'data-holders'. Data-holders are frequently characterized by lots of get/set pairs. They perform no useful function other than the getting and setting. If you see lots of data holders, suspect them. The only known fix is an object structure re-work. I believe this problem derives in large part from naive attempts to start object designs with data as the focus rather than behavior. I will say that there are often external constraints that make such pre-objects mandatory.
I've see many people fret needlessly over 'data-holders.' I agree with whoever posted above, that overuse indicates a data-centric view, but the opposite tack, to try to eliminate them completely is overzealous. I like ResultObject
, for instance. It is funny how little objects prompt refactoring. Each little object that you have is a location you can shift responsibility and behavior towards, renaming as necessary. More locations, more possibilities.
I don't get it. How does this discussion about data-holders address the exceptions to the rule? Or are there no exception cases?
will always be an exception to the rule since it's unlikely to have any more complex methods that get and set.
According to the rule, an object->method which used the DataHolder
would be moved onto the DataHolder
and this is probably wrong. Therefore, DataHolder
objects are a valid exception to the rule, and the argument moves onto Whether DataHolder
's are a GoodThing
or indicate a flaw in the design :-)
Or am I making this up?
Here I am, having received my copy of RefactoringImprovingTheDesignOfExistingCode
. I start reading it. On page 80, lo and behold, FeatureEnvy
. And of course, Kent and Martin have already addressed the Visitor and Strategy patterns as they apply to FeatureEnvy
. Next time, just tell me to read the book. -- HankRoark
"...if the document said: 'Person has the responsibility to hold address components. Envelope has the responsibility to append address components to make a printable address,' the document would be erroneous."
I cannot disagree with this statement more strongly. The design or implementation document is the source for the decisions regarding how the code must work. If there is a superior way discovered during coding then the design behind the document needs to be reworked to fit the better scheme. Changing code willy-nilly without a means of tracking what decisions were made and why leads to clutter and havoc. Documents first, code later.
Even if you are changing things. Especially
if you are changing things.