This morning: a five-minute refactoring to further eliminate duplication in the code. SQL statements are inherently redundant. They're also risky to put together without a test against a live database.

Let's hit the code. Here's the entire User class:

import java.util.*;

public class User {
   private String name;
   private String password;
   private static final String TABLE_NAME = "userdata";
   private static String[] columns = { "name", "password" };

   public User(String name, String password) {
      this.name = name;
      this.password = password;
   }

   public String getName() {
      return name;
   }

   public String getPassword() {
      return password;
   }

   public void save() {
      new JdbcAccess().execute(String.format("insert into " + TABLE_NAME + " ("
            + User.createColumnList() + ") values ('%s', '%s')", name, password));
   }

   private static String createColumnList() {
      StringBuilder builder = new StringBuilder();
      for (int i = 0; i < columns.length; i++) {
         if (i > 0)
            builder.append(',');
         builder.append(columns[i]);
      }
      return builder.toString();
   }

   public static User find(String nameKey) {
      JdbcAccess access = new JdbcAccess();
      List<String> row = access.executeQuery(String.format(
            "select " + createColumnList() + " from " + TABLE_NAME + " where name = '%s'",
            nameKey));
      return new User(row.get(0), row.get(1));
   }
}

This was three refactoring passes, each concluded with the execution of all tests in the project. The first pass involved extracting the table name to a constant; 30 seconds. The second pass involved extracting the construction of the insert column list to a separate method that uses a String array of column names; 4 minutes. The third pass involved using createColumnList from the find method; 30 seconds.

The table name and the column lists were an obvious place to start. What about the values list in the insert statement? What about the duplication inherent in the User attributes themselves?

Another thought: the Single Responsibility Principle is getting more and more abused. It's bad enough that the User domain class deals with persistence. Now there's a new method createColumnList that is a generic String utility method. It belongs elsewhere. But don't fret, we'll get to that soon.

posted by Jeff L.  # 10/14/2005 10:39:00 AM Database TDD Blogs