There is a particular case where you want to make things less simple as a starting point. For example, in C3 there is a hierarchy that is particularly hard to refactor. We have an amazing implementation of Composite that supports our various forms of Bin. Bin has a collection of all Parts ever put in there over all time. Generally all we ever do is balance the bin back over a calendar or fiscal year boundary. I'd like to replace the current Bin with a linked list, or some such, of some kind of object that knows its balance. Then a calendar year balance would never look back past the previous Bin, saving linear time and allowing for archiving of sufficiently old data.
We've tried several times to make changes like this to the Bin Composite hierarchy. It's so complicated and intertwingled, we can never make the tests all run.
What we're going to do next time is first refactor Bin out into its own hierarchy and make the tests run that way. This will involve duplicating a lot of methods - but only those that are really needed.
Then, we'll implement the new LinkedBin?
, refactor it into the system, and make that work.
Then, if it looks worthwhile, we'll refactor LinkedBin?
back into the Composite.
The effect will be that in the intermediate state, the system is less "simple", less OnceAndOnlyOnce
, than it was. But when we're done, we'll have function we have been unable to implement because of the extremely low energy of the Composite.
At some future time someone might write DarkSideOfCompositePattern
. -- RonJeffries
Yes. (free association) You cock the gun before you shoot... A slug hunches its back before it moves forward. Two modes of refactoring.. make it simpler or gear up to do something different. -- MichaelFeathers
Yep. (more free association) You cock the gun, hand trembling. A slug hunches its back before moving forward to avoid the slug. Blam. You just refactored that slug all over the sidewalk. Talk about low energy. Never lower. His momma won't know him. Now what?
Is implementing a LinkedList
? The reason I ask this is because it sounds like it has been in the system for a while. Maybe you just need to cache the calendar year and fiscal year balances. Have your performance measurements shown that this is a bottleneck in your system? -- JeanineDeGuzman
Note that this page is an example in reply to a comment/question in ReFactor
to the effect that refactoring is "always" used to make the code simpler. This page points out an exception, that you may have to make the code more complex in order to accomplish the refactoring. So it's to an extent a made-up stretch example. But a real problem:
Have our performance measurements shown? Of course, isn't that the Extreme rule? Is it critical? Not yet?
the simplest thing? Don't know, maybe you can think of something simpler. It turns out that a typical Bin gets one Part added to it each pay period, read month, biweek, or week. In GemStone
, as you know, this means that the collection of Parts gets copied to new space every period, just to add that one part. So all the Bins that get used get copied to new space every period. So now clustering is blown and we have to recluster.
If instead each new part were [in] some kind of a "LinkedBin?
" that could be written to new space, and point to the old, and if each one cached the YTD balances so far, then the next YTD is just the local balance plus the YTD cached in the nearest old one. You could get any YTD in a maximum of two I/Os and avoid all the copying and clustering, which takes way over two I/Os now.
But again, the real point isn't that we'd necessarily use that particular solution, but rather that the intimate and quite elegant Compositing of all these classes makes it difficult to add a new element to the bunch. So one way to proceed is to extract the element you want to refactor out of the bunch, intentionally violating OnceAndOnlyOnce
, then refactor it to be what you want, then refactor it back into the bunch.
This particular form of refactoring adds complexity rather than reducing it. That's uncommon but sometimes necessary ... that was the real point here. Thanks for asking. -- RonJeffries
Based on my understanding so far, to compute a calendar year balance, you need a certain collection of parts. It is currently a pain to get these parts because, as you write, "Bin has a collection of all Parts ever put in there over all time."
What if your composite can maintain internal iterators, such that when clients need to access certain collections of parts, they can simply ask the composite for the proper iterator? These internal iterators will already know what collections they must iterator over, if you talk to them while adding and removing elements from the composite.
If I misunderstood the problem, please excuse me - I had to read it a few times and may well have missed the actual problem you are having.
I'm also quite interested in your experience of the DarkSideOfCompositePattern
. Can you explain some of the darkness in this particular case? -- JoshuaKerievsky
This sounds somewhat like a pattern I dimly recall from somewhere, called ArchitecturalSubstitution
, somewhat similar to the SubstituteAlgorithm?
You have a set of classes that implement an algorithm in a manner that turns out to be resistant to change. You have a notion of how a new set of classes implementing the same algorithm, with a different structure, would be more amenable to change. You implement the second algorithm, test that it is functionally equivalent to the old one, then "throw a switch" to move over from the old to the new; when this works, you remove the old algorithm. This pattern implies in a temporary violation of OAOO, which eventually gets restored. -- LaurentBossavit
Perhaps the problem isn't factoring out the bins, but the parts. Under the presumption that the bin's inventory is dated, the Bin Pattern could contain a CompositePattern
of Periods (year, month, day, pay period, etc) where the leaves point to your Parts. Then to total a bin over an arbitrary period use a modified VisitorPattern
that first locates the specified Bin(s) and then the specified Period(s) and finally the specified Part(s). Then use the appropriate attributes of your part. For example, something like:
See also AddingComplexityCanHelp