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 values = new HashMap();
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 values = new HashMap();
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.
