Database TDD Part 28: Generating Database Tables

Somewhere between last week’s blog entry and this one, I decided it was long past time for my annual machine rebuild. It doesn’t take long for a Windows machine to get overloaded with cruft. Some of it I put there on purpose, some of it I didn’t. Attempts at routine maintenance ultimately can’t hold up to all the things that want to invade your system.

So I put everything, or so I thought, in one spot to be backed up. Actually I’ve gotten my machine to a point where almost everything critical is in one place: under my profile directory. My Eclipse workspace, my documents, my FitNesse installation (I think I need to figure out how to install the product elsewhere and have it point to the profile directory for its documents), my email, my CVS repository, and just about anything else I want to have each time I rebuild a machine. That includes a list of software to download and install, not the software itself (unless I’m married to a specific version). It amounts to only about half a gig, enough to back up on a CD.

There’s always something I forget to centralize or back up. This time I forgot to back up my database. There’s no valuable data in it currently; about the only thing I wanted to keep was the schema for this blog series. Oh well.

After spending a bunch of time rebuilding the machine (there are so many pieces of software I don’t remember having, but I did, and they need to be there), I’m now ready to get back to the blog series. I remembered my mistake when I reopened the Eclipse project. Many of the tests don’t run, since there is no database! I was going to tackle something different in this blog installment, but I guess now is as good as any time to deal with the relationship between the application and the database.

If you’re fortunate enough to have control of the database for your application needs, there’s little reason to design it any differently than the application’s object structure (performance might be one reason). You might be in a less fortunate situation, where you have a high-falutin’ DBA dictating what you’re going to have to work with.

If you’re not stuck with someone else’s mandate, you can treat the database schema and persistable class definitions as different views on the same entity. One can generate the other, and vice versa. Since I no longer have a schema, I’m going to have the application generate it.

I scraped together the following test:

package schema;

import junit.framework.*;

public class TableGeneratorTest extends TestCase {
   public void testSimple() {
      TableGenerator<Simple> generator = new TableGenerator<Simple>();
      generator.create(new SimpleAccess());

      SimpleAccess access = new SimpleAccess();
      final String name = "abc";
      Simple simple = new Simple(name);
      access.save(simple);
      Simple retrieved = access.find(name);
      assertEquals(name, retrieved.getName());
   }
}

This test required me to build SimpleAccess and Simple:

Simple

package schema;

import persistence.*;

class Simple implements Persistable {
   public static final String NAME = "name";
   private String name;

   Simple(String name) {
      this.name = name;
   }

   public Object get(String key) {
      return name;
   }

   public String getName() {
      return name;
   }
}

SimpleAccess

package schema;

import java.util.*;

import persistence.*;
import persistence.types.*;

class SimpleAccess extends DataAccess<Simple> {
   private static Column[] COLUMNS = new Column[] {
      new StringColumn(Simple.NAME)
   };

   public String getTable() {
      return "testsimple";
   }

   public String getKeyColumn() {
      return Simple.NAME;
   }

   public Simple create(Map<String, Object> row) {
      return new Simple((String)row.get(Simple.NAME));
   }

   public Column[] getColumns() {
      return COLUMNS;
   }
}

Simple enough. (Sorry!)

Hmm. As soon as I started to code a solution, I realized that I wanted to break it down and code a test to prove I can build the correct SQL. Moving over to SqlTest, I wrote:

public void testCreateTable() {
   String statement = String.format(
         "create table t (a varchar(%d),b varchar(%<d))",
         StringColumn.DEFAULT_WIDTH);
   assertEquals(statement, sql.create());
}

I got it to pass with this code in Sql:

public String create() {
   Transformer columnDefinition = new Transformer() {
      public String transform(Object input) {
         Column column = (Column)input;
         String declaration = String.format("varchar(%d)", StringColumn.DEFAULT_WIDTH);
         return String.format("%s %s", column.getName(), declaration);
      }};
   String columnDefinitions = StringUtil.commaDelimit(columns, columnDefinition);
   return String.format("create table %s (%s)", table, columnDefinitions);
}

Which, of course, only supports fields of type String. I’ll fix that, but for now, it passes and meets the needs of TableGenerator, which only demonstrates generating tables with VARCHAR columns (I’ll fix that too).

For now, I’m ok with assuming all Strings are to be stored with a default width in the database. I’ll define DEFAULT_WIDTH in StringColumn as:

public static final int DEFAULT_WIDTH = 32;

With the Sql modification done, I coded TableGenerator.

package schema;

import persistence.*;
import sql.*;

public class TableGenerator<T extends Persistable> {
   public void create(DataAccess<T> access) {
      String sql = new Sql(access.getTable(), access.getColumns()).create();
      new JdbcAccess().execute(sql);
   }
}

Wow. I’m pretty thrilled about how easy it is to code new functionality. That’s a very simple class and method, maybe it belongs elsewhere. On the Persister class, perhaps? Maybe later; I need to get the rest of my unit tests working by building their tables.

One minor detail before I fix the problem with data types: I can only run the test once. I’ve got to ensure we delete the test table. Looking at JdbcAccessTest, its setUp and tearDown code creates and drops a table. I think it’s time to add those as capabilities to JdbcAccess itself. Further, I need to be able to try and drop a table without having it throw an exception if no such table exists.

JdbcAccessTest

public class JdbcAccessTest extends TestCase {
   ...
   private JdbcAccess access;

   protected void setUp() {
      access = new JdbcAccess();
      access.execute(String.format("create table %s (%s varchar(1))", TABLE,
            COLUMN_NAME));
   }

   protected void tearDown() {
      access.dropIfExists(TABLE);
   }

   public void testDrop() {
      access.drop(TABLE);
      try {
         access.drop(TABLE);
      }
      catch (JdbcException expected) {
         Exception e = (Exception)expected.getCause();
         assertTrue(e.getMessage(), e.getMessage().indexOf("Unknown table") != -1);
      }
   }

   public void testDropIfExists() {
      access.dropIfExists(TABLE);
      try {
         access.drop(TABLE);
      }
      catch (JdbcException expected) {
         Exception e = (Exception)expected.getCause();
         assertTrue(e.getMessage(), e.getMessage().indexOf("Unknown table") != -1);
      }
      access.dropIfExists(TABLE); // shouldn't throw exception
   }
   ...

I always post my blogs and then review them quickly to correct any formatting errors. In doing so, I notice that testDrop and testDropIfExistscontain the same try/catch code. I’ve already posted the code, but I’m going to make a refactoring change that will appear in the next drop.

JdbcAccess

public void drop(String table) {
   execute("drop table " + table);
}

public void dropIfExists(String table) {
   try {
      drop(table);
   }
   catch (JdbcException expected) {
      Exception e = (Exception)expected.getCause();
      if (e.getMessage().indexOf("Unknown table") == -1)
         throw expected;
   }
}

TableGeneratorTest now looks like this:

TableGeneratorTest

package schema;

import persistence.*;
import junit.framework.*;

public class TableGeneratorTest extends TestCase {
   private JdbcAccess jdbc;
   private SimpleAccess access;

   protected void setUp() {
      jdbc = new JdbcAccess();
      access = new SimpleAccess();
      jdbc.dropIfExists(access.getTable());
   }

   protected void tearDown() {
      jdbc.drop(access.getTable());
   }

   public void testSimple() {
      TableGenerator<Simple> generator = new TableGenerator<Simple>();
      generator.create(new SimpleAccess());

      final String name = "abc";
      Simple simple = new Simple(name);
      access.save(simple);
      Simple retrieved = access.find(name);
      assertEquals(name, retrieved.getName());
   }
}

Maybe I could put the TableGenerator code into JdbcAccess. No, not a good idea. That would create a dependency of JdbcAccess on the classes Persistable and DataAccess. Best to keep JdbcAccess as ignorant of anything else in the system as possible.

One final note: I moved the source folders into a single Eclipse source directory, src. I also included a lib directory with the MySQLConnector JAR file. If you download the code archive, your best bet might be to start afresh with a new project.

OK, now time to go back to SqlTest and get the create method to support IntegerColumn. Right now the tests all use the COLUMNS constant, currently defined as:

private static Column[] COLUMNS = { new StringColumn("a"),
      new StringColumn("b") };

I’ll change that to:

private static Column[] COLUMNS = { new StringColumn("a"),
      new IntegerColumn("b") };

…and see what breaks. Two test methods break. I change testInsert to:

public void testInsert() {
   final Object[] values = { "1", 2 };
   assertEquals("insert into t (a,b) values ('1',2)", sql.insert(values));
}

In testCriteria, I have to switch around the Equals and Like criteria clauses.

public void testCriteria() {
   int value = 1;
   String pattern = "p%";
   Criteria criteria = new And(new Equals(COLUMNS[1], value), new Like(
         COLUMNS[0], pattern));
   String sqlString = sql.select(criteria);
   assertEquals("select a,b from t where (b=1) and (a like 'p%')",
         sqlString);
}

Now I can fix testCreateTable (which I just now chose to rename to testCreate).

public void testCreate() {
   String statement = String.format(
         "create table t (a varchar(%d),b <b>integer</b>)",
         StringColumn.DEFAULT_WIDTH);
   assertEquals(statement, sql.create());
}

I can get that to quickly pass with this nasty bit of code:

public String create() {
   Transformer columnDefinition = new Transformer() {
      public String transform(Object input) {
         Column column = (Column)input;
         String declaration = null;
         if (column instanceof StringColumn)
            declaration = String.format("varchar(%d)", StringColumn.DEFAULT_WIDTH);
         else
            declaration = "integer";
         return String.format("%s %s", column.getName(), declaration);
      }};
   String columnDefinitions = StringUtil.commaDelimit(columns, columnDefinition);
   return String.format("create table %s (%s)", table, columnDefinitions);
}

Horrors. That defeats the point of the having the Column type hierarchy in the first place. I’ll move things over.

Here are all the changes to the Column type hierarchy to support deriving a declaration from a given Column object:

Column

package persistence.types;

public interface Column {
   String getName();
   String sqlValue(Object object);
   <b>String declaration();</b>
}

StringColumnTest

public void testDeclaration() {
   assertEquals(String.format("%s varchar(%s)", ColumnTest.NAME,
         StringColumn.DEFAULT_WIDTH), column.declaration());
}

StringColumn

public String declaration() {
  return String.format("%s varchar(%s)", super.getName(), DEFAULT_WIDTH);
}

IntegerColumnTest

public void testDeclaration() {
   assertEquals(String.format("%s integer", ColumnTest.NAME), column
         .declaration());
}

IntegerColumn

public String declaration() {
   return String.format("%s integer", super.getName());
}

Now I can fix the code in the create method.

public String create() {
   Transformer columnDefinition = new Transformer() {
      public String transform(Object input) {
         return ((Column)input).declaration();
      }};
   String columnDefinitions = StringUtil.commaDelimit(columns, columnDefinition);
   return String.format("create table %s (%s)", table, columnDefinitions);
}

OK, last step. I wrote this crummy little utility class to generate the tables I need:

package schema;

import domain.*;

public class TableCreator {
   public static void main(String[] args) {
      new TableGenerator<Customer>().create(new CustomerAccess());
      new TableGenerator<User>().create(new UserAccess());
   }
}

I’m not sure where to put this class or what to do with. I don’t think I want to run it all the time, i.e. as part of my JUnit runs. Or maybe I do. For now, I don’t care. I run TableCreator. I run my unit tests. All green! Time to get ready for the weekend.

Database TDD Part 27: Revisiting Dusty Code

It’s been a long, wonderful holiday season. Sure, I had a couple of jobs, but I also had a good time, seeing family and friends from out east, and doing all sorts of fun holiday things. Unfortunately it meant I slacked off a bit with this blog. I’m picking it up today and want to get it to a point where I can consider it “good enough” to go to production with. Soon.

I looked at the code I’d built so far. So alien, so written by someone else, or so it seems. I picked around through tests to help re-familiarize myself with the code. But one of the ways I prefer to use to help understand code, old or new, mine or someone else’s, is to do a bit of refactoring on it. Through careful refactoring, I get a good sense of how things interrelate, while at the same time improving it.

In the database layer, I took a look at the sql package. This is the one that contains the SqlGenerate class and its attendant Criteria implementation classes (LikeCriteria, AndCriteria, etc.).

Well, one thing I’ve been doing lately is a lot of Fitnesse. Fitnesse combines a wiki with a simple acceptance testing framework. You express tests in wiki pages. Test pages thus have names like TestPatronFinesOnOverdueBook, or TestTransferHolding. A wiki word must composed of at least two separate words.

Jeff B. cornered me in the airport in Dallas and suggested that my naming in the database layer was too prolix. Of course I was mildly defensive, but I also admitted that wiki wording had tainted my Java coding. I was naming way too many of my identifiers with at least two words! More often than not, this was unnecessary.

In the sql package, I took a few minutes and renamed a number of things. For example:

  • AndCriteria to: And
  • Criteria.sqlString to: Criteria.sql
  • SqlGenerator.createColumnList to: SqlGenerator.columnList
  • SqlGenerator.createValuesList to: SqlGenerator.valuesList

And so on. This cleaned up things a good deal. I did not rename all of the verbosely-named methods within SqlGenerator. For example, I retainedcreateSelect, since I didn’t want to have an ambiguous method named select. Is it a verb (action method) or noun (suggesting a query or retrieval)? Maybe selectStatement would have worked, but it’s equally wordy.

I then considered SqlGenerator. Right now, it’s virtually a utility class, since it has no attributes. All methods are data in, return value out. I may as well have made all the methods static. But maybe it should be an object, with attributes. Looking through the code, all SqlGenerator methods require a table name and a list of columns. Why not construct the object with those as arguments and save them in instance variables?

First I overloaded the constructor, so I now had two:

public class SqlGenerator {
   private String table;
   private Column[] columns;

   public SqlGenerator(String table, Column[] columns) {
      this.table = table;
      this.columns = columns;
   }

   public SqlGenerator() {}

This allowed me to incrementally make my changes and not break gobs of client code all at once. Methods that had looked like:

public String createSelect(String tableName, Column[] columns, Criteria criteria) {
   return String.format("%s where %s", createSelectAll(tableName, columns),
         criteria.sqlString());
}

were that much simpler to read:

public String createSelect(Criteria criteria) {
   return String.format("%s where %s", createSelectAll(), criteria.sql());
}

The tests got easier to read too.

I now had something close to “an object that represents SQL” than “an object that generates SQL.” Why not just call the class Sql? And at that point, I felt like it made more sense to have all the methods use a consistent noun scheme. So I changed, for example, createSelect to just select. There is an opportunity for modest confusion, but not for anyone who has more than zero familiarity with the class.

Best of all, I like the way these changes cleaned up the client production code. The primary client of the SQL classes is Persister. Here’s what it now looks like:

Persister

package persistence;

import java.util.*;

import persistence.types.*;
import sql.*;

public class Persister {
   private PersistableMetadata metadata;
   private JdbcAccess access;

   public Persister(PersistableMetadata metadata) {
      this(metadata, new JdbcAccess());
   }

   public Persister(PersistableMetadata metadata, JdbcAccess access) {
      this.metadata = metadata;
      this.access = access;
   }

   public void save(T persistable) {
      String sql = createSql().insert(
            extractValues(persistable, metadata.getColumns()));
      access.execute(sql);
   }

   private Sql createSql() {
      return new Sql(metadata.getTable(), metadata.getColumns());
   }

   private Object[] extractValues(T persistable, Column[] columns) {
      Object[] values = new Object[columns.length];
      for (int i = 0; i &lt; columns.length; i++)
         values[i] = persistable.get(columns[i].getName());
      return values;
   }

   public void saveAll(Collection collection) {
      String sql = createSql().preparedInsert();
      access.executeAll(sql, metadata.getColumns(),
            createInsertRows(collection));
   }

   private List&lt;Map&lt;String, Object&gt;&gt; createInsertRows(Collection collection) {
      List&lt;Map&lt;String, Object&gt;&gt; rows = new ArrayList&lt;Map&lt;String, Object&gt;&gt;();
      for (T persistable : collection)
         rows.add(createInsertRow(persistable));
      return rows;
   }

   private Map&lt;String, Object&gt; createInsertRow(T persistable) {
      Map&lt;String, Object&gt; row = new HashMap&lt;String, Object&gt;();
      for (Column column : metadata.getColumns()) {
         Object value = persistable.get(column.getName());
         row.put(column.getName(), value);
      }
      return row;
   }

   public T find(String key) {
      String sql = createSql().findByKey(metadata.getKeyColumn(), key);
      Map&lt;String, Object&gt; row = access.executeQueryExpectingOneRow(sql,
            metadata.getColumns());
      if (row == null)
         return null;
      return metadata.create(row);
   }

   public List getAll() {
      String sql = createSql().selectAll();
      return executeQuery(sql);
   }

   public List findMatches(Column column, String pattern) {
      String sql = createSql().select(column, pattern);
      return executeQuery(sql);
   }

   public List find(Criteria criteria) {
      return executeQuery(createSql().select(criteria));
   }

   private List executeQuery(String sql) {
      List&lt;Map&lt;String, Object&gt;&gt; rows = access.executeQuery(sql, metadata
            .getColumns());
      List results = new ArrayList(rows.size());
      for (Map&lt;String, Object&gt; row : rows)
         results.add(metadata.create(row));
      return results;
   }
}

Hmm, should Persister have an instance variable of type Sql?

Feedback as always welcome. Particularly, what do you want to see next?

Database TDD Part 26: Prepared Statements

Prepared statement support is essential in any RDBMS persistence layer. The issue is, how do I build this support safely? Even if it didn’t break the whole notion of encapsulating all this JDBC code, I wouldn’t want to entrust handing out PreparedStatement objects to clients.

As a general strategy, I can reverse the technique I use for retrieving information. Queries return rows in the form of a list of maps. Each map is a row, with column names and values being the key-value pairs in the map. The same strategy can work for inserting data using a prepared statement: clients will populate a list of map-rows. JDBC code can then manage the entire process from start to finish.

I’ll start in the simplest place, SQL generation.

SqlGeneratorTest

public void testPreparedInsert() {
   String sql = new SqlGenerator().createPreparedInsert(TABLE, COLUMNS);
   assertEquals("insert into t (a,b) values (?,?)", sql);
}

SqlGenerator

public String createPreparedInsert(String table, Column[] columns) {
   return String.format("insert into %s (%s) values (%s)", table,
         createColumnList(columns), createPlaceholderList(columns));
}

private Object createPlaceholderList(Column[] columns) {
   final Transformer questions = new Transformer() {
      public String transform(Object ignored) {
         return "?";
      }
   };
   return StringUtil.commaDelimit(columns, questions);
}

Nothing earth shattering there. These methods are getting easier to write as I add more functionality.

The new JDBC code:

JdbcAccessTest

public void testExecutePreparedStatement() {
  List<Map<String,Object>> rows = new ArrayList<Map<String,Object>>();
  Map<String,Object> row1 = new HashMap<String,Object>();
  row1.put(COLUMN_NAME, VALUE1);
  Map<String,Object> row2 = new HashMap<String,Object>();
  row2.put(COLUMN_NAME, VALUE2);

  rows.add(row1);
  rows.add(row2);

  String sql = String.format("insert into %s values(?)", TABLE);
  access.executeAll(sql, COLUMNS, rows);

  assertResults(access.executeQuery(createSelectSql(), COLUMNS));
}

private void assertResults(List<Map<String, Object>> rows) {
  assertEquals(2, rows.size());
  assertContains(rows, VALUE1);
  assertContains(rows, VALUE2);
}

JdbcAccess

private PreparedStatement preparedStatement;
...
public void executeAll(String sql, Column[] columns,
      List<Map<String, Object>> rows) {
   try {
      createPreparedStatement(sql);

      for (Map<String, Object> row : rows) {
         int i = 0;
         for (Column column : columns)
            preparedStatement.setObject(++i, row.get(column.getName()));
         preparedStatement.execute();
      }

      connection.close();
   }
   catch (SQLException e) {
      throw new JdbcException(sql, e);
   }
}

OK, so here’s the problem: I’m introducing a construct that is solely for performance needs. Yet I now force the client of JdbcAccess to populate a list of map-rows in order to use a PreparedStatement. Yuk. Granted, database performance concerns are far more significant than in-memory performance. But it seems unfortunate to add performance overhead at the same time I’m trying to address it. I’ll take a look at this in either the next or a future installment.

Now back to the bane of this suite, the stub/mock friendly PersisterTest. I finally got fed up with the difficulty of understanding tests in this class. I spent about 15 minutes incrementally refactoring it so that the mock JdbcAccess definitions appear within each test method (for the most part). This makes it clear which behavior of JdbcAccess is getting mocked and which is not; I think it makes the tests much easier to follow.

PersisterTest

package persistence;

import java.util.*;

import junit.framework.*;
import persistence.types.*;
import sql.*;

public class PersisterTest extends TestCase {
   private static final String TABLE = "x";

   private static final Persistable RETURN_OBJECT1 = new Persistable() {
      public Object get(String key) {
         return null;
      }
   };
   private static final Persistable RETURN_OBJECT2 = new Persistable() {
      public Object get(String key) {
         return null;
      }
   };

   private static final String BAD_KEY = "not found";
   private static final String COLUMN1 = "a";
   private static final String COLUMN2 = "b";
   private static final String ROW1_VALUE1 = "a1";
   private static final String ROW1_VALUE1_PATTERN = "a%";
   private static final String ROW1_VALUE2 = "a2";
   private static final String ROW2_VALUE1 = "b1";
   private static final String ROW2_VALUE2 = "b2";
   private static final Column[] COLUMNS = { new StringColumn(COLUMN1),
         new StringColumn(COLUMN2) };

   private String lastSql;
   private PersistableMetadata<Persistable> metadata;

   private Persister<Persistable> persister;
   private Persistable persistable;

   protected void setUp() {
      metadata = new PersistableMetadata<Persistable>() {
         public String getTable() {
            return TABLE;
         }

         public String getKeyColumn() {
            return COLUMNS[0].getName();
         }

         public Column[] getColumns() {
            return COLUMNS;
         }

         public Persistable create(Map<String, Object> row) {
            if (row.get(COLUMN1).equals(ROW1_VALUE1)
                  && row.get(COLUMN2).equals(ROW1_VALUE2))
               return RETURN_OBJECT1;
            if (row.get(COLUMN1).equals(ROW2_VALUE1)
                  && row.get(COLUMN2).equals(ROW2_VALUE2))
               return RETURN_OBJECT2;
            return null;
         }
      };

      persistable = new Persistable() {
         public Object get(String key) {
            if (key.equals(COLUMN1))
               return ROW1_VALUE1;
            if (key.equals(COLUMN2))
               return ROW1_VALUE2;
            return null;
         }
      };
   }

   public void testSave() {
      createPersister(new JdbcAccess() {
         public void execute(String sql) {
            lastSql = sql;
         }
      });

      persister.save(persistable);
      assertLastSql(String.format("insert into %s (%s,%s) values ('%s','%s')",
            TABLE, COLUMN1, COLUMN2, ROW1_VALUE1, ROW1_VALUE2));
   }

   public void testSaveAll() {
      createPersister(new JdbcAccess() {
         public void executeAll(String sql, Column[] columns,
               List<Map<String, Object>> rows) {
            lastSql = sql;
            Assert.assertEquals(COLUMNS, columns);
            Assert.assertEquals(1, rows.size());
            Map<String, Object> row = rows.get(0);
            Assert.assertEquals(ROW1_VALUE1, row.get(COLUMN1));
         }
      });

      Collection<Persistable> collection = new ArrayList<Persistable>();
      collection.add(persistable);
      persister.saveAll(collection);
      assertEquals(String.format("insert into %s (%s,%s) values (?,?)", TABLE,
            COLUMN1, COLUMN2), lastSql);
   }

   public void testFindBy() {
      createPersister(new JdbcAccess() {
         public Map<String, Object> executeQueryExpectingOneRow(String sql,
               Column[] columns) {
            Assert.assertEquals(COLUMNS, columns);
            lastSql = sql;
            return createRow1();
         }
      });

      final String key = ROW1_VALUE1;
      assertEquals(RETURN_OBJECT1, persister.find(key));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, key));
   }

   public void testFindNotFound() {
      createPersister(new JdbcAccess() {
         public Map<String, Object> executeQueryExpectingOneRow(String sql,
               Column[] columns) {
            lastSql = sql;
            return null;
         }
      });
      assertNull(persister.find(BAD_KEY));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, BAD_KEY));
   }

   public void testGetAll() {
      createMockedExecuteQueryPersister();
      assertQueryResults(persister.getAll());
      assertEquals(String.format("select %s,%s from %s", COLUMN1, COLUMN2,
            TABLE), lastSql);
   }

   public void testFindMatches() {
      createMockedExecuteQueryPersister();
      assertQueryResults(persister.findMatches(COLUMNS[0], ROW1_VALUE1_PATTERN));
      assertLastSql(String.format("select %s,%s from %s where %s like '%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, ROW1_VALUE1_PATTERN));
   }

   public void testFindWithCriteria() {
      createMockedExecuteQueryPersister();
      Criteria criteria = new EqualsCriteria(COLUMNS[0], ROW1_VALUE1);
      assertQueryResults(persister.find(criteria));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, ROW1_VALUE1));
   }

   private void createMockedExecuteQueryPersister() {
      createPersister(new JdbcAccess() {
         public List<Map<String, Object>> executeQuery(String sql,
               Column[] columns) {
            lastSql = sql;
            List<Map<String, Object>> results = new ArrayList<Map<String, Object>>();
            results.add(createRow1());
            results.add(createRow2());
            return results;
         }
      });
   }

   private void createPersister(JdbcAccess accessMock) {
      persister = new Persister<Persistable>(metadata, accessMock);
   }

   private void assertLastSql(String sql) {
      assertEquals(sql, lastSql);
   }

   private void assertQueryResults(List<Persistable> results) {
      assertEquals(2, results.size());
      assertTrue(results.contains(RETURN_OBJECT1));
      assertTrue(results.contains(RETURN_OBJECT2));
   }

   protected Map<String, Object> createRow1() {
      return createRow(ROW1_VALUE1, ROW1_VALUE2);
   }

   protected Map<String, Object> createRow2() {
      return createRow(ROW2_VALUE1, ROW2_VALUE2);
   }

   protected Map<String, Object> createRow(String value1, String value2) {
      Map<String, Object> row = new HashMap<String, Object>();
      row.put(COLUMN1, value1);
      row.put(COLUMN2, value2);
      return row;
   }
}

Persister

public void saveAll(Collection<T> collection) {
   String sql = new SqlGenerator().createPreparedInsert(metadata.getTable(),
         metadata.getColumns());
   access.executeAll(sql, metadata.getColumns(), createInsertRows(collection));
}

private List<Map<String, Object>> createInsertRows(Collection<T> collection) {
   List<Map<String, Object>> rows = new ArrayList<Map<String, Object>>();
   for (T persistable : collection)
      rows.add(createInsertRow(persistable));
   return rows;
}

private Map<String, Object> createInsertRow(T persistable) {
   Map<String, Object> row = new HashMap<String, Object>();
   for (Column column : metadata.getColumns()) {
      Object value = persistable.get(column.getName());
      row.put(column.getName(), value);
   }
   return row;
}

After creating the saveAll code in Persister, I did a little refactoring on the save method:

Persister

public void save(T persistable) {
   String sql = new SqlGenerator().createInsert(metadata.getTable(),
         metadata.getColumns(), extractValues(persistable, metadata
               .getColumns()));
   access.execute(sql);
}

private Object[] extractValues(T persistable, Column[] columns) {
   Object[] values = new Object[columns.length];
   for (int i = 0; i < columns.length; i++)
      values[i] = persistable.get(columns[i].getName());
   return values;
}

It looks like there are some good similarities between the two save methods that I want to try to reconcile in the near future.

To test all this code out I wrote the following (live) database test.

CustomerAccessTest

package domain;

import java.util.*;

import persistence.*;

import junit.framework.*;

public class CustomerAccessTest extends TestCase {
   private CustomerAccess access;

   protected void setUp() {
      access = new CustomerAccess();
      JdbcAccess jdbc = new JdbcAccess();
      jdbc.execute("truncate table " + access.getTable());
   }

   public void testPersist() {
      final String name = "a";
      final String id = "1";
      final int amount = 100;

      Customer customer = new Customer(id, name);
      customer.charge(amount);

      access.save(customer);
      Customer retrievedCustomer = access.find(id);
      assertEquals(id, retrievedCustomer.getId());
      assertEquals(name, retrievedCustomer.getName());
      assertEquals(amount, retrievedCustomer.getBalance());
   }

   public void testPersistLots() {
      final int count = 10;
      Collection<Customer> customers = new ArrayList<Customer>();
      for (int i = 0; i < count; i++) {
         String id = "" + i;
         Customer customer = new Customer(id, "a");
         customer.charge(i);
         customers.add(customer);
      }

      access.saveAll(customers);

      for (int i = 0; i < count; i++) {
         String id = "" + i;
         Customer retrievedCustomer = access.find(id);
         assertEquals(i, retrievedCustomer.getBalance());
      }
   }
}

In doing so I recognized that the customer table needed to get cleared out with each execution, so I added the setUp method. Here’s the implementation in the DataAccess superclass. (If you’re looking at older code, note that I recognized and corrected a deficiency with my declaration of the parameterized type.)

DataAccess

abstract public class DataAccess<T extends Persistable> implements
      PersistableMetadata<T> {
   ...
   public void saveAll(Collection<T> collection) {
      new Persister<T>(this).saveAll(collection);
   }
   ...
}

I note that I’ve just added another “live” persistence test to CustomerAccess. This will start to increase the amount of time to execute my complete suite of tests. Still, I’m at a very comfortable ~5 seconds. I think the next time I feel compelled to add such a live “confidence” test I’ll revisit what I want to do about this potential execution time bloat. Maybe it’s not a concern–I’m not writing these tests for every possible DataAccess subclass. I think there are a few missing tests that I might add, but I don’t know that they’ll severely increase test execution time.

I’m still acting non-traditionally, by the way, in working this backward. Inside-out, some might call it. This is partly because the need for PreparedStatement support is artificial (I was too lazy to dream up and work something down from the application level). It’s also because sometimes it’s the easiest way for me to approach solving the problem.

Database TDD Part 25: Modifying the Application Mock

Before launching into the latest changes, first I’d like say that puppies and sleep don’t mix.

The Persister class now supports a generalized find that takes a Criteria object as an argument. Backing things up, the code in DataAccess becomes:

DataAccess

public List findMatches(Criteria criteria) {
   return new Persister(this).find(criteria);
}

Hmm, I don’t recall writing a test for this code. That’s something I’ll have to go back and do. Remind me please. It’ll get tested in context, but I’m not willing to let that sit for long.

Code within Application becomes:

public List findMatchingCustomers(String namePattern) {
   Column name = customerAccess.getColumn(Customer.NAME);
   return customerAccess.findMatches(new LikeCriteria(name, namePattern));
}

There are two significant results of this change. As mentioned, I wanted to back responsibility for coming up with query criteria to the client code, i.e. the application itself in this case. That closes the persistence layer off to all new inevitable query criteria. But this decision results in the fact that the application layer must now import from the persistence package. The persistence package is one big honking set of code. Maybe it’s time for the package to be broken out. The obvious point of separation, based on this new change, is to pull Criteria and perhaps SQL generation classes out into their own package.

Also, note that the client is now responsible for extracting a Column object based on a name. This is because the criteria objects require complete column information in order to be able to construct the appropriate SQL. One alternative would be to pass the column name to the criteria object, and let the criteria object obtain the appropriate column, but I don’t like the dependency that introduces. Another alternative would be to resurrect the column information when the SQL is rendered, in the Persister class. For now I’ll stick with what I have, and fix it later if it looks to be unacceptable duplication in client code.

Now, the other problem with changing Application implementation details is that my mock no longer works! Ugh. Bad trap–this is why I suggested that you should keep mocks as simple as possible. The problem is that mocks must inherently mirror some level of implementation detail. I simplified my MockCustomerAccess class considerably:

MockCustomerAccess

package application;

import java.util.*;

import persistence.*;
import sql.*;
import domain.*;

class MockCustomerAccess extends CustomerAccess {
   private Persistable customer;

   public List findMatches(Criteria criteria) {
      List results = new ArrayList();
      if (criteria.sqlString().indexOf("'n%'") &gt;= 0)
         results.add((Customer)customer);
      return results;
   }

   public void save(Persistable persistable) {
      customer = persistable;
   }
}

It’s now pretty hard-coded to the test that uses it. That’s the way I want it. In fact, I’d just as soon the mock appear in the test itself, that way the two remain tightly bound:

MockCustomerAccess

public void testBrowseCustomers() {
   final String id1 = "id1";
   final String name1 = "name1";
   final String matchingCustomerPattern = "'n%'";

   application.setCustomerAccess(new CustomerAccess() {
      private Persistable customer;

      public List findMatches(Criteria criteria) {
         List results = new ArrayList();
         if (criteria.sqlString().indexOf(matchingCustomerPattern) &gt;= 0)
            results.add((Customer)customer);
         return results;
      }

      public void save(Persistable persistable) {
         customer = persistable;
      }
   });

   Customer customer1 = new Customer(id1, name1);
   application.add(customer1);
   assertTrue(application.findMatchingCustomers("'a%'").isEmpty());
   List customers = application
         .findMatchingCustomers(matchingCustomerPattern);
   assertEquals(1, customers.size());
   Customer retrievedCustomer = customers.get(0);
   assertEquals(id1, retrievedCustomer.getId());
   assertEquals(name1, retrievedCustomer.getName());
}

The test is now a bit long, but I’m ok with that. A second test related to customer access would no doubt expose duplication to be refactored.

The dogs kept me up all night, so I’m looking to wrap this up. Here’s the new package structure:

persistence:
   DataAccess
   JdbcAccess
   JdbcException
   Persistable
   Persister

persistence.types:
   AbstractColumn
   Column
   IntegerColumn
   StringColumn

sql:
   AndCriteria
   BinaryOperator
   Criteria
   EqualsCriteria
   LikeCriteria
   LogicalCriteria
   OrCriteria
   SqlGenerator

Enjoy. I’m pretty happy with the way this is shaping up so far. I’m sure there are things you can find to shake a stick at, too. That’s inevitable, and that’s also why this might be a far more interesting exercise for a pair. I figure a couple of solid programmers should be able to get to the point that I’ve gotten to here within a day, at most two days.

For future challenges in the blog series, I intend on taking a look at prepared statements, and then maybe some real ugliness such as join statements and subselects.

 

Original Comments:

I would almost certainly make the classes in the sql package And, Binary, Criteria, Like, Logical, Or, and Sql.

factoring duplication, a technique Fred showed me leaves the class names long, and uses a static import on the SqlGenerator to import and, or, like, criteria, etc. methods.

===
I forgot to sign a post, so -probably- the previous message is –JeffBay, unless its been haxx0red.

===
I’m not following the second sentence. Is that a good thing or not, it seems to contradict what you suggest in the first sentence.

I’ve used static import in tests for the class under test, and I think it’s great. I’ll take a look at the SqlGenerator class and see what that ends up looking like.

Right now SqlGenerator is just that, it’s all functional methods. It has no state, that’d be the only reason I’d resist renaming it to Sql. But I still might.

Database TDD Part 24: Refactoring After Breaks

Adding support for new where clause criteria is going to get very tedious very quickly. I want the client to be able to put this criteria together, but not by supplying SQL text strings. Instead, the client code should be able to supply a set of criteria objects in a parse tree form.

Starting with simple equality tests, I create an EqualsCriteria class:

EqualsCriteriaTest

package persistence;

import junit.framework.*;

public class EqualsCriteriaTest extends TestCase {
   private static final String FIELD_NAME = "field";

   public void testString() {
      Column column = new StringColumn(FIELD_NAME);
      Criteria criteria = new EqualsCriteria(column, "value");
      assertEquals("field='value'", criteria.sqlString());
   }

   public void testDifferentType() {
      Column column = new IntegerColumn(FIELD_NAME);
      Criteria criteria = new EqualsCriteria(column, 1);
      assertEquals("field=1", criteria.sqlString());
   }
}

I suppose I could get rid of the duplication in the assertions. Sometimes I think it’s easier to read the tests when the expected value is clearly spelled out in a complete, hardcoded string. But some other times, I think the J2SE 5.0 formatting capability allows for the assertion to be almost as succinct, while at the same time eliminating the duplication:

EqualsCriteriaTest, refactored

package persistence;

import junit.framework.*;

public class EqualsCriteriaTest extends TestCase {
   private static final String FIELD_NAME = "field";
   private Column column;

   public void testString() {
      column = new StringColumn(FIELD_NAME);
      final String value = "value";
      Criteria criteria = new EqualsCriteria(column, value);
      assertEquals(String.format("%s='%s'", FIELD_NAME, value), criteria.sqlString());
   }

   public void testDifferentType() {
      column = new IntegerColumn(FIELD_NAME);
      final int value = 1;
      Criteria criteria = new EqualsCriteria(column, value);
      assertEquals(String.format("%s=%d", FIELD_NAME, value), criteria.sqlString());
   }
}

The production class is pretty small and simple:

EqualsCriteria

package persistence;

public class EqualsCriteria implements Criteria {
   private Column column;
   private Object value;

   public EqualsCriteria(Column column, Object value) {
      this.column = column;
      this.value = value;
   }

   public String sqlString() {
      return String.format("%s=%s", column.getName(), column.sqlValue(value));
   }
}

The idea is that there will be multiple Criteria implementations, each with the ability to return an SQL string representation.

Criteria

package persistence;

public interface Criteria {
   String sqlString();
}

Putting together various other Criteria types is a quick, simple exercise.

LikeCriteriaTest

package persistence;

import junit.framework.*;

public class LikeCriteriaTest extends TestCase {
   private static final String FIELD_NAME = "field";

   public void testString() {
      Column column = new StringColumn(FIELD_NAME);
      final String pattern = "v%";
      Criteria criteria = new LikeCriteria(column, pattern);
      assertEquals(String.format("%s like '%s'", FIELD_NAME, pattern), criteria
            .sqlString());
   }

   // no other types are valid
}

LikeCriteria

package persistence;

public class LikeCriteria implements Criteria {
   private Column column;
   private String pattern;

   public LikeCriteria(Column column, String pattern) {
      this.column = column;
      this.pattern = pattern;
   }

   public String sqlString() {
      return String.format("%s like %s", column.getName(), column.sqlValue(pattern));
   }

}

There does seem to be some duplication creeping up between the various criteria types. But it’s not obvious to me yet how I’d eliminate it to any real benefit.

The fun part, and where this really starts paying off, is in adding support for logical operators. An “and” criteria introduces the recursive tree descent used to generate the entire where clause. It simply combines two other criteria, asking each for its SQL representation.

AndCriteriaTest

package persistence;

import junit.framework.*;

public class AndCriteriaTest extends TestCase {
   public void testString() {
      Criteria criteria1 = new LikeCriteria(new StringColumn("a"), "p%");
      Criteria criteria2 = new EqualsCriteria(new StringColumn("b"), "v");
      Criteria criteria = new AndCriteria(criteria1, criteria2);
      assertEquals(String.format("(%s) and (%s)", criteria1.sqlString(),
            criteria2.sqlString()), criteria.sqlString());
   }
}

AndCriteria

package persistence;

public class AndCriteria implements Criteria {
   private Criteria left;
   private Criteria right;

   public AndCriteria(Criteria left, Criteria right) {
      this.left = left;
      this.right = right;
   }

   public String sqlString() {
      return String
            .format("(%s) and (%s)", left.sqlString(), right.sqlString());
   }

}

Hmm, I just spotted some minor duplication with respect to the hardcoded SQL keywords I’ve been introducing (like, and). The code you download may have this fixed.

OrCriteriaTest

package persistence;

import junit.framework.*;

public class OrCriteriaTest extends TestCase {
   public void testString() {
      Criteria criteria1 = new LikeCriteria(new StringColumn("f1"), "v%");
      Criteria criteria2 = new EqualsCriteria(new StringColumn("f2"), 'x');
      Criteria criteria = new OrCriteria(criteria1, criteria2);
      assertEquals(String.format("(%s) or (%s)", criteria1.sqlString(),
            criteria2.sqlString()), criteria.sqlString());
   }
}

Looking at the implementation and test for OrCriteria, I do recognize a now significant amount of duplication.

OrCriteria

package persistence;

public class OrCriteria implements Criteria {
   private Criteria left;
   private Criteria right;

   public OrCriteria(Criteria left, Criteria right) {
      this.left = left;
      this.right = right;
   }

   public String sqlString() {
      return String
            .format("(%s) or (%s)", left.sqlString(), right.sqlString());
   }
}

Sometimes as I’m coding, I don’t really notice the duplication, or am too lazy to care about it (this is why I need a pair). I find that it’s a good idea to revisit the code a little later in my programming session, maybe when I come back after a 15-minute break. In fact, sometimes it’s hard to come up to speed after I’ve been distracted by something else, maybe a discussion of my fantasy team. A quick refactoring exercise after a break is a great way to refamiliarize myself with what I was just working on.

To make all these criteria classes useful, I’m going to need to modify how SQL gets generated.

SqlGeneratorTest

...
public void testCriteria() {
   String value = "v";
   String pattern = "p%";
   Criteria criteria = new AndCriteria(new EqualsCriteria(COLUMNS[0], value), 
      new LikeCriteria(COLUMNS[1], pattern));
   String sql = generator.createSelect(TABLE, COLUMNS, criteria);
   assertEquals("select a,b from t where (a='v') and (b like 'p%')", sql);
}
...

SqlGenerator

public String createSelect(String tableName, Column[] columns, Criteria criteria) {
   return String.format("%s where %s", createSelectAll(tableName, columns),
         criteria.sqlString());
}

It’s interesting how my methods seem to be getting shorter and shorter. This is a direct result of eliminating duplication all along: it tends to create more abstract, reusable methods, which allows me to more readily take advantage of them.

Backing out to the application, here’s some more code that readies the client to be able to use the Criteria classes.

PersisterTest

public void testFindWithCriteria() {
   Criteria criteria = new EqualsCriteria(COLUMNS[0], ROW1_VALUE1);
   assertQueryResults(persister.find(criteria));
   assertLastSql(String.format("select %s,%s from %s where %s='%s'",
         COLUMN1, COLUMN2, TABLE, COLUMN1, ROW1_VALUE1));
}

Persister

public List find(Criteria criteria) {
   return executeQuery(new SqlGenerator().createSelect(metadata.getTable(),
         metadata.getColumns(), criteria));
}

When I finally got to the existing, sample Application code, I found some new challenges that I’ll discuss tomorrow.

With respect to this blog, I wrote the code earlier in the day (about 30 minutes worth of coding), then came back and wrote the text around it. And as you can see, I wasn’t very happy with the code when I tried to describe it. Rather than rework the code, I figured maybe it was educational to show the first cut, and then, following, the refactored results.

CriteriaTestConstants

package persistence;

public class CriteriaTestConstants {
   static final String FIELD_NAME = "field";
   static final String STRING_VALUE = "value";
   static Column STRING_COLUMN = new StringColumn(FIELD_NAME);

   static final int INT_VALUE = 1;
   static Column INT_COLUMN = new IntegerColumn(FIELD_NAME);

   static final String PATTERN = "v%";
}

I don’t really care for “constants” classes, but for isolated test purposes I’m ok with it.

EqualsCriteriaTest

package persistence;

import junit.framework.*;
import static persistence.CriteriaTestConstants.*;

public class EqualsCriteriaTest extends TestCase {
   static final Criteria STRING_CRITERIA = new EqualsCriteria(STRING_COLUMN,
         STRING_VALUE);
   static final Criteria INT_CRITERIA = new EqualsCriteria(INT_COLUMN,
         INT_VALUE);

   public void testString() {
      assertEquals(String.format("%s='%s'", FIELD_NAME, STRING_VALUE),
            STRING_CRITERIA.sqlString());
   }

   public void testDifferentType() {
      assertEquals(String.format("%s=%d", FIELD_NAME, INT_VALUE), INT_CRITERIA
            .sqlString());
   }
}

EqualsCriteria

package persistence;

public class EqualsCriteria extends BinaryOperator implements Criteria {
   public EqualsCriteria(Column column, Object value) {
      super(column, value);
   }

   public String sqlString() {
      return super.binaryClause("=");
   }
}

This common superclass might end up with a better name later:

BinaryOperator

package persistence;

public class BinaryOperator {
   private Column column;
   private Object value;

   public BinaryOperator(Column column, Object value) {
      this.column = column;
      this.value = value;
   }

   public String binaryClause(String operator) {
      return String.format("%s%s%s", column.getName(), operator, column
            .sqlValue(value));
   }
}

LikeCriteriaTest

package persistence;

import junit.framework.*;
import static persistence.CriteriaTestConstants.*;

public class LikeCriteriaTest extends TestCase {
   static final Criteria CRITERIA = new LikeCriteria(STRING_COLUMN, PATTERN);

   public void testString() {
      assertEquals(String.format("%s%s'%s'", FIELD_NAME, LikeCriteria.LIKE,
            PATTERN), CRITERIA.sqlString());
   }

   // no other types are valid
}

LikeCriteria

package persistence;

public class LikeCriteria extends BinaryOperator implements Criteria {
   static final String LIKE = " like ";

   public LikeCriteria(Column column, String pattern) {
      super(column, pattern);
   }

   public String sqlString() {
      return super.binaryClause(LIKE);
   }
}

AndCriteriaTest

package persistence;

import junit.framework.*;

public class AndCriteriaTest extends TestCase {
   public void testString() {
      Criteria criteria1 = LikeCriteriaTest.CRITERIA;
      Criteria criteria2 = EqualsCriteriaTest.STRING_CRITERIA;
      Criteria criteria = new AndCriteria(criteria1, criteria2);
      assertEquals(String.format("(%s) %s (%s)", criteria1.sqlString(), AndCriteria.AND,
            criteria2.sqlString()), criteria.sqlString());
   }
}

AndCriteria

package persistence;

public class AndCriteria extends LogicalCriteria implements Criteria {
   public static final String AND = "and";

   public AndCriteria(Criteria left, Criteria right) {
      super(left, right);
   }

   public String sqlString() {
      return binarySqlString(AND);
   }
}

LogicalCriteria

package persistence;

public class LogicalCriteria {
   protected Criteria left;
   protected Criteria right;

   public LogicalCriteria(Criteria left, Criteria right) {
      this.left = left;
      this.right = right;
   }

   protected String binarySqlString(String operator) {
      return String.format("(%s) %s (%s)", left.sqlString(), operator, right
            .sqlString());
   }
}

OrCriteriaTest

package persistence;

import junit.framework.*;

public class OrCriteriaTest extends TestCase {
   public void testString() {
      Criteria criteria1 = EqualsCriteriaTest.STRING_CRITERIA;
      Criteria criteria2 = LikeCriteriaTest.CRITERIA;
      Criteria criteria = new OrCriteria(criteria1, criteria2);
      assertEquals(String.format("(%s) %s (%s)", criteria1.sqlString(), OrCriteria.OR,
            criteria2.sqlString()), criteria.sqlString());
   }
}

OrCriteria

package persistence;

public class OrCriteria extends LogicalCriteria implements Criteria {
   public static final String OR = "or";

   public OrCriteria(Criteria left, Criteria right) {
      super(left, right);
   }

   public String sqlString() {
      return binarySqlString(OR);
   }
}

There are still some areas of duplication. For now, further refactoring would seem to hold diminishing returns.

Database TDD Part 23: Select Criteria

The pseudo-application I’m building is going to have varied query needs, involving multiple conditionals, matching strings, relational operators, and so on. I want the ability to be more flexible with my where clauses. So far, the existing database layer supports only finding a row by key. I’m going to add the ability to support a second type of criteria, a matching operation against a single column.

Driving from the application down:

ApplicationTest

public void testBrowseCustomers() {
   final String id1 = "id1";
   final String name1 = "name1";

   application.setCustomerAccess(new MockCustomerAccess());

   Customer customer1 = new Customer(id1, name1);
   application.add(customer1);
   assertTrue(application.findMatchingCustomers("a%").isEmpty());
   List customers = application.findMatchingCustomers("n%");
   assertEquals(1, customers.size());
   Customer retrievedCustomer = customers.get(0);
   assertEquals(id1, retrievedCustomer.getId());
   assertEquals(name1, retrievedCustomer.getName());
}

The above test demonstrates the ability to search against the customer name using SQL wildcard characters (%).

As I often do, I’ll dig my way down through code, laying in the interface, until I find the point at which I can insert some real code, with tests of course. Then I’ll start working my way back up to the application level. The application code I want to get working:

ApplicationTest

public class Application {
   ...
   private CustomerAccess customerAccess = new CustomerAccess();
   ...
   void setCustomerAccess(CustomerAccess customerAccess) {
      this.customerAccess = customerAccess;
   }
   ...
   public void add(Customer customer) {
      customerAccess.save(customer);
   }

   public List findMatchingCustomers(String namePattern) {
      return customerAccess.findMatches(Customer.NAME, namePattern);
   }
}

I had to make the Customer constant NAME public. I don’t think that’s a bad thing, but for now I chose to make only that constant public and not any other constants.

I built the MockCustomerAccess class to contain a bit of logic. It converts the SQL pattern matching string into a regex string (presuming that only the wildcard character is specified). This code starts treading some dangerous waters. Mock code should generally be simple and straightforward, meeting only the purposes of the tests. If you find yourself maintaining mock code more than you find yourself maintaining production code, you’ve uncovered a nasty smell. I’ll monitor the need to revisit this code in the future.

MockCustomerAccess

package application;

import java.util.*;

import persistence.*;
import domain.*;

class MockCustomerAccess extends CustomerAccess {
   private static List customers = new ArrayList();

   public List findMatches(String columnName, String pattern) {
      String regexPattern = pattern.replaceAll("%", ".*");
      List results = new ArrayList();
      for (Customer customer: customers)
         if (((String)customer.get(columnName)).matches(regexPattern))
            results.add(customer);
      return results;
   }

   public void save(Persistable persistable) {
      customers.add((Customer)persistable);
   }
}

The findMatches method ends up being implemented on the DataAccess superclass, since it’s just as generic as anything else. As soon as I started coding it I recognized that it would have to translate a column name to a Column object. This makes for simpler client code, and also encapsulates client code from any changes to the Column interface.

DataAccess

public List findMatches(String columnName, String namePattern) {
   Column column = getColumn(columnName);
   return new Persister(this).findMatches(column, namePattern);
}

public Column getColumn(String name) {
   for (Column column: getColumns())
      if (column.getName().equals(name))
         return column;
   return null;
}

Of course, I built the getColumn method test-first. Here’s the new test class for DataAccess:

DataAccessTest

package persistence;

import java.util.*;
import junit.framework.*;

public class DataAccessTest extends TestCase {
   private static final String NAME1 = "col1";
   private static final String NAME2 = "col2";
   private static final Column COLUMN1 = new StringColumn(NAME1);
   private static final Column COLUMN2 = new StringColumn(NAME2);

   public void testGetColumn() {
      DataAccess access = new DataAccess() {
         public String getTable() {
            return null;
         }

         public String getKeyColumn() {
            return null;
         }

         public Object create(Map row) {
            return null;
         }

         public Column[] getColumns() {
            return new Column[] { COLUMN1, COLUMN2 };
         }
      };

      assertEquals(COLUMN1, access.getColumn(NAME1));
      assertEquals(COLUMN2, access.getColumn(NAME2));
   }
}

Moving down the chain, PersisterTest is always the big bit of pain in writing tests, since it’s where the JDBC mocking comes into play. The listing here shows a bit of refactoring that I did in order to make the tests short and simple.

PersisterTest

package persistence;

import java.util.*;

import junit.framework.*;

public class PersisterTest extends TestCase {
   private static final String TABLE = "x";
   private static final Object RETURN_OBJECT1 = "object";
   private static final Object RETURN_OBJECT2 = "object2";
   private static final String BAD_KEY = "not found";
   private static final String COLUMN1 = "a";
   private static final String COLUMN2 = "b";
   private static final String ROW1_VALUE1 = "a1";
   private static final String ROW1_VALUE1_PATTERN = "a%";
   private static final String ROW1_VALUE2 = "a2";
   private static final String ROW2_VALUE1 = "b1";
   private static final String ROW2_VALUE2 = "b2";
   private static final Column[] COLUMNS = { new StringColumn(COLUMN1),
         new StringColumn(COLUMN2) };

   private JdbcAccess access;
   private String lastSql;
   private PersistableMetadata<object width="300" height="150"> metadata;
   private Persister<object> persister;
   private Persistable persistable;

   protected void setUp() {
      access = new JdbcAccess() {
         public void execute(String sql) {
            lastSql = sql;
         }

         public Map&lt;String, Object&gt; executeQueryExpectingOneRow(String sql,
               Column[] columns) {
            lastSql = sql;
            if (sql.indexOf(BAD_KEY) &gt; -1)
               return null;
            return createRow1();
         }

         public List&lt;Map&lt;String, Object&gt;&gt; executeQuery(String sql,
               Column[] columns) {
            lastSql = sql;
            return createTwoRows();
         }
      };

      metadata = new PersistableMetadata<object>() {
         public String getTable() {
            return TABLE;
         }

         public String getKeyColumn() {
            return COLUMNS[0].getName();
         }

         public Column[] getColumns() {
            return COLUMNS;
         }

         public Object create(Map&lt;String, Object&gt; row) {
            if (row.get(COLUMN1).equals(ROW1_VALUE1)
                  &amp;&amp; row.get(COLUMN2).equals(ROW1_VALUE2))
               return RETURN_OBJECT1;
            if (row.get(COLUMN1).equals(ROW2_VALUE1)
                  &amp;&amp; row.get(COLUMN2).equals(ROW2_VALUE2))
               return RETURN_OBJECT2;
            return null;
         }
      };

      persistable = new Persistable() {
         public Object get(String key) {
            if (key.equals(COLUMN1))
               return ROW1_VALUE1;
            if (key.equals(COLUMN2))
               return ROW1_VALUE2;
            return null;
         }
      };

      persister = new Persister<object>(metadata, access);
   }

   protected Map&lt;String, Object&gt; createRow1() {
      return createRow(ROW1_VALUE1, ROW1_VALUE2);
   }

   protected Map&lt;String, Object&gt; createRow2() {
      return createRow(ROW2_VALUE1, ROW2_VALUE2);
   }

   private List&lt;Map&lt;String, Object&gt;&gt; createTwoRows() {
      List&lt;Map&lt;String, Object&gt;&gt; results = new ArrayList&lt;Map&lt;String, Object&gt;&gt;();
      results.add(createRow1());
      results.add(createRow2());
      return results;
   }

   protected Map&lt;String, Object&gt; createRow(String value1, String value2) {
      Map&lt;String, Object&gt; row = new HashMap&lt;String, Object&gt;();
      row.put(COLUMN1, value1);
      row.put(COLUMN2, value2);
      return row;
   }

   public void testSave() {
      persister.save(persistable);
      assertLastSql(String.format(
            "insert into %s (%s,%s) values ('%s','%s')", TABLE, COLUMN1,
            COLUMN2, ROW1_VALUE1, ROW1_VALUE2));
   }

   public void testFindBy() {
      final String key = ROW1_VALUE1;
      assertEquals(RETURN_OBJECT1, persister.find(key));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, key));
   }

   public void testFindNotFound() {
      assertNull(persister.find(BAD_KEY));
   }

   public void testGetAll() {
      assertQueryResults(persister.getAll());
      assertEquals(String.format("select %s,%s from %s", COLUMN1,
            COLUMN2, TABLE), lastSql);
   }

   public void testFindMatches() {
      assertQueryResults(persister.findMatches(COLUMNS[0], ROW1_VALUE1_PATTERN));
      assertLastSql(String.format("select %s,%s from %s where %s like '%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, ROW1_VALUE1_PATTERN));
   }

   private void assertLastSql(String sql) {
      assertEquals(sql, lastSql);
   }

   private void assertQueryResults(List<object> results) {
      assertEquals(2, results.size());
      assertTrue(results.contains(RETURN_OBJECT1));
      assertTrue(results.contains(RETURN_OBJECT2));
   }
}


The new test method is <code>testFindMatches</code>. It executes the <code>findMatches</code> method against the Persister class. The test then verifies two things: that the correct SQL was built and passed to the JDBC <code>executeQuery</code> method, and that the (hardcoded) results of the query were gathered correctly (<code>assertQueryResults</code>).

Within Persister, the new <code>findMatches</code> method has a lot in common with <code>getAll</code>. Both need to translate a set of results into domain objects. The only distinction is in the where clause of the SQL select statement.
<h4>Persister</h4>
<pre>public List<t> getAll() {
   String sql = new SqlGenerator().createSelectAll(metadata.getTable(),
         metadata.getColumns());
   return executeQuery(sql);
}

public List<t> findMatches(Column column, String pattern) {
   String sql = new SqlGenerator().createSelect(metadata.getTable(),
         metadata.getColumns(), column, pattern);
   return executeQuery(sql);
}

private List<t> executeQuery(String sql) {
   List&lt;Map&lt;String, Object&gt;&gt; rows = access.executeQuery(sql, metadata
         .getColumns());
   List<t> results = new ArrayList<t>(rows.size());
   for (Map&lt;String, Object&gt; row : rows)
      results.add(metadata.create(row));
   return results;
}
</t></t></t></t></t>

Here are SqlGeneratorTest and SqlGenerator. I refactored SqlGenerator after adding createSelect.

SqlGeneratorTest

public class SqlGeneratorTest extends TestCase {
   private static Column[] COLUMNS = { new StringColumn("a"),
         new StringColumn("b") };
   ...
   public void testFindMatches() {
      final String pattern = "v%";
      final Column column = COLUMNS[1];
      assertEquals("select a,b from t where b like 'v%'", generator
            .createSelect(TABLE, COLUMNS, column, pattern));
   }
}

SqlGenerator

public class SqlGenerator {
   ...
   public String createSelectAll(String table, Column[] columns) {
      return String.format("select %s from %s", createColumnList(columns),
            table);
   }

   public String createFindByKey(String tableName, Column[] columns,
         String keyColumn, String keyValue) {
      final String criteria = String.format("%s='%s'", keyColumn, keyValue);
      return createSelectWithCriteria(tableName, columns, criteria);
   }

   public String createSelect(String tableName, Column[] columns,
         Column column, String pattern) {
      final String criteria = String.format("%s like '%s'", column.getName(),
            pattern);
      return createSelectWithCriteria(tableName, columns, criteria);
   }

   private String createSelectWithCriteria(String tableName, Column[] columns,
         String criteria) {
      String whereClause = String.format("where %s", criteria);
      return String.format("%s %s", createSelectAll(tableName, columns),
            whereClause);
   }
}

Where clause criteria needs are going to change. As mentioned earlier, I’ll need multiple conditionals and so on. Right now I’m ok with what I have, but I want to soon shift the burden of constructing a where clause to the client. Otherwise the SqlGenerator class and other code will need to change infinitely into the future, with almost each new query conditional that a client requires. Fixing that is a future installment.

Database TDD Part 22: Multiple Rows

Lots of code today. The application needs to be able to obtain a list of all the user names that are registered in the system. Starting at the application layer:

ApplicationTest

package application;

import java.util.*;

import junit.framework.*;

public class ApplicationTest extends TestCase {
   final String name1 = "name1";
   final String password1 = "password1";
   final String name2 = "name2";
   final String password2 = "password2";

   private Application application;

   protected void setUp() {
      application = new Application();
      application.setUserAccess(new MockUserAccess());
   }

   public void testVerifyUser() {
      assertFalse(application.isRegistered(name1, password1));
      application.registerUser(name1, password1);
      assertTrue(application.isRegistered(name1, password1));
   }

   public void testRegisteredUsers() {
      application.registerUser(name1, password1);
      application.registerUser(name2, password2);
      Set<String> users = application.getRegisteredUsers();
      assertEquals(2, users.size());
      assertTrue(users.contains(name1));
      assertTrue(users.contains(name2));
   }
}

Application

package application;

import java.util.*;

import domain.*;

public class Application {
   private UserAccess userAccess = new UserAccess();

   public void registerUser(String name, String password) {
      User user = new User(name, password);
      userAccess.save(user);
   }

   public boolean isRegistered(String name, String password) {
      User user = userAccess.find(name);
      return user != null;
   }

   void setUserAccess(UserAccess userAccess) {
      this.userAccess = userAccess;
   }

   public Set<String> getRegisteredUsers() {
      Set<String> results = new HashSet<String>();
      for (User user: userAccess.getAll())
         results.add(user.getName());
      return results;
   }
}

To build this solution, I drove all the way down from the application layer, programming by intent. Here in the Application class, the job was to flesh out what the getRegisteredUsers method might look like. I declared the need for a getAll method defined in the data access class.

Getting the code to work meant updating the mock class.

MockUserAccess

...
public class MockUserAccess extends UserAccess {
   private static User user;
   private static List<User> users = new ArrayList<User>();

   public User find(String key) {
      return MockUserAccess.user;
   }

   public void save(Persistable persistable) {
      MockUserAccess.user = (User)persistable;
      users.add(user);
   }

   public List<User> getAll() {
      return users;
   }
}

Note that there are two kinds of things going on here. I’m not worrying about that yet.

DataAccess

   public List<T> getAll() {
      return new Persister<T>(this).getAll();
   }

I just realized that this method won’t be getting tested as long as I’m using the mock class in ApplicationTest. Looks like a future exercise needs to be an acceptance level test. Right now I’m only concerned about building the units. Acceptance tests are a good end-of-day exercise. I figure so far, from blog entry #1 to #22, I’m probably up to 2:30 in the afternoon, were I to have been working on this codebase nonstop.

PersisterTest

package persistence;

import java.util.*;

import junit.framework.*;

public class PersisterTest extends TestCase {
   private static final String TABLE = "x";
   private static final Object RETURN_OBJECT1 = "object";
   private static final Object RETURN_OBJECT2 = "object2";
   private static final String BAD_KEY = "not found";
   private static final String COLUMN1 = "a";
   private static final String COLUMN2 = "b";
   private static final String ROW1_VALUE1 = "a1";
   private static final String ROW1_VALUE2 = "a2";
   private static final String ROW2_VALUE1 = "b1";
   private static final String ROW2_VALUE2 = "b2";
   private static final Column[] COLUMNS = { new StringColumn(COLUMN1),
         new StringColumn(COLUMN2) };

   private JdbcAccess access;
   private String lastSql;
   private PersistableMetadata<Object> metadata;
   private Persister<Object> persister;
   private Persistable persistable;

   protected void setUp() {
      access = new JdbcAccess() {
         public void execute(String sql) {
            lastSql = sql;
         }

         public Map<String, Object> executeQueryExpectingOneRow(String sql,
               Column[] columns) {
            lastSql = sql;
            if (sql.indexOf(BAD_KEY) > -1)
               return null;
            return createRow1();
         }

         public List<Map<String, Object>> executeQuery(String sql,
               Column[] columns) {
            lastSql = sql;

            List<Map<String, Object>> results = new ArrayList<Map<String, Object>>();
            results.add(createRow1());
            results.add(createRow2());
            return results;
         }
      };

      metadata = new PersistableMetadata<Object>() {
         public String getTable() {
            return TABLE;
         }

         public String getKeyColumn() {
            return COLUMNS[0].getName();
         }

         public Column[] getColumns() {
            return COLUMNS;
         }

         public Object create(Map<String, Object> row) {
            if (row.get(COLUMN1).equals(ROW1_VALUE1)
                  && row.get(COLUMN2).equals(ROW1_VALUE2))
               return RETURN_OBJECT1;
            if (row.get(COLUMN1).equals(ROW2_VALUE1)
                  && row.get(COLUMN2).equals(ROW2_VALUE2))
               return RETURN_OBJECT2;
            return null;
         }
      };

      persistable = new Persistable() {
         public Object get(String key) {
            if (key.equals(COLUMN1))
               return ROW1_VALUE1;
            if (key.equals(COLUMN2))
               return ROW1_VALUE2;
            return null;
         }
      };

      persister = new Persister<Object>(metadata, access);
   }

   protected Map<String, Object> createRow1() {
      return createRow(ROW1_VALUE1, ROW1_VALUE2);
   }

   protected Map<String, Object> createRow2() {
      return createRow(ROW2_VALUE1, ROW2_VALUE2);
   }

   protected Map<String, Object> createRow(String value1, String value2) {
      Map<String, Object> row = new HashMap<String, Object>();
      row.put(COLUMN1, value1);
      row.put(COLUMN2, value2);
      return row;
   }

   public void testSave() {
      persister.save(persistable);

      String expectedSql = String.format(
            "insert into %s (%s,%s) values ('%s','%s')", TABLE, COLUMN1,
            COLUMN2, ROW1_VALUE1, ROW1_VALUE2);
      assertEquals(expectedSql, lastSql);
   }

   public void testFindBy() {
      final String key = ROW1_VALUE1;
      assertEquals(RETURN_OBJECT1, persister.find(key));
      String expectedSql = String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, key);
      assertEquals(expectedSql, lastSql);
   }

   public void testFindNotFound() {
      assertNull(persister.find(BAD_KEY));
   }

   public void testGetAll() {
      persister.save(persistable);

      Persistable persistable2 = new Persistable() {
         public Object get(String key) {
            if (key.equals(COLUMN1))
               return ROW2_VALUE1;
            if (key.equals(COLUMN2))
               return ROW2_VALUE2;
            return null;
         }
      };
      persister.save(persistable2);
      List<Object> results = persister.getAll();

      String expectedSql = String.format("select %s,%s from %s", COLUMN1,
            COLUMN2, TABLE);
      assertEquals(expectedSql, lastSql);

      assertEquals(2, results.size());
      assertTrue(results.contains(RETURN_OBJECT1));
      assertTrue(results.contains(RETURN_OBJECT2));
   }
}

This class got a bit of refactoring. The concept for testing “get all” is pretty much the same as testing find-by, except that there are multiple rows that need to be verified. I’m still not enamored of the complexity in setting up the tests in this class.

Persister

...
public class Persister<T> {
   ...
   public List<T> getAll() {
      String sql = new SqlGenerator().createSelectAll(metadata.getTable(), metadata.getColumns());
      List<Map<String, Object>> rows = access.executeQuery(sql, metadata.getColumns());
      List<T> results = new ArrayList<T>(rows.size());
      for (Map<String, Object> row: rows)
         results.add(metadata.create(row));
      return results;
   }
}

The new method in Persister is much like the find method, except that once again it’s dealing with multiple rows. Here I had to comment code in getAlluntil I was able to get to a point where it would compile. That meant a diversion to get the new SqlGenerator and JdbcAccess methods in place. That’s my process: write a test, attempt to implement method by intent, comment out non-compiling code, write a test at the lower level as necessary, uncomment out compile code and retest. There are other ways of doing this, of course.

SqlGenerator code was pretty straightforward:

SqlGeneratorTest

   public void testGetAll() {
      assertEquals("select a,b from t", generator.createSelectAll(TABLE, COLUMNS));
   }

SqlGenerator

   public String createSelectAll(String table, Column[] columns) {
      return String.format("select %s from %s", createColumnList(columns),
            table);
   }

Hmm, some minor duplication between that and createFindByKey that I had forgotten about. Let me go back and fix that…

   public String createFindByKey(String tableName, Column[] columns,
         String keyColumn, String keyValue) {
      return String.format("%s where %s='%s'",
         createSelectAll(tableName, columns), keyColumn, keyValue);
   }

You’ll note that in Persister’s getAll method, the return value from executeQuery is a List of rows (maps), instead of a single map that represents a row. In order to build this code incrementally, I first named the new form of the executeQuery method executeQueryMultipleResults. Once I got everything working, I went back and flipped the method names around (so that the abnormal case–executing a query and expecting one row–was the more explicitly named method).

Here’s JdbcAccessTest:

package persistence;

import java.util.*;

import junit.framework.*;

public class JdbcAccessTest extends TestCase {
   ...
   private static final String VALUE1 = "a";
   private static final String VALUE2 = "b";
   ...
   public void testExecuteQuery() {
      insertRow();
      assertRow(access.executeQueryExpectingOneRow(createSelectSql(), COLUMNS));
   }
   ...
   public void testExecuteQueryNoResults() {
      assertNull(access.executeQueryExpectingOneRow(createSelectSql() + " where 1=0", COLUMNS));
   }

   public void testExecuteQueryMultipleResults() {
      insertRow(VALUE1);
      insertRow(VALUE2);

      List<Map<String, Object>> rows = access.executeQuery(createSelectSql(), COLUMNS);
      assertEquals(2, rows.size());
      assertContains(rows, VALUE1);
      assertContains(rows, VALUE2);
   }

   private void assertContains(List<Map<String, Object>> rows, String columnValue) {
      Map<String, Object> expectedRow = new HashMap<String, Object>();
      expectedRow.put(COLUMN_NAME, columnValue);
      assertTrue(rows.contains(expectedRow));
   }

   private void assertRow(Map<String, Object> row) {
      assertEquals(1, row.size());
      assertEquals(VALUE1, row.get(COLUMN_NAME));
   }

   private void insertRow() {
      insertRow(VALUE1);
   }

   private void insertRow(String value) {
      String sql = String.format("insert into %s values('%s')", TABLE, value);
      access.execute(sql);
   }
   ...
}

And finally, JdbcAccess, to which I added one comment regarding something that I might want to look at for performance considerations later.

JdbcAccess

...
public class JdbcAccess {
   ...
   public Map<String, Object> executeQuery(String sql) {
      return executeQueryExpectingOneRow(sql, null);
   }

   public Map<String, Object> executeQueryExpectingOneRow(String sql, Column[] columns) {
      List<Map<String, Object>> rows = executeQuery(sql, columns);
      if (rows.isEmpty())
         return null;
      return rows.get(0);
   }

   public List<Map<String, Object>> executeQuery(String sql, Column[] columns) {
      try {
         createStatement();

         ResultSet results = statement.executeQuery(sql);
         List<Map<String,Object>> rows = new ArrayList<Map<String,Object>>();

         if (columns == null) // what if results is empty, wasteful
            columns = createPseudoColumns(results.getMetaData());

         while (results.next()) {
            Map<String, Object> row = getRow(results, columns);
            rows.add(row);
         }
         results.close();

         connection.close();
         return rows;
      } catch (SQLException e) {
         throw new JdbcException(sql, e);
      }
   }
   ...
}

One thing I derived from today’s exercise: adding new functionality was a pretty straightforward exercise of digging down through the layers, with a bit of refactoring cleanup along the way. A lot of the code necessary was already in place. As I add more functionality, it creates new groundwork that makes addition of future functionality even easier.

Database TDD Part 21: Revisiting Performance

The casting in the domain classes is going to continue to be a major nuisance. The Customer class is already demonstrating a good amount of casting clutter for such a small class.

Customer

public class Customer implements Persistable {
   static final String ID = "id";
   static final String NAME = "name";
   static final String BALANCE = "balance";
   private Map<String,Object> values = new HashMap<String,Object>();

   public Customer(String id, String name) {
      values.put(ID, id);
      values.put(NAME, name);
      values.put(BALANCE, 0);
   }

   public String getId() {
      return (String)get(ID);
   }

   public String getName() {
      return (String)get(NAME);
   }

   public Object get(String key) {
      return values.get(key);
   }

   public int getBalance() {
      return (Integer)values.get(BALANCE);
   }

   public void charge(int amount) {
      values.put(BALANCE, getBalance() + amount);
   }
}

Casting is a form of duplication that I can get rid of in several ways. One solution would be to create a different collection for each stored type. Another solution would be to isolate it to a separate class, so none of the domain classes would need to cast.

Each domain class can store an instance of a FieldMap, which encapsulates all the nuisances of casting. It can also handle common update operations as they are coded, such as adding to an already-stored value (like the charge method does).

FieldMapTest

package domain;

import junit.framework.*;

public class FieldMapTest extends TestCase {
   private static final String KEY = "key";
   private FieldMap map;

   protected void setUp() {
      map = new FieldMap();
   }

   public void testGet() {
      Object object = new Object();
      map.put(KEY, object);
      assertSame(object, map.get(KEY));
   }

   public void testString() {
      final String value = "abc";
      map.put(KEY, value);
      assertEquals(value, map.getString(KEY));
   }

   public void testNullString() {
      map.put(KEY, null);
      assertNull(map.getString(KEY));
   }

   public void testStringNotPut() {
      assertNull(map.getString(KEY));
   }

   public void testInteger() {
      map.put(KEY, 1);
      assertEquals(1, map.getInt(KEY));
   }

   public void testIntegerNotPut() {
      assertEquals(0, map.getInt(KEY));
   }

   public void testAdd() {
      map.put(KEY, 1);
      map.add(KEY, 2);
      assertEquals(3, map.getInt(KEY));
   }
}

FieldMap

package domain;

import java.util.*;

public class FieldMap {
   private Map<String,Object> values = new HashMap<String,Object>();

   public void put(String key, Object value) {
      values.put(key, value);
   }

   public String getString(String key) {
      return (String)values.get(key);
   }

   public int getInt(String key) {
      Integer value = (Integer)values.get(key);
      if (value == null)
         return 0;
      return value.intValue();
   }

   public void add(String key, int amount) {
      int value = getInt(key);
      put(key, value + amount);
   }

   public Object get(String key) {
      return values.get(key);
   }

}

Modifications to the domain classes get rid of much of the clutter.

Customer

package domain;

import persistence.*;

public class Customer implements Persistable {
   static final String ID = "id";
   static final String NAME = "name";
   static final String BALANCE = "balance";

   private FieldMap values = new FieldMap();

   public Customer(String id, String name) {
      values.put(ID, id);
      values.put(NAME, name);
      values.put(BALANCE, 0);
   }

   public String getId() {
      return values.getString(ID);
   }

   public String getName() {
      return values.getString(NAME);
   }

   public int getBalance() {
      return values.getInt(BALANCE);
   }

   public void charge(int amount) {
      values.add(BALANCE, amount);
   }

   public Object get(String key) {
      return values.get(key);
   }
}

User

package domain;

import persistence.*;

public class User implements Persistable {
   static final String PASSWORD = "password";
   static final String NAME = "name";
   private FieldMap values = new FieldMap();

   public User(String name, String password) {
      values.put(NAME, name);
      values.put(PASSWORD, password);
   }

   public String getName() {
      return values.getString(NAME);
   }

   public String getPassword() {
      return values.getString(PASSWORD);
   }

   public Object get(String key) {
      return values.get(key);
   }
}

There’s still the common get method and declaration of the values FieldMap instance. The get method is expected by clients of the Persistable interface. I could conceivably foist those elements into a superclass, but I’m going to hold off a bit on that.

One concern that use of the FieldMap might bring up is its overhead in performance cost. With respect to numeric operations, the code now must wrap and unwrap int values in order to do any math with them. It’s pretty unlikely that anything here would truly present a problem–the real overhead in production will be the cost of persistence, not the cost of wrapping and unwrapping. But let’s say that it does become an issue.

As a pretty contrived example, suppose I’ve been told that the charge method must support 5,000 operations per second. Currently, it’s only supporting about 1,000 operations per second.

First step is, as always, find some way to measure the existing performance.

User

package domain;

import junit.framework.*;

public class CustomerPerformanceTest extends TestCase {
   public void testBalance() {
      Customer customer = new Customer("","");
      final int amount = 2;
      final int iterations = 1000;
      assertEquals(0, customer.getBalance());

      long start = System.nanoTime();
      for (int i = 0; i < iterations; i++)
         customer.charge(amount);
      assertEquals(amount * iterations, customer.getBalance());
      long stop = System.nanoTime();

      long microseconds = (stop - start) / 1000;
      assertTrue("microseconds: " + microseconds, microseconds < 200);
   }
}

Sure enough, this test fails fairly consistently (on my sluggish laptop). It takes about 800 microseconds to complete 1000 iterations.

The design is flexible enough to allow me to optimize things:

Customer

package domain;

import persistence.*;

public class Customer implements Persistable {
   static final String ID = "id";
   static final String NAME = "name";
   static final String BALANCE = "balance";
   // balance is cached to meet performance goals
   private int balance = 0;

   private FieldMap values = new FieldMap();

   public Customer(String id, String name) {
      values.put(ID, id);
      values.put(NAME, name);
   }

   public String getId() {
      return values.getString(ID);
   }

   public String getName() {
      return values.getString(NAME);
   }

   public int getBalance() {
      return balance;
   }

   public void charge(int amount) {
      balance += amount;
   }

   public Object get(String key) {
      if (key == BALANCE)
         return new Integer(balance);
      return values.get(key);
   }
}

I added the balance field to allow for direct int manipulation. When the persistence framework needs to access the balance, code in the get method uses a guard clause to treat the balance as a specially cached value.

Comments are important here. Optimization is usually a curious looking thing in the code. Developers can innocently refactor performance optimization away if it’s not clear why it exists. Stating the performance goals in the form of tests can go a long way toward diminishing the verbosity of such comments. Short-term, I’ll leave the optimized code in place, but I’ll probably delete it (and the requirement/test) next time I touch the Customer class.

So far, the design of the system hasn’t prevented me from being able to keep it running fast. I’ll make sure I revisit performance goals as I go.

Database TDD Part 20: Changing the Result Row to a Map

One thing I’m displeased with is the way that a query result gets mapped into the domain object.

   private static Column[] COLUMNS = new Column[] {
      new StringColumn(Customer.ID),
      new StringColumn(Customer.NAME),
      new IntegerColumn(Customer.BALANCE)
   };
   ...
   public Customer create(List<Object> row) {
      Customer customer = new Customer((String)row.get(0), (String)row.get(1));
      customer.charge((Integer)row.get(2));
      return customer;
   }

You know that the 0th element in a row is the customer ID only by correlating with the COLUMNS declaration at the top of CustomerAccess. This portends all sorts of maintenance nightmares that will occur when developers update the list of columns and forget to modify thecreate method, or vice versa. A better solution would be to have the query result returned as a map between column and value.

Here is the modified JdbcAccessTest:

package persistence;

import java.util.*;

import junit.framework.*;

public class JdbcAccessTest extends TestCase {
   private static final String TABLE = "JdbcAccessTest";
   private static final String COLUMN_NAME = "x";
   private static final String VALUE = "a";
   private static final Column[] COLUMNS = { new StringColumn(COLUMN_NAME) };

   private JdbcAccess access;

   protected void setUp() {
      access = new JdbcAccess();
      access.execute(String.format("create table %s (%s varchar(1))", TABLE,
            COLUMN_NAME));
   }

   protected void tearDown() {
      access.execute("drop table " + TABLE);
   }

   public void testExecuteQuery() {
      insertRow();
      assertResults(access.executeQuery(createSelectSql(), COLUMNS));
   }

   public void testExecuteQueryWithoutMetadata() {
      insertRow();
      assertResults(access.executeQuery(createSelectSql()));
   }

   public void testExecuteQueryNoResults() {
      assertNull(access.executeQuery(createSelectSql() + " where 1=0", COLUMNS));
   }

   private void assertResults(Map<String, Object> row) {
      assertEquals(1, row.size());
      assertEquals(VALUE, row.get(COLUMN_NAME));
   }

   private void insertRow() {
      String sql = String.format("insert into %s values('%s')", TABLE, VALUE);
      access.execute(sql);
   }

   private String createSelectSql() {
      return String.format("select %s from %s", COLUMN_NAME, TABLE);
   }
}

You’ll note that I refactored this code a good extent from the last state it was in. Once I had modified the executeQuery tests, I recognized lots of little problems that I cleaned up in a matter of minutes.

You might also note that there is no longer a method that explicitly tests the execute method. I chose to extract this test to its own class, JdbcAccessExecuteTest:

package persistence;

import java.util.*;

import junit.framework.*;

public class JdbcAccessExecuteTest extends TestCase {
   final String TABLE = "JdbcAccessTest";
   final String COLUMN_NAME = "x";
   private JdbcAccess access;

   protected void setUp() {
      access = new JdbcAccess();
   }

   public void testExecute() {
      try {
         access.execute(String.format("create table %s (%s varchar(1))", TABLE,
               COLUMN_NAME));
         assertEquals(0, getCount());
      } finally {
         access.execute(String.format("drop table %s", TABLE));
      }
   }

   private long getCount() {
      final String sql = String.format("select count(*) from %s", TABLE);
      Map<String, Object> row = access.executeQuery(sql);
      return ((Long)row.get("count(*)")).longValue();
   }
}

Putting testExecute into its own test class allowed for consistent symmetry between the setUp and tearDown methods in both test classes. The sole method in JdbcAccessExecuteTest does not require prior creation of a test table–creating the test table is the thing that’s getting tested. Each of the test methods in JdbcAccessTest do require that a test table get created and subsequently dropped after the test method completes.

The new preferred form of the executeQuery method, specified in the JdbcAccessTest method testExecuteQuery, takes a column array as its second argument. The name of each column is used as a key for putting each corresponding column value into the result row hash table.

...
public class JdbcAccess {
   ...
   public Map<String, Object> executeQuery(String sql) {
      return executeQuery(sql, null);
   }

   public Map<String, Object> executeQuery(String sql, Column[] columns) {
      try {
         createStatement();

         ResultSet results = statement.executeQuery(sql);
         Map<String, Object> row = null;
         if (results.next()) {
            if (columns == null)
               columns = createPseudoColumns(results.getMetaData());

            row = getRow(results, columns);
         }
         results.close();

         connection.close();
         return row;
      } catch (SQLException e) {
         throw new JdbcException(sql, e);
      }
   }

   private Column[] createPseudoColumns(ResultSetMetaData metadata) throws SQLException {
      Column[] columns = new Column[metadata.getColumnCount()];
      for (int i = 1; i <= metadata.getColumnCount(); i++)
         columns[i - 1] = new StringColumn(metadata.getColumnName(i));
      return columns;
   }

   private Map<String, Object> getRow(ResultSet results, Column[] columns) throws SQLException {
      Map<String, Object> row = new HashMap<String, Object>();
      for (Column column : columns)
         row.put(column.getName(), results.getObject(column.getName()));
      return row;
   }
   ...
}

If no column metadata is provided, the executeQuery method simply constructs dummy columns from the result set’s metadata.

After I got this working, I moved back to the Persister class, which is the only place that the executeQuery method is called. As a more incremental, intermediary step, I chose to not change the PersistableMetadata interface, so that the create method (T create(List<Object> row)) still expects a list instead of a map. This allowed me to isolate changes for the time being to Persister and its test.

package persistence;

import java.util.*;

import junit.framework.*;

public class PersisterTest extends TestCase {
   private static final String TABLE = "x";
   private static final Object RETURN_OBJECT = "object";
   private static final String BAD_KEY = "not found";
   private static final String COLUMN1 = "a";
   private static final String COLUMN2 = "b";
   private static final String VALUE1 = "1";
   private static final String VALUE2 = "2";
   private static final Column[] COLUMNS = { new StringColumn(COLUMN1),
         new StringColumn(COLUMN2) };

   private JdbcAccess access;
   private String lastSql;
   private PersistableMetadata<Object> metadata;
   private Persister<Object> persister;

   protected void setUp() {
      access = new JdbcAccess() {
         public void execute(String sql) {
            lastSql = sql;
         }

         public Map<String, Object> executeQuery(String sql, Column[] columns) {
            lastSql = sql;
            if (sql.indexOf(BAD_KEY) > -1)
               return null;
            Map<String, Object> results = new HashMap<String, Object>();
            results.put(COLUMN1, VALUE1);
            results.put(COLUMN2, VALUE2);
            return results;
         }
      };

      metadata = new PersistableMetadata<Object>() {
         public String getTable() {
            return TABLE;
         }

         public String getKeyColumn() {
            return COLUMNS[0].getName();
         }

         public Object create(List<Object> row) {
            if (row.get(0).equals(VALUE1) &amp;&amp; row.get(1).equals(VALUE2))
               return RETURN_OBJECT;
            return null;
         }

         public Column[] getColumns() {
            return COLUMNS;
         }
      };

      persister = new Persister<Object>(metadata, access);
   }

   public void testSave() {
      Persistable persistable = new Persistable() {
         public Object get(String key) {
            if (key.equals(COLUMN1))
               return VALUE1;
            if (key.equals(COLUMN2))
               return VALUE2;
            return null;
         }
      };

      persister.save(persistable);

      String expectedSql = String.format(
            "insert into %s (%s,%s) values ('%s','%s')", TABLE, COLUMN1,
            COLUMN2, VALUE1, VALUE2);
      assertEquals(expectedSql, lastSql);
   }

   public void testFindBy() {
      final String key = VALUE1;
      assertEquals(RETURN_OBJECT, persister.find(key));
      String expectedSql = String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, key);
      assertEquals(expectedSql, lastSql);
   }

   public void testFindNotFound() {
      assertNull(persister.find(BAD_KEY));
   }
}

(You’ll note a bit of refactoring here too.) The setup for the test method testFindBy in PersisterTest had to change accordingly. I modified the executeQuery method override for JdbcAccess to create a map with forced results. The create method override for PersistableMetadata now ensures that the result row contains these appropriate results.

In Persister, I changed the find method to pass the column list into executeQuery. I then added the temporary code to push the values from the result row hashmap into the ordered list row (the row that gets passed to the metadata’s create method).

   public T find(String key) {
      String sql = new SqlGenerator().createFindByKey(metadata.getTable(),
            metadata.getColumns(), metadata.getKeyColumn(), key);
      Map<String, Object> row = access.executeQuery(sql, metadata.getColumns());
      if (row == null)
         return null;
      List<Object> tempRow = new ArrayList<Object>();
      for (Column column: metadata.getColumns())
         tempRow.add(row.get(column.getName()));
      return metadata.create(tempRow);
   }

The last set of steps is an incremental refactoring exercise.

  • Step 1: Add an overloaded create method (T create(Map row);) to the PersistableMetadata interface.
  • Step 2: Fix all compilation errors by implementing the interface where appropriate.
  • Step 3: Build the appropriate create method override code in PersisterTest.
  • Step 4: Modify Persister to call the new create method. This will break tests for UserAccess and CustomerAccess.
  • Step 5: Implement the new create method correctly in UserAccess and CustomerAccess. Ensure tests pass.
  • Step 6: Remove the old create method from PersistableMetadata and eliminate its implementations in PersisterTest, UserAccess, and CustomerAccess.

The final relevant bits of code:

PersistableMetadata

package persistence;

import java.util.*;

public interface PersistableMetadata&lt;T&gt; {
   String getTable();
   String getKeyColumn();
   T create(Map<String,Object> row);
   Column[] getColumns();
}

PersisterTest

   protected void setUp() {
      ...
      metadata = new PersistableMetadata<Object>() {
         ...
         public Object create(Map<String,Object> row) {
            if (row.get(COLUMN1).equals(VALUE1) && row.get(COLUMN2).equals(VALUE2))
               return RETURN_OBJECT;
            return null;
         }
      };

      persister = new Persister<Object>(metadata, access);
   }

Persister

   public T find(String key) {
      String sql = new SqlGenerator().createFindByKey(metadata.getTable(),
            metadata.getColumns(), metadata.getKeyColumn(), key);
      Map<String,Object> row = access.executeQuery(sql, metadata.getColumns());
      if (row == null)
         return null;
      return metadata.create(row);
   }

UserAccess

   private static Column[] COLUMNS = new Column[] {
      new StringColumn(User.NAME),
      new StringColumn(User.PASSWORD)
   };

   public User create(Map<String,Object> row) {
      return new User((String)row.get(User.NAME), (String)row.get(User.PASSWORD));
   }

CustomerAccess

   private static Column[] COLUMNS = new Column[] {
      new StringColumn(Customer.ID),
      new StringColumn(Customer.NAME),
      new IntegerColumn(Customer.BALANCE)
   };

   public Customer create(Map<String,Object> row) {
      Customer customer =
         new Customer((String)row.get(Customer.ID), (String)row.get(Customer.NAME));
      customer.charge((Integer)row.get(Customer.BALANCE));
      return customer;
   }

I think the big lesson today is again one of controlled, incremental refactoring. At no time did I spend more than a couple minutes before I saw a green bar. I accomplished this in at least one case by introducing code that I knew would disappear shortly. The tradeoff is then one of guaranteed forward progress for slightly increased overall development time. Without taking this approach, I’m targeting potentially slightly diminished overall development time, but I run a risk of significantly increased overall development time if things go awry.

Database TDD Part 19: Incremental Refactoring

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<Object> 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.

Atom