As I mentioned yesterday, I want to get rid of the notion of having separate arrays for a column’s name and its type. Today I underwent another dramatic refactoring, one that ended up taking about 30 minutes. I approached the refactoring very incrementally. The measure was always to ensure a green bar every 5 minutes, no more than 10. I ended up averaging a green bar about every two minutes.
Part of the trick to this incremental refactoring is to build up parallel implementations. Overloading methods and adding new methods are your first tactic. Once changed functionality is in place, you can go back and remove redundant implementations. After that’s done, you might find some automated renaming refactorings are in order (because the best names might have already been taken!).
The general thought today is that metadata should be represented as a collection of Column objects:
private static Column[] COLUMNS = new Column[] {
new StringColumn(Customer.ID),
new StringColumn(Customer.NAME),
new IntegerColumn(Customer.BALANCE)
};
To support this, the definition of PersistableMetadata would need to change:
public interface PersistableMetadata {
String getTable();
String getKeyColumn();
T create(List row);
Column[] getColumns();
}
But I really didn’t take the big step of deleting the older methods
right away (getColumns which returned a String array, and
getTypeswhich returned an array of PersistableType). That happened
only after everything was working with a new definition for
getColumns, which I had temporarily named getAllColumns.
From there, the next goal was to drive changes into the Persister class.
I updated PersisterTest and incrementally got both its save and
findmethods working, without touching SqlGenerator. In order to do
this, it meant writing translation code. For example, the save method
calls SqlGenerator’s createInsert method. This method was still
expecting a column names array and a types array. The translation code
in Persister iterated the new Column[] and built both of these arrays.
After the translation code proved to work, I moved into SqlGenerator and incrementally refactored it to support taking an array of Column. Then it was a matter of working backward and refitting the call to SqlGenerator from Persister. Once that was done, I was able to tackle the fun part of deleting now-dead code.
The dead code included PersistableType. In the process of refactoring, the fact that each column needed to have a name and a type made me feel that an enum to represent type wasn’t necessarily the best idea. Why not just a simple polymorphic hierarchy instead?
ColumnTest
package persistence;
import junit.framework.*;
abstract public class ColumnTest extends TestCase {
protected static final String NAME = "name";
protected Column column;
protected void setUp() {
column = createColumn();
}
abstract protected Column createColumn();
public void testCreate() {
assertEquals(NAME, column.getName());
}
}
Column
package persistence;
public interface Column {
String getName();
String sqlValue(Object object);
}
AbstractColumn
package persistence;
abstract public class AbstractColumn {
private String name;
public AbstractColumn(String name) {
this.name = name;
}
public String getName() {
return name;
}
}
IntegerColumnTest
package persistence;
public class IntegerColumnTest extends ColumnTest {
public void testSqlValue() {
assertEquals("0", column.sqlValue(0));
}
public void testNullValue() {
assertNull(column.sqlValue(null));
}
@Override
protected Column createColumn() {
return new IntegerColumn(ColumnTest.NAME);
}
}
IntegerColumn
package persistence;
public class IntegerColumn extends AbstractColumn implements Column {
public IntegerColumn(String name) {
super(name);
}
public String sqlValue(Object value) {
if (value == null)
return null;
return value.toString();
}
}
StringColumnTest
package persistence;
public class StringColumnTest extends ColumnTest {
public void testSqlValue() {
assertEquals("'a'", column.sqlValue("a"));
}
public void testNullValue() {
assertNull(column.sqlValue(null));
}
@Override
protected Column createColumn() {
return new StringColumn(ColumnTest.NAME);
}
}
StringColumn
package persistence;
import util.*;
public class StringColumn extends AbstractColumn implements Column {
public StringColumn(String name) {
super(name);
}
public String sqlValue(Object input) {
if (input == null)
return null;
return StringUtil.wrap(input.toString(), '\'');
}
}
Note that a bunch of code from the PersistableType enum just moved across. I didn’t throw away the code, I recycled it. Also note use of the abstract test pattern, which helps ensure all superclass characteristics are consistently tested across all implementation subclasses.
So: a thirty minute exercise just to introduce a new type that I probably should have thought of at the outset? I can certainly justify the thirty minutes. In absence of having unit tests, I’ve done exercises like this in the past in one fell swoop. Mass change and replace, hack, whack, and 20 minutes later I have what I think is the problem. Only trouble is, I go to (manually) test it, and discover I did something stupid, or forgot something. In the process of even this incremental refactoring, I missed a couple of simple things; my tests told me immediately.
Old world result: 20 more minutes later, I might have come up with the proper solution, but I’d no longer bet on it. Nor would I bet on it being the correct solution.
As far as knowing ahead of time to use a column object? I might have. I also might have chosen the wrong level of abstraction. Doubtful here, but that’s not the real point.
In every system I’ve ever worked there have been numerous poor choices when it comes to design. I’ve also worked on systems where the initial design was pretty darn good. The team came up with a clean design that accommodated all of the requirements. Yet six months later, or two years later, someone came up with a new requirement that couldn’t be met easily with the existing design.
If you learn how to build systems using TDD and incremental redesign, you may find that you still end up down some of the same ratholes. You will spend more time getting to the better design than if you could have dreamed up that better design in the first place. But you’ll have the advantage in at least two ways: you’ll know a safe and sound approach for how to get from bad point A to good point B. You also will have spared the up-front effort that was wrong at least half of the time.
