Introducing New Features - Done Right

One of my customers offers articles in campaigns to his customers and allows orders for these campaigns. Once upon a time I was involved in the creation of the specific web application with some fellow students. And I am still engaged in maintaining and extending the system.
So far there was the restriction that there are no temporally parallel campaigns, they had to be in strict chronological order. Now my client wants to offer several campaigns at a time. Up to now, the concept of a "campaign" was hidden within a GUI component, namely a JavaServer Faces backing bean. So the following requirements had to be fulfilled:

• Present multiple, parallel campaigns to the customer
• Select one of those campaigns
• Present the order process for the selected campaign

So first of all, as an apprentice of the Acceptance Test-Driven Development community, I wrote an acceptance test for the new feature (slightly simplified):

Given there is an active campaign C with product P
And there is an active campaign D with product Q
When I order 17 pieces of P
And I order 42 pieces of Q
Then I want to receive 17 pieces of P
And 42 pieces of Q

Afterwards I automated the acceptance test as an end-to-end test using Selenium/Webdriver 2. Good boy!

The Mistake

I carried out the necessary implementation steps in the wrong order.

1. First of all I blew up all of my unit / integration tests by introducing a list of the former unknown concept "Campaign". Afterwards it took me more than a week to move the functionality from the backing bean to the new Campaign class and refit the existing tests.
2. Bad enough, but I could do worse. I spotted several dirty parts in the existing nested objects of Campaign, respectively the backing bean (e.g. an Order or an OrderLine). And I started to fix them immediately: specialize and devide some functionality in subclasses, generalize other parts and so on. But I didn't formulate and rearrange my unit tests first, I just changed the production code. So while my tests were already failing, I changed the production code even further, breaking more and more tests. So what should have been a small change ended up in more than a week of work for a single commit...

So what went wrong?

While succeeding in ATDD, I simply missed basic concepts of TDD:

1. Test comes first, afterwards refactor the code or write new code to pass the test.
2. When a change is hard to do, refactor first to enable the change.

I didn't evolve the new functionality from the existing one, but did a rewrite in fact (and reused parts of the existing code).

How you do better

At first, extract the concept of a Campaign from the backing bean (Extract Class Refactoring); reference just one Campaign from the backing bean. By forwarding the method calls to the backing bean to the new Campaign object, all of the tests stay green while moving attributes and methods. When all of the functionality is moved bit by bit to the new Campaign class, you can inline the delegating methods in the backing bean (Inline Method Refactoring). So these methods disappear and the access to the newly created Campaign field will become explicit in the tests. Finally move the existing tests concerning the functionality of Campaign from the existing BackingBeanTest to a new CampaignTest. All the tests stay green all the time. And that gives you

• little risk
• more confidence
• and a whole lot of fun. Commit.

While browsing the code you might spot some ugly parts. But don't touch them while changing another part of the system. Simply write it down on your todo-list. When your first change is done, you can turn toward the next item. When you want to rename some of your classes for the sake of more expressiveness, you can search for the corresponding name in your build path. Then you rename the class, all references to that class, the corresponding test class, and the variables referencing an object of the class, consistently (Rename Class Refactoring). Commit.
One method to clean up a class violating the Single Responsibility Principle is creating subclasses addressing the different responsibilities. You split up the existing test class into two separate test classes. You can then pick the necessary test cases / methods from the former test class and assign them to the new test classes by their relevance to the former mixed up responsibilities (similar to the Replace Type Code With Subclasses Refactoring, see also the "Anti-If" school). You can pull up common custom assertions into the superclass. After reorganizing the test cases you can split the production code into two subclasses, move the method and fields specific to just one subclass in the corresponding file and keep common functionality in the superclass. This process takes a little more time but it's a task of reasonable size. Commit.
Ah, and finally you can introduce the list of Campaigns easily and determine the selected one in the backing bean. Change done. Acceptance test passes. Mission accomplished.

What I learned from my mistake

• Refactoring and evolving a new functionality are fundamentally different. The sequence of the steps is nearly inverse.
• Refactoring demands more discipline.
• And the top priority is the following: don't ever break your tests!