Having explicit fields in Java is a source of redundancy–one that I’m usually willing to live with. But in the case of persisting fields from objects, they’re all one and the same kind of thing, so why deal with the nuisance of having them stored in separately named fields? Moving forward, I’ve decided to eliminate this redundancy so that my persistence code can be cheaply maintained.

The first step is to convert the internal representation of the customer fields to a domain map.

    package domain;
    
    import junit.framework.*;
    
    public class CustomerTest extends TestCase {
       private Customer customer;
    
       public void testCreate() {
          final String id = "123";
          final String name = "Smelt Systems";
          customer = new Customer(id, name);
          assertField(id, customer.getId(), Customer.ID);
          assertField(name, customer.getName(), Customer.NAME);
       }
    
       private void assertField(String expected,
             String actual, String key) {
          assertEquals(key, expected, actual);
          assertEquals(key, expected, customer.get(key));
       }
    }

Question: should I write a test against the values map? I intend for it to be exposed at package level access, at least for the time being. I’ll keep the test for now and if it causes any dependency headaches, it’s gone. I think the issue will resolve itself when I get the persistence stuff updated for both domain classes.

    package domain;
    
    import java.util.*;
    
    public class Customer {
       static final String ID = "id";
       static final String NAME = "name";
       private Map values = new HashMap();
    
       public Customer(String id, String name) {
          values.put(ID, id);
          values.put(NAME, name);
       }
    
       public String getId() {
          return values.get(ID);
       }
    
       public String getName() {
          return values.get(NAME);
       }
    
       String get(String key) {
          return values.get(key);
       }
    }

This raises a couple of flags. First off, I’m still dealing only with String fields, which makes things simpler. Ultimately we’ll see how refactorable this is when I introduce a new type. Second, what of null values? Or what happens if a field is not initialized (i.e. the constructor never puts a value in)?

The first question is easy to resolve:

       public void testNullValues() {
          Customer customer = new Customer(null, null);
          assertNull(customer.getId());
          assertNull(customer.getName());
       }

The test passes. The second question relates to something I don’t care about yet, but maybe it’s a good idea to write a sketch of a test.

       /*
       public void futuretestNulls() {
          Customer customer = new Customer();
          assertNull(customer.getId());
       }
       */

The problem with “reminder” tests like these is that they sometimes stick around forever. I tend to delete them when I see them on principle. Here I might keep it around for my own use since I know that I’m going to hit this code shortly. I’d probably avoid checking in reminder tests like these.

For the keys to the domain map, I deliberately chose the same names as the column names in the customer table. Why not? The only problem I have with this scheme is that it’s a hidden dependency that should somehow be made more explicit. Implications of the dependency are that things can break if a developer is unaware of it; also, the Customer class must recompile if the column names change. I’m ok with the second part. Suggestions on how to properly document the hidden dependency?

Here’s where the fun begins. The save method in CustomerAccess becomes more generic:

    public class CustomerAccess {
       private static final String TABLE = "cust";
       private static String[] COLUMNS = { Customer.ID, Customer.NAME };
    
       public void save(Customer customer) {
          String[] fields = new String[COLUMNS.length];
          for (int i = 0; i < COLUMNS.length; i++)
             fields[i] = customer.get(COLUMNS[i]);
          String sql = new SqlGenerator().createInsert(TABLE, COLUMNS, fields);
          new JdbcAccess().execute(sql);
       }

The only thing specific is the Customer type–I hear an interface calling.

After changing the implementation of User to also be a domain map, and after making the same changes to the UserAccess, I can now start to factor out the duplication. Both Customer and User have the values map. I could have them both inherit from a common class, but I'm very reluctant to introduce inheritance, since you get one shot. For now, I'm going to concentrate on the duplication in the save method, and use an interface to define the commonality between Customer and User.

    package domain;
    
    public interface Persistable {
       String get(String key);
    }

Now the save method can be written as:

       public void save(Persistable persistable) {
          String[] fields = new String[COLUMNS.length];
          for (int i = 0; i < COLUMNS.length; i++)
             fields[i] = persistable.get(COLUMNS[i]);
          String sql = new SqlGenerator().createInsert(TABLE, COLUMNS, fields);
          new JdbcAccess().execute(sql);
       }

… in both UserAccess and CustomerAccess.

I can now either create a new class that contains the save method, and delegate from the access objects. Or I can push up the method into a common superclass. Either solution requires a bit of work. The problem with pushing up the method is that it refers to the subclass-specifc TABLE and COLUMNS fields. I could push that information into abstract method overrides, and set save up to be a template method.

If I go the delegation route, the new class will need to be passed the pertinent metadata as well as the persistable object. I'm tending to go this route, due to my resistance of inheritance (particularly at this stage in the game). Right now, the code is screaming out for some sort of abstraction to represent the metadata; I think that's what I'm going to tackle next.

Share your comment