August 3, 2015

...Learn TDD with Codemanship

Refactoring Abstract Feature Envy

Feature Envy is a code smell that describes how a method of one object is really about a different object, evidenced by numerous feature calls to that second object. It suggests we've assigned this responsibility to the wrong object, and - in the simplest examples - we just need to move the method to where it really belongs.

A fiddlier variation on Feature Envy is what I call Abstract Feature Envy, illustrated by this example below:



The feedAnimal() method on Zookeeper is really not about the Zookeeper, it's about the Animal. But Animal is an interface. We can't move this method, because that requires an instance target to move it to, and we can't know until runtime what the parameter animal is an instance of - a Tiger, perhaps, or a Zebra?

To move feedAnimal, we have to add it to the interface, and have somewhere for the implementation that contains the feature envy to go. So we have to do a bit of a dance.

I might start by extracting a super-class from Tiger and Zebra, which would be where I'd eventually move the method:



Note that AnimalBase has no methods yet. That's just what I want. This will be the target for moving feedAnimal.

Okay, the next mini-step would be to add an instance of AnimalBase as a parameter to feedAnimal, passing in a new instance from the client code, so we have a target to move an instance method to:



Okay, now we can move the method to AnimalBase easy peasy. I left a delegate method to keep things simple at the client end:



Next, we make both parameter values the same target object - tiger or zebra - so that the object feedAnimal is acting on is effectively itself:



Next, we make it so AnimalBase implements the Animal interface - which will requires us to make it abstract, because we don't want it to implement the other methods - and remove that implements from the sub-classes Tiger and Zebra:



Next, we pull up feedAnimal from AnimalBase to the Animal interface:



Phew. Nearly there.

Next, since we made them the same object when we pass them in from the tests, I can substitute references to the animal parameter in the implementation with this:



Notice that I also removed the animal parameter, as it's no longer needed.

Then, last, we change the signature of the delegate method on Zookeeper back to what it was originally, as our temporary placeholder for AnimalBase (which is now the same thing as Animal as far as the tests are concerned) is not needed any more:



There will, no doubt, be slicker and quicker ways of achieving the same end result. And we might also like to think about how we might refactor to a "containment and delegation" solution instead, for the purists out there. But this gives us a half-way house to do that, should we wish to.







Posted 2 years, 5 months ago on August 3, 2015