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%'") >= 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) >= 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.
Comments
Anonymous December 4, 2005 at 12:20am
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.
Anonymous December 4, 2005 at 12:21am
I forgot to sign a post, so -probably- the previous message is –JeffBay, unless its been haxx0red.
Jeff Langr 12/05/2005 at 09:39am
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.