We all know code needs to be refactored. Duplication sometimes need to be factored out. Sometimes we think of a more clear name for a method or variable and wish to rename something. Sometimes we accidentally use names that are ambiguous together and need to rename something, etc…
Using the data mapper pattern it is much easier to “make and go” with your refactorings. I am talking about defect localization. In TDD we use small isolated unit tests to drive the creation of production code (test first development). A common property of a good test suite is generally considered to be defect localization. If you are adding a feature to one part of the system, and that breaks something in another part of the system, the tests should tell you what broke, and should tell you specifically. Instead of pointing you to the general component it should point you to exactly which method the defect is introduced. This way you can respond faster and remain “agile” (as much of a buzz word that is).
Here’s how the data mapper comes into play. We have this object called the Employer:
/** @param Employer_Size */
public function setSize( $size )
{
$this->size = $size;
}
/** @return Employer_Size */
public function getSize()
{
return $this->size;
}
I want to simply rename that method to EnrollmentSize, because in this context we are actually referring to the approximate number of enrolled students at an employer’s school.
The first step is to have tests that cover this method in all layers:
function testSize()
{
$size = new Employer_Size;
$this->employer->setSize( $size );
$this->assertSame( $size, $this->employer->getSize() );
}
Test the objects.
function testSize()
{
$this->assertSameModel( $this->expected->getEnrollmentSize(), $this->actual->getEnrollmentSize() );
}
Test the data mapper for the Employer & Employer_Size classes.
Once we have this part under test, we need not worry about the fact the column is named ‘size_id’ in the database when we go to rename things in the code. The data mapper’s unit test saves & reloads the object, and each test method verifies that each mapping did its saving & loading.
So the first thing I did was search my code base for occurrences of “getSize”. There are a few but they are all luckily the ones I am intending to rename. So I go ahead and do a find & replace in my code, for both getSize and setSize. They have been renamed to getEnrollmentSize and setEnrollmentSize
I run my tests. I get failures, which I expected. The tests are telling me that the Employer_Mapper cannot find that old method Employer->getSize. That is what we expected, but if for some reason we thought we were done the tests let us know we still have work left to do.
In the data mapper I make a small change. I simply pass an option telling it that we have renamed the method and to use the new method name instead.
Before:
->addSingle( 'enrollment_size', 'Employer_Size' );
After:
->addSingle( 'enrollment_size', 'Employer_Size', array( 'property' => 'EnrollmentSize' ) );
Then I run the tests again. We passed (for the Mappers tests). We still have a failure in the EmployerTest. It tells me that this other method ->hasSize() was referencing the old ->getSize() method. We better update that reference to ->getSize(). We should also rename ->hasSize() to ->hasEnrollmentSize() to keep our code consistent with itself.
Before:
/** @return bool */
public function hasSize()
{
if( $this->getSize() instanceof Shuffler_Null )
{
return false;
}
return true;
}
After:
/** @return bool */
public function hasEnrollmentSize()
{
if( $this->getEnrollmentSize() instanceof Shuffler_Null )
{
return false;
}
return true;
}
I also rename its usages in my tests.
We still have not renamed the size_id in the employer table. We still have not renamed the Employer_Size class to Employer_EnrollmentSize. In TDD we work in these small, isolated steps. Because I run my tests before and after renaming the method, I prove to myself that the method can be renamed.
I run all my tests. They all pass. So now we can confidently tell our users that we *safely* made a refactoring. Our tests prove that if we push this update it will not cause any problems for our users.
I know I am not breaking any of the other many classes that use that method. That is how bugs are introduced. If we had just gone thru and started renaming everything, later on you will have a “bunch of places that needs to be updated”.
The way TDD differs from traditional development is that you do not let the code become “out of date with itself” for more than about 5 minutes maximum.
We could save renaming the database column until the next “major update”. That would in fact be a wise project management strategy. Batching your database updates allows your users to make wise decisions like scheduling the updates for non-peak hours, but in the mean time you can make minor tweaks to the code like renaming stuff.
Or depending on your project you could go ahead and rename the size_id column now. The only import thing is that we were able to make a small isolated change, and run the tests before & after. Then we can safely commit our code without breaking anything.
In programming, if you just go thru and make these types of changes, even if you are only making small changes to the code “for a few minutes”, it is funny how you can easily end up with hours of work going around to “update all the places it was used”. TDD is like risk management. If you force yourself to work in smaller change sets, the worst tragedy that could happen is reverting 5 minutes of work and taking a break.
Without the tests you could easily be trying to debug that code for hours to see which parts of your changes were related to the ‘bug’. If you work in small isolated change sets, reverting 5 minutes of code is no big deal. If you went around and made a bunch of changes without running your tests, you might be inclined to put in so much work that you become emotionally attached to the new code, and will spend hours debugging it since you are reluctant to revert it.
So the moral of this post is work in small change sets, and don’t be afraid to revert that sucker.
