(In the following, I will use abbreviations such as CoV for Connascence of Value. Because tired fingers.)
In the first article, I wrote a test and made it pass. That introduced some CoV (level 8) and CoM (level 3). I fixed the CoV first, by passing the offending value as a parameter and by tidying up the test.
In the second article I fixed (some of) the CoM. I did that by wrapping an integer value in a new domain class: Money. I noted that I ought to do the same for the strings that represent product codes, but I decided for the moment that it wasn’t serious enough to justify a whole new class. I created a new test (by recycling the first one) in order to flesh out the Money class a little.
In the third episode I wrote a new test — for multi-buy discounts — and made it pass in the simplest way I could. That introduced some more CoV, which I again fixed by pushing the values up the call stack into the test. This, in turn, created some CoP (level 5), and I decided that I didn’t have enough information to choose between several possible fixes. In order to create that information I added a new test (by again recycling an existing one) and made it pass naively. This again introduced some CoV, which I (yet again) fixed in time-honoured fashion. Doing so made the CoP worse, and I fixed it by introducing a new domain object to represent the discount rule. I noticed that, in so doing, I introduced some CoA (level 4).
In episode four I fixed two different examples of CoA — the first by moving a parameter from a method to a constructor, and the second by extracting an algorithm to a new method and moving that into the object that “wanted” it more. I then realised that this had, in turn, created yet more CoA. I eliminated that by introducing a factory object.
Finally, in the fifth part I backtracked. I removed the factory object and replaced it with a factory method that could clone an object. This solution felt more domain-oriented (to me).
Here’s a timeline showing the severity of the worst connascence in the code at any time:
Most of the time I created a spike in connascence when I made the next test pass. I’m taking that as a good sign, showing that I adopted Beginner’s Mind and didn’t think / design too much.
You can see the final state of the code collected together in this gist. Now I want to review that code from two points of view: What does connascence say about it, and what does my gut say about it?
First, the connascence. As ever, there is CoN (level 1) and CoT (level 2); these will probably never go away, and I’m fine with that. There is also that string representing the SKU code, which is CoM (level 3). That is worse now, because three different classes are coupled by this knowledge. But I think that’s it; I can’t see any different connascence (please let me know if you can!)
Second, what does my gut say about the code? I will need to add tests to cover situations in which there are several multi-buy discounts in force, and to cover different kinds of special offer (for example, buy any two of “A”, “B” and “C” and get the cheapest for half price). I am nervous about the Checkout having a direct dependency on MultiBuyDiscount. But I know that it won’t be difficult to generalise to a more realistic solution, particularly after I fixed the CoA and moved the discounting algorithm into the rule object itself.
I am uncomfortable about that Money parameter to scan(). I feel as if there should be a PriceList or somesuch, because at the moment it is only the test that associates SKUs with their prices. Conversely, the code as it stands is complete and consistent, and a PriceList lookup could be added as a decorator to the current Checkout, with its own set of tests.
I also worry about that Money parameter because the price of an “A” isn’t fixed during a single run of the Checkout. That feels “wrong”, but I have no requirement to back up that feeling. And indeed it could be argued that the business may eventually have reason to allow the price of an SKU to change during a single customer’s checkout process. So again, my gut may be worrying unjustifiably.
Finally, there is “duplication” between the test methods. Or is there? I have chosen to use the same discount rule each time, but that’s probably simple laziness; maybe I should randomize that? And while I might simplify each test method by extracting some shared fields, that would also couple them to each other in a different way. So again, my gut and the connascence disagree, and I find myself accepting connascence’s point of view.
My principal objectives with these articles were:
- to demonstrate that the strict connascence hierarchy can help in deciding what to refactor next;
- to show working examples of the main types of connascence; and
- to show some simple refactorings that can eliminate these types of connasence.
I hope I have also shown that connascence doesn’t have to be as scary as its name. For me, it is a far more useful tool than code smells. Give it a try, and write up your own experiences with it?