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:

1. Create a persister class to contain the common save method
2. Abstract the metadata elements needed for saving (i.e. the columns and table name) into an interface
3. Implement the interface in the UserAccess class
4. From the UserInterface save method, construct a persister with the metadata
5. Call the save method 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.

Today's code