OK, I lied. I’m not tackling building an abstraction for the metadata yet. I’d rather hit duplication first and see what that leads me to.
Concerned about how long I might take to get to a green bar, I put the timer on.
Goal: factor the duplicate save methods into a common representation.
The steps in mind:
-
Create a persister class to contain the common
savemethod -
Abstract the metadata elements needed for saving (i.e. the columns and table name) into an interface
-
Implement the interface in the UserAccess class
-
From the UserInterface
savemethod, construct a persister with the metadata -
Call the
savemethod on the persister, passing the object to be persisted
UserAccess
package domain;
import java.util.*;
import persistence.*;
public class UserAccess implements PersistableMetadata {
private static final String TABLE = "userdata";
private static String[] COLUMNS = { User.NAME, User.PASSWORD };
public void save(Persistable persistable) {
new Persister(this).save(persistable);
}
public User find(String nameKey) {
...
}
public String getTable() {
return TABLE;
}
public String[] getColumns() {
return COLUMNS;
}
}
PersistableMetadata
package domain;
public interface PersistableMetadata {
String getTable();
String[] getColumns();
}
Persister
package domain;
import persistence.*;
public class Persister {
private PersistableMetadata metadata;
public Persister(PersistableMetadata metadata) {
this.metadata = metadata;
}
public void save(Persistable persistable) {
String[] columns = metadata.getColumns();
String[] fields = new String[columns.length];
for (int i = 0; i < columns.length; i++)
fields[i] = persistable.get(columns[i]);
String sql = new SqlGenerator().createInsert(metadata.getTable(), columns, fields);
new JdbcAccess().execute(sql);
}
}
Pretty simple. Checking the timer, I'm at a little over 4 minutes, and I have a green bar.
Even though my tests pass, I try to never extract a new class without adding some level of test to it. If I can't get a reasonable test against the new class, then it suggests something's awry with my refactoring. Timer on.
package domain;
import persistence.*;
import junit.framework.*;
public class PersisterTest extends TestCase {
private static final String TABLE = "x";
private static final String[] COLUMNS = { "a", "b" };
private String lastSql;
public void testSave() {
PersistableMetadata metadata = new PersistableMetadata() {
public String getTable() {
return TABLE;
}
public String[] getColumns() {
return COLUMNS;
}
};
Persistable persistable = new Persistable() {
public String get(String key) {
if (key.equals(COLUMNS[0]))
return "0";
if (key.equals(COLUMNS[1]))
return "1";
return null;
}
};
JdbcAccess access = new JdbcAccess() {
public void execute(String sql) {
lastSql = sql;
}
};
Persister persister = new Persister(metadata, access);
persister.save(persistable);
assertEquals("insert into x (a,b) values ('0','1')", lastSql);
}
}
Gak. Three anonymous inner class overrides. Who has a problem with that? I've come across many developers who just hate anonymous inner classes. Me, I tend to use them all the time when I need to stub something for purposes of a test.
Here, the metadata and persistable dynamic class definitions do
something slightly different: they define an invariant class that I can
guarantee for purposes of the test. They are no different than any other
class I might define. The JdbcAccess override, however, dummies out the
execute method, so that I can (a) avoid having to do actual, slow
persistence when running the test and (b) prove that Persister is
working by testing against the SQL string that would otherwise be
executed.
Using the JdbcAccess override does require changes to the Persister class itself. I had to add an additional constructor, and then do something similar to Parameterize Constructor (see Michael Feathers' book Working Effectively With Legacy Code).
package domain;
import persistence.*;
public class Persister {
private PersistableMetadata metadata;
private JdbcAccess access;
// general use production constructor
public Persister(PersistableMetadata metadata) {
this(metadata, new JdbcAccess());
}
// new overloaded constructor
public Persister(PersistableMetadata metadata, JdbcAccess access) {
this.metadata = metadata;
this.access = access;
}
public void save(Persistable persistable) {
String[] columns = metadata.getColumns();
String[] fields = new String[columns.length];
for (int i = 0; i < columns.length; i++)
fields[i] = persistable.get(columns[i]);
String sql = new SqlGenerator().createInsert(metadata.getTable(), columns, fields);
access.execute(sql); // no longer directly instantiating here
}
}
Timer check: about 5 minutes (typing fast). Final step for today: refactor CustomerAccess the same way. Timer check: about 45 seconds.
Next: there's now some obvious duplication in CustomerAccess and UserAccess.
