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:
public List<T> findMatches(Criteria criteria) {
return new Persister<T>(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<Customer> 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:
package application;
import java.util.*;
import persistence.*;
import sql.*;
import domain.*;
class MockCustomerAccess extends CustomerAccess {
private Persistable customer;
public List<Customer> findMatches(Criteria criteria) {
List<Customer> results = new ArrayList<Customer>();
if (criteria.sqlString().indexOf("'n%'") >= 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:
public void testBrowseCustomers() {
final String id1 = "id1";
final String name1 = "name1";
final String matchingCustomerPattern = "'n%'";
application.setCustomerAccess(new CustomerAccess() {
private Persistable customer;
public List<Customer> findMatches(Criteria criteria) {
List<Customer> results = new ArrayList<Customer>();
if (criteria.sqlString().indexOf(matchingCustomerPattern) >= 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<Customer> 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 SqlGeneratorEnjoy. 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.
code (Note that I did an optimize imports against the entire source base, so there are a few more changes than you might expect.)
February 2004 March 2004 May 2004 September 2004 October 2004 January 2005 February 2005 September 2005 October 2005 November 2005 December 2005 January 2006 February 2006 March 2006 June 2006 August 2006 January 2007 February 2007 March 2007 April 2007 September 2007 October 2007 November 2007 December 2007 January 2008