Kevin Rutherford's Blog, page 4

Connascence: a retrospective

In the previous five articles I have test-driven a classic code kata, using only connascence to guide me during the factoring steps. Here I will summarise the steps I took, and review the final code.

(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?

Connascence of Algorithm revisited

Last time I fixed two instances of Connascence of Algorithm. Then I realised that I had introduced some more, so I fixed that too. After I had put that post to bed I also noticed that I had missed some alternative solutions. This article puts that right.

Towards the end of the previous article I noticed that the following test would fail:

@Test
public void independentCheckouts() {
Money priceOfA = randomPrice();
MultibuyDiscount discount = new MultibuyDiscount("A", Money.fromPence(20), 2);
Checkout checkout1 = new Checkout(discount);
Checkout checkout2 = new Checkout(discount);
checkout1.scan("A", priceOfA);
checkout2.scan("A", priceOfA);
assertEquals(priceOfA, checkout1.currentBalance());
assertEquals(priceOfA, checkout2.currentBalance());
}

I fixed it by introducing a MultibuyDiscountFactory, so that the Checkout could always create its own instance of the discounter. And then I went for a walk, during which I realised that I was uncomfortable about having two classes — the rule and the factory — for one concept, and both with the same constructor parameters.

So, after much tramping across fields, I back out the change that introduced the factory. I now know that I can fix that Connascence of Algorithm without even touching the test: I will update the MultibuyDiscount so that it can be cloned by the Checkout:

public class Checkout {
//...
public Checkout(MultibuyDiscount discount) {
this.discount = discount.reset();
}
}
public class MultibuyDiscount {
//...
public MultibuyDiscount reset() {
return new MultibuyDiscount(watchedSku, discount, trigger);
}
}

(On method naming: I decided that “clone” wasn’t a domain term, whereas it seems reasonable to ask a discount rule to reset itself before use. I may well find an even better name in the future.)

Next time I plan to look ahead at what remains to be done in this code, and also to look back and reflect on the journey so far…

Connascence of Algorithm

This is the fourth post in a series in which I am test-driving a classic code kata, and using only the rules of connascence to tell me what to refactor.

If you have been following along, you’ll recall that my most recent refactor was to create a new domain object for MultiBuyDiscount, thereby alleviating a nasty case of Connascence of Position. The tests pass, so let’s review what connascence remains in this code:

Level 1, Connascence of Name, for example because various classes know the names of methods to call on each other. This is benign, and isn’t going away anytime soon.
Level 2, Connascence of Type, for example because the test knows which classes to instantiate. Similarly, this is largely benign in this code.
Level 4, Connascence of Algorithm. There are two problems here: Firstly, knowledge of the parameters of scan() persists across calls. This means that the caller (in this case, the test) has to know something about how the Checkout works, in order to get it to behave predictably. And secondly, the Checkout itself knows that the MultiBuyDiscount has a trigger, an SKU and a discount amount. The MultiBuyDiscount itself has no privacy, and if the design of that domain concept changes we would have to change code in the Checkout too.

I plan to fix these two instances of Connascence of Algorithm in the order described above. First, the coupling between the test and the Checkout.

The state of the Checkout instance now depends on the value of the parameters to the various calls to scan(). If I change the special offers while scanning the items in a single basket, the result is inconsistent behaviour. For example:

@Test
public void connascenceOfExecutionOrder() {
Money priceOfA = randomPrice();
Money firstTotal = new Checkout()
.scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 1))
.scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 2))
.currentBalance();
Money secondTotal = new Checkout()
.scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 2))
.scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 1))
.currentBalance();
assertEquals(firstTotal, secondTotal);
}

This test fails, which is surely a defect in the design. Thankfully the remedy is simple: I pass the special offer to the Checkout via its constructor, not via the scan() method:

public class Checkout {
//...
private MultibuyDiscount discount;

public Checkout(MultibuyDiscount discount) {
this.discount = discount;
}

public Checkout scan(String sku, Money price) {
balance = balance.add(price);
if (sku.equals(discount.discountSku))
discountItems++;
if (discountItems == discount.discountTrigger)
balance = balance.subtract(discount.discount);
return this;
}
}

This fixes the worst of the Connascence of Algorithm, which coupled the test to the state of the Checkout instance. Next I tackle the other instance of Connascence of Algorithm, between the Checkout and the MultiBuyDiscount. This is a case of “Feature Envy”, and I must admit that I think twice before fixing it at all. It could be argued that the code is reasonably okay as it stands, and that this coupling would only need to be fixed if  the business introduced a different kind of discount in the future. However, the code as it stands is aesthetically unpleasant, so I decide to press on.

I move the guts of the discount algorithm out of the Checkout:

public class Checkout {
//...
public Checkout scan(String sku, Money price) {
balance = balance.add(price);
balance = balance.subtract(discount.discountFor(sku));
return this;
}
}

and into the MultiBuyDiscount:

public class MultibuyDiscount {
private String watchedSku;
private Money discount;
private int trigger;
private int itemsSeen = 0;

public MultibuyDiscount(String sku, Money discount, int trigger) {
this.watchedSku = sku;
this.discount = discount;
this.trigger = trigger;
}

public Money discountFor(String sku) {
if (sku.equals(watchedSku))
itemsSeen++;
return (itemsSeen == trigger) ? discount : Money.ZERO;
}
}

So, that’s it — the Connascence of Algorithm is all gone. Well, not quite: In making this change I have only moved the connascence around! Suppose I create two Checkouts, but have them share a MultibuyDiscount? The following test fails:

@Test
public void independentCheckouts() {
Money priceOfA = randomPrice();
MultibuyDiscount discount = new MultibuyDiscount("A", Money.fromPence(20), 2);
Checkout checkout1 = new Checkout(discount);
Checkout checkout2 = new Checkout(discount);
checkout1.scan("A", priceOfA);
checkout2.scan("A", priceOfA);
assertEquals(priceOfA, checkout1.currentBalance());
assertEquals(priceOfA, checkout2.currentBalance());
}

The second customer gets a discount, despite only buying one “A”!

The discount object represents the state of the customer’s basket, because it remembers how many items of the trigger SKU have been scanned. So the client (again, only a test in our case, but it could be anything) must remember to create new discount objects each time it creates a new checkout. That’s Connascence of Algorithm. The question is: how to fix it?

I could ensure that each Checkout instance creates its own discount rule, so that they are never shared. I would have to be careful not to re-introduce the Connascence of Value, so it would still need to be the test that would know the rule’s details.
I could look for a way to split the MultiBuyDiscount into two pieces: one to know the business rules, and the other to keep a track of how many trigger SKUs have been scanned in the current customer’s basket. The Checkout could hold an instance of the latter, while the test holds the former.

When I look at these in detail I see that they are essentially the same solution. And since I’m working in Java, my options are fairly closely constrained. The rule object passed from the test into the Checkout’s constructor must be a factory that the Checkout can use to create its own personal discounter. I create a new factory class:

public class MultibuyDiscountFactory {
private String watchedSku;
private Money discount;
private int trigger;

public MultibuyDiscountFactory(String sku, Money discount, int trigger) {
this.watchedSku = sku;
this.discount = discount;
this.trigger = trigger;
}

public MultibuyDiscount create() {
return new MultibuyDiscount(watchedSku, discount, trigger);
}
}

and then use it in the Checkout:

public class Checkout {
//...
public Checkout(MultibuyDiscountFactory discount) {
this.discount = discount.create();
}
}

Well, that was a lot more work than I expected when I first noticed the Connascence of Algorithm (aka Feature Envy) back in the previous post! Next time I will revisit the decision to use a factory here…

Connascence of Position

This is part three of a series in which I am test-driving a classic code kata, and using only the rules of connascence to tell me what to refactor. Last time I continued working on the checkout kata, and I fixed some Connascence of Meaning by introducing a new class to represent the domain concept of Money. Today I want to take that same code a little further to explore what happens when I continue simply responding to the connascence I see.

Here’s the code so far:

public class CheckoutTests {
@Test
public void basicPrices() {
Money priceOfA = randomPrice();
Checkout checkout = new Checkout();
Money priceOfB = randomPrice();
checkout.scan("A", priceOfA).scan("B", priceOfB);
assertEquals(priceOfA.add(priceOfB), checkout.currentBalance());
}

private Money randomPrice() {
int pence = new Random().nextInt(1000);
return Money.fromPence(pence);
}
}

public class Checkout {
private Money balance = Money.ZERO;

public Checkout scan(String sku, Money price) {
balance = balance.add(price);
return this;
}

public Money currentBalance() {
return balance;
}
}

It’s time now to add special offers. I add a test that says we get a discount of 20p if our basket contains two “A” items:

@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Checkout checkout = new Checkout();
checkout.scan("A", priceOfA).scan("A", priceOfA);
assertEquals(priceOfA.add(priceOfA).subtract(Money.fromPence(20)), checkout.currentBalance());
}

I make this pass naively, by checking the current scanned item against its predecessor:

public class Checkout {
private String previousSku;

//...

public Checkout scan(String sku, Money price) {
balance = balance.add(price);
if (sku.equals(previousSku))
balance = balance.subtract(Money.fromPence(20));
previousSku = sku;
return this;
}
}

This feels like a huge hack, and quite a lot of code to write for one failing test. But on the plus side, the test now passes, so I take stock ready to refactor. As before, I have introduced some Connascence of Value, because both the Checkout and the test know about that -20 for the discount amount. This is level 8 (of 9) connascence, so I better remove it before I do anything else. As before, I can fix it quickly by passing it into the Checkout from the test:

@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money discount = Money.fromPence(20);
Checkout checkout = new Checkout();
checkout.scan("A", priceOfA, discount)
.scan("A", priceOfA, discount);
Money expectedTotal = priceOfA.add(priceOfA).subtract(discount);
assertEquals(expectedTotal, checkout.currentBalance());
}

As a brief aside, I should mention that this code feels awful to me: All of my instincts and experience are screaming that these Money values should have been passed to the Checkout’s constructor, and not via the scan() method as above. I suspect that’s because I do a good amount of domain modelling in my head as I’m coding. For these blog posts I am deliberately “following the rules”; I have no idea whether this will lead to code I like, and I guess that’s half the fun. Anyway, allons-y…

That last refactoring eliminated the Connascence of Value, which is good. But now I have Connascence of Position instead. That’s because both the test and the Checkout need to agree on the order in which those two Money values are passed into the scan() method. The problem here is that the compiler can’t help, so it would be quite easy for the caller to pass them in the wrong order. While that’s not a big problem for the test, it could be financially disastrous if any other client of the Checkout passed the values in the wrong order.

(Connascence of Position is level 5 — more serious than Connascence of Meaning, but less serious than Connascence of Value. One of the things I find most appealing about connascence is this strict hierarchy: I always know which coupling to address next.)

I can think of a few ways to tackle this Connascence of Position:

Move either Money parameter to the Checkout’s constructor. This would fix the Connascence of Position because there would no longer be a single method with two Money parameters.
Wrap the SKU and its price in a single object. This is attractive, because it has the early scent of a PriceList. This might go some way towards quieting the domain modeller in my head.
Pass the discount via a new method on Checkout. This would remove the Connascence of Position, but would introduce a “setter” — a method that configures the Checkout after it has been constructed. That would be a terrible idea, because it forces the object’s user to remember the order in which these methods must be called. That would be Connascence of Execution Order — level 6 on the scale of 9..
Wrap the discount in a new SpecialOffer object. Wrapping a single object inside another, just to avoid Connascence of Position, seems overkill.
Wrap the two Money values in a dictionary. One of the primary ways of eliminating CoP is to use named parameters. I’m working in Java today, so that isn’t an option. I could fake it by creating a Map, but these two values don’t feel as it they belong inside the same data structure.

Now I see these options, I realise that all of this is premature. I could take the code in any number of different directions at this point, and I don’t have enough information to decide which will be most appropriate. So I need the next test, in order to make this decision in context. I decide to recycle the test, adding a different item between the two As:

@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money priceOfB = randomPrice();
Money discount = Money.fromPence(20);
Checkout checkout = new Checkout();
checkout.scan("A", priceOfA, discount)
.scan("B", priceOfB, discount);
.scan("A", priceOfA, discount);
Money expectedTotal = priceOfA.add(priceOfA)
.add(priceOfB)
.subtract(discount);
assertEquals(expectedTotal, checkout.currentBalance());
}

As always, I make this pass without thought of any design aesthetics:

public class Checkout {

//...
private int countOfAs = 0;

public Checkout scan(String sku, Money price, Money discount) {
balance = balance.add(price);
if (sku.equals("A"))
countOfAs++;
if (countOfAs == 2)
balance = balance.subtract(discount);
return this;
}
}

It seems I always manage to introduce Connascence of Value! This time, both the test and the Checkout know which product is discounted, and how many items trigger that discount. As before, I fix that by passing the duplicated knowledge via a parameter:

@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money priceOfB = randomPrice();
Checkout checkout = new Checkout();
Money discount = Money.fromPence(20);
checkout.scan("A", priceOfA, "A", discount, 2)
.scan("B", priceOfB, "A", discount, 2)
.scan("A", priceOfA, "A", discount, 2);
Money expectedTotal = priceOfA.add(priceOfA)
.add(priceOfB)
.subtract(discount);
assertEquals(expectedTotal, checkout.currentBalance());
}

At this point I can take stock. I have made the Connascence of Position worse, because now the caller also has to know which of those two SKU parameters was scanned and which is the indicator for the special offer. I have also created other connascence; more about that in the next post. For now, Connascence of Position is still my biggest problem, and my options for fixing it are now suddenly much more clear: The three discount-related parameters represent aspects of the same domain concept: a MultiBuyDiscount. I can defeat the Connascence of Position by wrapping those values in a new class:

@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money priceOfB = randomPrice();
Checkout checkout = new Checkout();
MultibuyDiscount discount = new MultibuyDiscount("A", Money.fromPence(20), 2);
checkout.scan("A", priceOfA, discount)
.scan("B", priceOfB, discount)
.scan("A", priceOfA, discount);
Money expectedTotal = priceOfA.add(priceOfA)
.add(priceOfB)
.subtract(Money.fromPence(20));
assertEquals(expectedTotal, checkout.currentBalance());
}
public class Checkout {
//...

public Checkout scan(String sku, Money price, MultibuyDiscount discount) {
balance = balance.add(price);
if (sku.equals(discount.discountSku))
discountItems++;
if (discountItems == discount.discountTrigger)
balance = balance.subtract(discount.discount);
return this;
}
}

As before, by fixing some connascence I have discovered a new domain object and given it a name and a real existence. But as yet, that new object is simply a data container: it has three public fields, and all of the processing that involves them is still in the Checkout. This situation breaks the strong suggestion of Demeter (and it looks pretty smelly too). I’ll need to fix that soon.

However, this post has grown much longer than I like, so I’ll stop here. Next time I will review this code for more connascence…

Connascence of Meaning

In the previous article I wrote a test, made it pass, and then refactored away the strongest coupling. That coupling took the form of some Connascence of Value between the test and the Checkout. Later, after the excitement of publishing the post had died away, I realised there was still some non-trivial connascence in the code. Today it’s time to fix that.

Here is the test I finished with last time:

public class CheckoutTests {

@Test
public void basicPrices() {
int price = randomPrice();
Checkout checkout = new Checkout();
checkout.scan("A", price);
assertEquals(price, checkout.currentBalance());
}
}

First, let’s review the connascence in this code. (And please let me know if I have missed any this time too!)

Connascence of Name: The test knows the names of the methods to call on the checkout object. This is level 1 (of 9) on the connascence scale — the weakest and least damaging form of coupling.
Connascence of Type: The test knows which class to instantiate, and it knows the types of the various method parameters. This is level 2 on the scale, and is thus also relatively benign.
Connascence of Meaning: Both classes know that we are representing monetary values using ints, and products using strings. This is level 3 on the scale; while still relatively harmless, it is worth removing before the code gets too much bigger.

I think it is worth taking a moment to examine why I feel that this is Connascence of Meaning and not purely Connascence of Type. Yes, the test and the Checkout have to agree on the types of the parameters and return values, but I think there’s more: both also have to agree on what those values mean. Does that int represent pence, pounds, euros? Could that product string in future become a guid, or a barcode? The test and the Checkout are coupled by their shared knowledge of how those domain concepts (monetary values and product codes) are represented and interpreted. If I let that knowledge proliferate, it could become difficult to change either of them. I may incur subtle bugs if either is interpreted or represented in a different way somewhere in the application in future. This is the Ariane 5 bug in waiting.

(I clearly remember the pain of encountering a large .Net financial application in which monetary values were represented as ints, decimals or floats in various areas of the code. The compiler “helped” by casting between these representations, so the inevitable bugs were only discoverable at runtime.)

So although I have “only” Connascence of Meaning, I want to nip it in the bud early. The cost of doing that is low right now, and the cost of not doing it could escalate quickly later. So, on with the motley…

I’ll deal with the money first.

As before, I can weaken the connascence by bringing the “ends” of the coupling together in one place. But this time I cannot easily inject a value into the Checkout, because the problem here is one of types. So instead, I create a new type and hide the int inside it. Money will be the only class that knows how monetary values are stored:

The change to the test is simple:

public class CheckoutTests {

@Test
public void basicPrices() {
Money price = randomPrice();
Checkout checkout = new Checkout();
checkout.scan("A", price);
assertEquals(price, checkout.currentBalance());
}

private Money randomPrice() {
int pence = new Random().nextInt(1000);
return Money.fromPence(pence);
}
}

Similarly the checkout now uses Money objects instead of ints:

public class Checkout {
private Money balance;

public Checkout scan(String sku, Money price) {
balance = price;
}

public Money currentBalance() {
return balance;
}
}

And finally, here is the new Money class itself:

public class Money {
private int pence;

private Money(int pence) {
this.pence = pence;
}

public static Money fromPence(int pence) {
return new Money(pence);
}
}

Note that I clearly document the meaning of the parameter to Money’s factory method, while the actual constructor is hidden from view. This gives me more control over how Money objects are created, and helps keep the internal representation private.

Next, I will move on to do something very similar with product codes. But before I do that I just want to expand on (ie. recycle) the current test so that it can cater for multiple items:

@Test
public void basicPrices() {
Money priceOfA = randomPrice();
Checkout checkout = new Checkout();
Money priceOfB = randomPrice();
checkout.scan("A", priceOfA).scan("B", priceOfB);
assertEquals(priceOfA.add(priceOfB), checkout.currentBalance());
}

This simple change forces Money to acquire a wee bit more richness:

public class Money {
public static final Money ZERO = new Money(0);
private int pence;

private Money(int pence) {
this.pence = pence;
}

public static Money fromPence(int pence) {
return new Money(pence);
}

public Money add(Money other) {
return new Money(pence + other.pence);
}

@Override
public boolean equals(Object other) {
Money m = (Money) other;
return pence == m.pence;
}

@Override
public int hashCode() {
return new Integer(pence).hashCode();
}
}

I like this. By fixing some Connascence of Meaning I have “discovered” a domain concept, and it has quickly fleshed out with a perfectly sensible set of behaviours. This would be perfect if only Java allowed operator overloading…

Now I take a look at those strings that represent the products. Arguably the Connascence of Meaning here is less severe, because as yet the Checkout isn’t using the product code passed to it. I could leave this one until I have tests that force Checkout to check that string. I decide to do just that, because I can’t predict when that will be, or how the code will turn out in that uncertain future.

So, to summarise, I fixed Connascence of Meaning by hiding the choice of data representation for monetary values. If I later decide to switch to pounds with decimal places representing the pence, I only have to change the Money class. The rest of the application is isolated from such change.

This feels like a lot of work for just one test. Was it justified? Well, first I note that it is two tests, because I recycled one. Secondly, I have named a domain concept and the very simple test I have has already caused it to flesh out with some behaviours. I am happy with this code, and I am comfortable that it satisfies Extreme Normal Form. I also expect that some people will disagree…

Connascence of Value

Connascence is a way of describing the coupling between different parts of a codebase. And because it classifies the relative strength of that coupling, connascence can be used as a tool to help prioritise what should be refactored first.

For example, let’s tackle @pragdave‘s  classic Back to the Checkout kata in Java. My first test checks that we can scan a single item and calculate the total correctly:

public class CheckoutTests {

@Test
public void basicPrices() {
Checkout checkout = new Checkout();
checkout.scan("A");
assertEquals(50, checkout.currentBalance());
}
}

Now I make it pass in the simplest way I can think of:

class Checkout {
public int currentBalance() {
return 50;
}

public Checkout scan(String item) { }
}

Clearly these two classes are now coupled (if they weren’t, the test wouldn’t pass). But is that coupling good or bad?

I can see three four kinds of connascence between the test and the production code:

Connascence of Name, because the test knows the names of the methods to call on the checkout object. This is level 1 (of 9) on the connascence scale — the weakest and least damaging form of coupling.
Connascence of Type, because the test knows which class to instantiate. This is level 2 on the scale, and is thus also relatively benign.
Connascence of Meaning, because both classes know that we are representing monetary values using ints. (I missed this first time around — d’oh!)
Connascence of Value, because both the test and the Checkout know the price of item “A”:

The Connascence of Value here means that the tests will break if I change the price of item “A”; I definitely wouldn’t want to release this into production.

Connascence of Value is level 8 on the scale of 9 types of connascence. The scale defines seven weaker forms of coupling, and only one more serious kind. I can use that model to help me prioritise the Refactor step in my TDD cycle: Connascence of Value is a serious problem, and should be fixed before I do anything else. The only question is: how?

The first thing I note is that connascence is weaker with proximity, which means that either of the following options would be preferable:

Thus, if I can move knowledge of the price of “A” so that only one of my classes has it, then the effects of the coupling are greatly diminished.

I can get some help from SOLID here, because the Dependency Inversion Principle also tells me that this code has a problem. The DIP says that we should depend on abstractions, not on details. And yet here I have a test that only works due to its knowledge of one of the details inside the production code.

The DIP (and @jbrains) also tells me what to do next: I should move the detail up towards the tests. That means I need to change the Checkout so that the test injects the value 50 via a parameter. I could pass it in via the scan method:

public class CheckoutTests {

@Test
public void basicPrices() {
Checkout checkout = new Checkout();
checkout.scan("A", 50);
assertEquals(50, checkout.currentBalance());
}
}

Alternatively I could inject it via  the Checkout’s constructor:

public class CheckoutTests {

@Test
public void basicPrices() {
Checkout checkout = new Checkout(50);
checkout.scan("A");
assertEquals(50, checkout.currentBalance());
}
}

Either way, I have now removed the Connascence of Value between the Checkout and the test: I can change the price of item “A” by changing only one method.

The worst of the coupling is now gone, but I can do better. There is still Connascence of Value, albeit very localized, within that test method. Is it worth fixing?

I like my tests to be expressive and easy to read. I wouldn’t want to extract the value 50 to a constant, for example, because I would then have to scan up and down through the test class to discover exactly what the test was doing. But equally, that magic value 50 makes me a little nervous. Does it have business significance? Not in this case, and a new team member might not pick that up.

In cases such as this I like my tests to use random values, to help ensure that the code under test hasn’t made any unfortunate assumptions. So I replace that 50 with a call to a random price generator:

public class CheckoutTests {
@Test
public void basicPrices() {
Checkout checkout = new Checkout();
checkout.scan("A", randomPrice());
assertEquals(randomPrice(), checkout.balance());
}
}

But now the test is broken again, and that Connascence of Value is the guilty party, telling us that the two values need to be the same. I fix it by replacing the Connascence of Value by Connascence of Name:

public class CheckoutTests {
@Test
public void basicPrices() {
Checkout checkout = new Checkout();
int priceOfA = randomPrice();
checkout.scan("A", priceOfA);
assertEquals(priceOfA, checkout.balance());
}
}

To summarise, I find connascence useful in guiding my refactoring efforts during the TDD cycle. In this case, I weakened the coupling between the code and test by pushing details up the call stack; then I removed the Connascence of Value altogether by replacing it with Connascence of Name.

Date arithmetic

This morning I got up at eight minutes past six. So what, you ask? Well, that means I got out of bed at 06:08 10/12/14*, which is a very nice arithmetic progression. That is, today’s date is a series of numbers with a constant difference (in this case, the constant difference is 2).

Question: Which dates (and times, if you wish) next year will form arithmetic progressions? And which, if any, will form a geometric progression (in which each term after the first is found by multiplying its predecessor by a fixed constant)?

*Unless you live in the US — in which case, pretend today is October 12th.

Priorities

A thought, in response to comments and discussion in various places, sparked by TDD for teams:

In a large software application that is worked on by many (> 2) people, which is more important: “good” design or consistency?

TDD for teams

I strongly suspect that TDD for teams is different than TDD for individuals.

There’s a proverb in software development to the effect that:

“TDD is a design technique, not a testing technique”

I agree. But that doesn’t mean it’s the only design technique we need. And it doesn’t also mean that everyone will use it equivalently or get the same results with it. For example, take a look at the approaches used by Seb Rose, Ron Jeffries and Alistair Cockburn to solving the Letter Diamond kata. (Click on their names to read their blog posts, then hop back here if you still have any energy left.) They each tackled the same set of requirements in completely different ways. Each used TDD, and yet their designs were completely different.

In fact, while I was drafting this post, George Dinwiddie had a go too, and Ron made a second attempt. So now we have 5 different design approaches to one toy kata from 4 developers, all in the space of one weekend. (And there are probably more that I haven’t heard about.) Of course, it would have been weird if they had all produced identical designs. We are all different, and that’s definitely a good thing. But I worry that this can cause problems for teams doing TDD.

A couple of years ago I remember doing a performance kata in which I paired with Mark Kirschstein to tackle the supermarket checkout kata. My role was to write the tests and Mark’s was to make them pass. At the beginning of the session I made the bold claim that I could get Mark to implement the checkout in a way he had never seen before, with a design that he had not conceived. The audience, and Mark, were skeptical. They were used to thinking of the problem as framed by the tests in Dave Thomas’ original problem statement. And so they were expecting the first test to be something like this:

public class CheckoutTests {
@Test
public void oneA() {
Checkout checkout = new Checkout();
checkout.scan("A");
assertEquals(30, checkout.total());
}
}

But in fact my first test was this:

public class CheckoutTests2 implements ScannerListener {
int priceReported = 0;
String productReported = null;

@Test
public void oneA() {
Scanner scanner = new Scanner(this);
scanner.scan(new SKU("A"));
assertEquals(30, priceReported);
assertEquals("A", productReported);
}

public void itemScanned(String product, int price) {
productReported = product;
priceReported = price;
}
}

(expressed using the SelfShunt pattern, as anyone who has attended any of my training courses will recognise immediately). Mark, to his surprise, was gradually led to creating a checkout implementation based on notifications and listeners, and with no getters or setters.

[Supplementary challenge: implement the supermarket checkout kata without conditionals!]

While this was (I hope) great theatre, there’s a deeper message in all of this when it comes to whole teams working on non-trivial problems: If a team is to produce software that exhibits internal consistency and is understandable by all of its developers, then somehow these individual differences must be surrendered or homogenized in some way. Somehow the team must create designs — and a design style — that everyone agrees on.

Does it make sense for teams to operate as isolated pairs, each programming away on their specific tasks, without regard for how their designs will integrate with those of their team-mates? I see too many teams doing just that; ditching design sessions on the basis of reading TDD books and blogs in which a single person or pair did all of the thinking. I see far too many codebases in which personal style is the major design force; where the same domain concept is implemented in two or more radically different ways; where duplication is thus very hard to spot, and even harder to remove.

Perhaps we need more published examples of team-based TDD, showing techniques for creating and sharing Just Enough Design Up FrontTM.

XP includes the key practices of Coding Standard and System Metaphor; are they enough to solve the problem? How can pairs Refactor Mercilessly if there is no team consensus as to what constitutes “good” and “consistent”?

What does your team do to balance the needs of “enough design” and “too much design up front”?

Happy numbers again: spoilers

If you’ve tried the Happy numbers kata you may have noticed a couple of things about the algorithm. Firstly,

13 => 32 + 12 => 10 => 1

is the same as

31 => 12 + 32 => 10 => 1.

That is, happiness and unhappiness are preserved under all permutations of the digits. So if you have calculated the status of 134, you also know the status of 143, 314, 341, 413 and 431!

And secondly, zeroes have no effect on the outcome. So 103 is just as happy as 13; and similarly 1300, 1000300000 and 30010 are all happy.

In a sense, that means it is not meaningful to ask how many numbers can be examined in 5 seconds, say — because the answer is always infinite! So let’s redefine the kata:

Write a program to report the number of happy numbers in the range [1, n]. What is the largest n you can deal with in 5 seconds?

Some interesting questions now come to mind:

Where does the time go? Is it faster to calculate the happiness of a number than it is to create all permutations of its digits?
What is the best data structure for holding the results? Can a recursive or functional algorithm compete with a procedural approach?
What is the best way to deal with numbers containing zeroes?
Are the answers to these questions dependent on programming language?
Is this really an algorithm on the integers, or is it an algorithm on lists of digits?

Finally, something quite bizarre. The observations above regarding zeroes and digit permutations mean that we can acquire the answer to the challenge while examining fewer integers. How many fewer?

My rough calculations suggest that we only need to examine 54 of the integers below 100 in order to have complete information about every number in the range 1-100. Thats 54%. But we only need to examine 229 of the first 1000, which is 22.9%. To cover 1-10000 we only need to examine 714 numbers, and for 1-100000 we only need to examine 2002 numbers — ie. 2% of them! It would appear that this algorithm gets significantly faster as we move to larger and larger data sets. How much difference does this make to what your implementation can achieve?