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
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 QAfterwards 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.- 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. - 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. anOrder
or anOrderLine
). 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:- Test comes first, afterwards refactor the code or write new code to pass the test.
- 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 aCampaign
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.
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
Campaign
s 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!
Keine Kommentare:
Kommentar veröffentlichen