Let’s take a closer look at the broader structure of refactoring, and maybe dive a little deeper into some of the tactics we referenced in the last post, with specific reference to Gilded Rose.
Testing is obviously key to being able to refactor, but the legacy code that needs the most work often won’t have tests, or be written in a way that’s easily testable. You can’t write detailed tests until the code is fixed, but you shouldn’t fix the code until you’re protected by tests. Characterization tests are the key: they let you get the signature of a chunk of code for a given input, and sound alarm bells when the output changes. For our refactoring workshop, we used characterization tests developed by Alastair McKinnell for the Gilded Rose kata using the Approvals and Rspec gems. Testing is key – don’t refactor without it.
DRY is important, of course. But sometimes, within the process of refactoring it helps to be more verbose and introduce duplication, as horrible as that feels. By taking bits of code that are applied to everything, and actually inserting them everywhere they need to be run, you can effectively “untie the knot” of code and move towards clarity and being able to extract useful methods and object functionality. Reworking code to follow parallel patterns may require some repetition, but it’s often a prerequisite for concision. When you’re refactoring, understanding the code is important, and any code worth refactoring is hard to understand. Make the code easier to understand, and simplify once the knot has been untied.
Restrict scope of refactoring
While it may seem attractive to start hacking at every possible problem, it is important to restrict the spillover of the changes you’re making unless you want your refactoring to spill ever onward. In the Gilded Rose kata, the restriction against changing the Item.rb file is a useful one, prompting creative hacks (Alistair’s adapter object is particularly cool), but also simulating an environment where your code relies upon things you can’t easily change. Similarly, you should take care when modifying public interfaces, like the name of the update_quality method; while it has an undeniably terrible name, and tons of side effects, touching it may have unexpected effects elsewhere. If you find yourself having to change your public interfaces, that’s a time to think about the risk of refactoring. If it isn’t worth the risk, focus on implementation methods. if it is, godspeed!
Proximity of concepts
While the control flow issues are the most obvious problems with Gilded Rose, possibly a bigger issue is proximity of concepts. Logic for dealing with specific types of items is scattered across the update_quality method, making extracting methods extremely difficult, and rendering the code almost unreadable. All ‘Aged Brie’ related logic should be grouped together. Doing so provides a means to both simplify the code and an easy transition to object-orientation.
Refactor entities into classes
No one got there this time, there just wasn’t enough time, but almost everyone was on the path. If you have a bunch of related entities with slightly different behavior, it’s a perfect time to refactor them out into objects using inheritance.
Learning them helps: many of the senior devs referred to established refactoring patterns. If you aren’t familiar with them, it’s a good idea to start learning them. The book Refactoring is an excellent guide to these patterns, and there’s even a ruby version. But if you don’t want to buy the book right now, there are tons of resources available online.
Refactoring is just one of the tools in your developer tool belt, but like a carpenter’s chisel it will likely be your main tool. Few workplaces provide endless opportunities for developing new code, and even fewer code bases are perfectly crafted. Expect to run across refactoring opportunities fairly frequently – and by honing your skill at refactoring, you will be in a position to act when it is advantageous. Remember the boy scout rule: “Always leave the campground cleaner than you found it.” Refactor often and you will reap the rewards.
We have posted our attempts at refactoring in my repo [http://github.com/k00ka/developer-testing] under various branches. You’ll find functional reworkings under the alex_and_mike_refactoring and david_initial branches. And a completed object refactoring under the david_final branch. Enjoy and we hope to see you in November!