Rational behavior and the Gilded Rose kata…

The following is based on a reply to an internal post that I almost wrote this morning, before I decided that it might be of more general interest. It will take a little time to get to my point so perhaps this would be a good time to grab whatever beverage is at the top of your beverage preference backlog.

Are you back? Okay, I’ll get started…

A while back my team spent an afternoon working on the Gilded Rose kata. We used it to focus on our development skills using a pairing & TDD approach. This kata involves taking a very tangled routine (deliberately tangled AFAICT) and extending it to support a new requirement. It is traditionally viewed as an exercise in refactoring, and in the introduction to the kata, I suggested that my team work on pinning tests before refactoring or adding new functionality. My team found the kata to be quite enjoyable, as it involved a puzzle (unraveling the code) and the opportunity to work on code that is much worse than our existing codebase.

This week, another team did the String Calculator kata (a nice introduction to katas and one that works well with pairing/TDD), and somebody suggested that Gilded Rose would be a good next step. I started to write a quick reply, which became a longer reply, which then evolved into this post.

My initial reply was around how to encourage people to take the proper approach (if it’s not obvious, the “proper” approach is the one I took when I last did the kata…) – which was “write pinning tests, refactor to something good, and then add new functionality”. When writing it, however, it seemed like I was being overly prescriptive, and it would make more sense to remind people of a general principle that would lead them to a good approach.

At this point, I realized that I had a problem. Based on the way the kata is written, I was not longer sure that my approach *was* the proper choice. The reason is that the kata provides a detailed and accurate description of the current behavior of the system and the desired changed behavior. After a bit of thought, I came up with four alternatives to move forward:

1. Attempt to extract out the part of the code that is related to the area that I need to change, write pinning tests for it, and then TDD in the new functionality.
2. Write pinning tests on the current implementation, refactor the code, then add new functionality.
3. Write pinning tests on the current implementation, write new code that passes all the pinning tests, then add new functionality.
4. Reimplement the current functionality from scratch using TDD and based on the detailed requirements, then add new functionality.

Which one to choose?

Well, the first one is something I commonly do in production code to make incremental changes. You can make things a little better with a little investment, and many times that’s the right choice WRT business value. If you want more info on this approach, Feather’s “Working Effectively With Legacy Code” is all about this sort of approach. It’s one of the two books that sits on my desk (anybody want to guess the other one?).

Presuming that a kata that I finish quickly isn’t really my going, that leaves me with three options. My experience says that, if I have the Gilded Rose code and a complete specification of the desired behavior, I’m going for the last option. The problem isn’t very complex and, doing TDD is going to be straightforward, so I’m confident that I can get to an equivalent or better endpoint with less effort than dealing with refactoring the existing code.

That conclusion was a bit surprising to me, given that this is commonly thought to be a refactoring kata, but it’s the logical outcome of having a perfect specification for the current functionality available. Last year I had the chance to fix a buggy area of code that handled UI button enabling/disabling, and I took this exact approach; I knew what the functionality would be, wrote a nice clean class & tests, and tossed the old code out. Worked great.

In reality, however, this case is somewhat rare; most times you just have the code and your requirement is to add something new without changing the existing behavior, whatever it might be. Or, you have some requirements and some code, but the requirements are out of date and don’t match the code. So… I think the Gilded Rose kata would be a lot better if you didn’t have the description of the current behavior of the system, because:

1. That’s when pinning becomes important, to document the existing behavior.
2. Automated refactoring becomes more interesting because it allows me to make *safe* changes even if my tests aren’t perfect.
3. I get led towards these approaches automatically.

If I don’t have the description, I’m going to choose between the last two approaches. The tradeoff likely depends on how good my pinning tests are; if they are perfect I should just choose the option that is quicker. If they are less than perfect – which is generally going to be the case - then doing it through refactoring is likely a better choice.

Or, to put it another way, the effort to write a reasonable set of pinning tests and refactor the code is generally going to be less than the effort to write a perfect set of pinning tests and write the code from scratch.

Comments

• Anonymous
  July 11, 2014
  This kata is actually a whole class related to how to both be a good developer (ie. the refactoring and implementing a new feature part) and how to build a successful company (ie. the goblin in the corner part). In the long run, a company that has that goblin in the corner might not fail, but it will invariably have internal problems and the solution to this is to fire that goblin in the corner before he gets too big. What's that you say? He knows everything about the code and firing him removes a huge chunk of our internal knowledge? Well, then the solution is to that is to not get painted into that corner. Don't let any one employee sit on core information in such a way that firing that person will get you (the company) in trouble. Press for documentation, documentation, documentation, and then for pair programming or similar techniques to get the critical knowledge of one employee "copied" into others.

• Anonymous
  July 11, 2014
  As for the Kata approach, it depends on your goals. For production code, pick choice 1. Attempt to extract out the part to change, write tests for it, then change it. Minimal changes. Business is rarely interested in that big cleanup anyway. They have something that works, except that they need some extra functionality that currently isn't implemented. They're not interested in hearing about code cleanup and refactoring and all that nonsense. As for theoretical approach, choice 4 is ideal. How better to test if your specification is up to date than by reimplementing everything from scratch, using a document that is specifying how everything is supposed to work as a guideline? Presumably you have tests in place that tests your specification and then some. For a "kata" approach, it depends on what the kata is trying to teach. Refactoring? Then do incremental changes. Understanding? Then do a rewrite. Bugfixing? Then debug and fix as needed. The correct answer for a real scenario is probably a mixture of all the choices. Extract what you need to change if you cannot justify changing neighbouring code (for whatever reason). Rewrite from scratch if specifications are good and current implementation is bad, but most code you're going to deal with lies somewhere in between. It's not good, but it's not all bad either.