|YaK:: WebLog #535 Topic : 2007-01-10 21.57.15 matt : refactoring with courage||[Changes] [Calendar] [Search] [Index] [PhotoTags]|
|[Back to weblog: pretention]|
Test Driven Development , a practice of writing a failing unit test first before implementing functionality.
Armed with these tools and getting the hang of them is very empowering, and provides a lot of courage to expend. My pair, coming from a team with heaps of legacy C++ code with no unit tests, is a little more timid than I am about refactoring. ( Luis was similar when we first started coding together.) This is very common until you get used to having the safety nets mentioned above, but it's also easy to become gunshy after working on a codebase where you're afraid to change *anything* -- even for a feature, nevermind a refactoring.
When I see a code smell , I take baby steps to get the code green so we can then safely attack the jugular and make it clean. Some people think code smells and refactoring are these amorphous, highly subjective things that don't have a real beginning or end. While many undisciplined coders have hijacked the word refactoring for their own self-serving exercises in code eugenics, most other people who use these practices don't work this way. Code smells are documented in the Refactoring books, as are the refactorings themselves. Generally, there are several small refactorings in one complete transformation. Extract a method, introduce a parameter, extract a base class, pull up a field to the base class, make a test subclass, override a method, extract an interface, etc. At each one of these small steps, my tests are green and I can commit -- this also means I can stop at the completion of any one of these steps if I need to. There is a definite beginning and a definite end, and I don't get stuck in a k-hole .
Trying to do all the refactorings at once and getting utterly lost leads to two things: reverting to get back to sanity and trying again (or giving up), banging one's head against the brick wall trying to get everything compiling again, or banging one's head some more to reconcile the copious new test failures because one of the changes somewhere didn't preserve behaviour and you made so many changes you don't know/remember which one could've caused the problem. (Many times, people will actually delete the tests instead of making them pass. If you actually do this, don't admit it in front of me -- I'm likely not to speak to you again. Just kidding ;>).
So, my pair was a little scared. We were looking at a single class that had grown organically and have several responsibilities. It was pulling things directly out of the UI (via the DOM), transforming them into JSON, and sending an XmlHttpRequest (XHR). It set a callback on the XHR so that when the response came back, a method in the class would update the UI directly (via the DOM). Obviously, this class is doing too much, but where to start?
Now what, do we commit? If we commit this, are we expressing our intention and direction well enough? If not, what's the simplest thing we can do to express our intention and then commit? In this case, we felt our intention would be expressed decently if we named our new class appropriately. We were extracting UI-specific elements and encapsulating those details in this new class, which seemed to imply it was a View a la the Model-View-Controller (MVC) pattern. We renamed the class and checked it in. A habit I did personally that we also do is to say what the next step is in the commit message. In this case, the next step was to move another UI-specific method into our new View class.
Which method did we choose next? The one that looked like it would be the simplest. We were wrong in the next choice, it ended up being too complicated, so we reverted and tried again with successful results. Reverting sounds like a waste of time, but because we committed in a small increment, it didn't take us that far back. The upside of the mistake is we learned about some dependencies between the methods we were moving that influenced our next refactoring.
Once we had a couple of methods in this new View class, we noticed the duplication of hard-coded DOM element references. To fix this, we made a field that stored that element, and then added a parameter to the constructor for the new View class. We then added the DOM-element parameter to where the View was being constructed -- in the original class. Wait, you say, you're putting the DOM-specific stuff back into the class you were trying to pull it out from anyways! You're correct, and it's a bad smell. So, we need to initialize the View class somewhere else and pass it into the constructor of the other class. We found where the other class was being constructed, in the global onLoad() function, and created our View there. We ran our tests, which passed, then we committed again.
We were now approaching something that is a pretty good design. It all started from identifying that the original class was violating the Single Responsibility Principle (SRP), and starting to alleviate some of its burden by extracting a method relating to one of its responsibilities into a new class. It just took the courage to make the first step, and then everything that came after was a matter of following our noses. With frequent commits, TDD, and decent tools, it made the courage easier to come by. Even without those tools, and in a different language/platform/environment, it just takes a little courage to get started and to keep moving at a steady pace if you have the humility to keep your steps small.
Discussion:showing all 0 messages
|(last modified 2007-01-10) [Login]|