Connascence: a mis-step

I had a very interesting discussion today with Ross about my recent connascence/TDD posts. Neither of us was happy about either of the solutions to the corruption problem with the special offer object. Even with the cloning approach, it still seems that both the Checkout and the MultiBuyDiscount have to collude in avoiding the issue; if either gets it wrong, the tests will probably fail.

After a few minutes, we realised that the root of the problem arises from the MultiBuyDiscount having state, and we began to cast around for alternatives. At some point it dawned on me that the origins of the problem go right back to the first article and the first couple of refactorings I did.

Let’s revisit that early code. After I had fixed the CoV arising from making the first test pass, the Checkout looked like this:

public class Checkout {
  private int balance = 0;

  public void scan(String sku, int price) {
    balance = price;
  }

  public int currentBalance() {
    return balance;
  }
}

Then in the second article, after I had recycled the test to add multiple scanned items, the Checkout had become:

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;
  }
}

I had blindly made the Checkout keep a running balance, without pausing to realise that there was an alternative: I could have made it remember the scanned items, and only total them up in the currentBalance() method. Had I done so, it would then have been natural to calculate the discounts there too. And that could have been done by a special offer object that had no state. Thus avoiding all that faffing about with factories!

The problem I have with that, though, is that the code I wrote for the first (recycled) test was simpler than the alternative. I had no test, and no connascence, pushing me to remember the scanned items and total them later. At least, right now I can’t see that test or that connascence.

I feel that either my blind approach has led me to a poor design that will cost a lot to correct, or that I failed to spot something that could have prevented this. Food for thought…

Advertisements

6 thoughts on “Connascence: a mis-step

  1. What about the following example? We know we get a 20p discount for 2 A’s. Let’s assume another rule that states there are no discounts at all if at least one B was scanned (just for the sake of exposing the “problem”). In that case calculating the right balance in the method ‘scan’ would be harder, because for the test case “AAB”, the method would have to somehow undo the discount for two A’s. Now, alternative would be to save the potential 2A-discount as a field and apply it in the ‘currentBalance’ method. (that’s Connascence of Algorithm because every new type of discount forces me change both methods). Further, the rules could get so complex that all sorts of potential discounts would undo each other after every scan (Contranascence of discounts). On the other hand, if we calculate the discounts in ‘currentBalance’ we save ourselves the problem of undoing or shared algorithm across 2 methods. Does that make any sense?

  2. You may have fallen into a common trap when using TDD – expecting it to solve the problem rather than guiding your solution into a good implementation. TDD works well if you have a broad idea of how you want to solve the problem – however it will often fail at driving out the solution because it appears to force most people to thinking at the micro (rather than the macro) level. A classic illustration of this is summarised here http://www.infoq.com/news/2007/05/tdd-sudoku

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s