malvasia bianca

I can now handle books with more than two authors. Yay. Nice to be able to spend ten or fifteen minutes making a change that actually affects what books I can handle instead of spending months making a change whose effects nobody else can see.

Not that I’m done with the behind-the-scenes changes yet – I still have some amount of cleanup to do, getting rid of no-longer-necessary code, so I’m trying to combine each new feature with a bit of cleanup. And there was one unpleasant surprise when doing that: I was shuffling responsibilities between acceptance tests, when all of a sudden one of them stopped working. And stopped working in a quite strange way – it was updating a row in a database, and some of the columns got the new value while others had their old values. I don’t quite see how this could be any sort of obvious bug in my code – tests are pinning down the behavior pretty tightly, and if anything goes wrong an exception should be thrown that would make it up to the top. And my code should be pretty deterministic, yet the problem in question magically cured itself a few minutes later.

So I’m pretty disturbed. I have no idea if it’s related to my other current mystery, the random hangs I mentioned earlier. Just to be on the safe side, I read through the JDBC Javadoc a bit more, to look for places where I’m not releasing resources; I’m now calling close() on my java.lang.sql.Connection objects, just in case. (I was already calling it on my ResultSet objects, though that is one place where I could imagine there being gaps in my unit test coverage.) But this latest problem doesn’t feel like a resource leak to me…

Much of the future cleanups are pretty obvious, but there is one place where I have a question. Right now, I have an abstract interface Collection with two implementations, MemoryCollection and SqlCollection. The former is the original implementation, from the days when the list of books was represented by Java code; the latter is the implementation I’m using now. So I could delete the former, were it not for the fact that it’s being used in unit tests in a fair number of places.

So: do I keep it around, or do I retrofit the unit tests? It’s not clear to me that keeping it around is going to pose that much of a burden; and if it becomes a burden at some point, I can always retrofit the tests then. (Which will, admittedly, take a while, which is one reason to think about starting on it now.) So if keeping it around is the easiest way to write unit tests, then that’s what I’ll do.

One aspect of keeping it around is this: to create, say, a new Book object using MemoryCollection, I call a factory function, passing it the title and author, and then call various write accessor to update other attributes that I feel like setting. (E.g. the ISBN, or when I read it.) Which is an okay way to do things; it’s certainly better than creating an SQL row by hand. But now that I have this CLI tool to create and edit entities, I could use its interface instead, which is pretty easy to use: something like

add book author Fred title “This Book” isbn 1-234-56789-x

is easy enough to write, probably even easier than calling various write accessors.

There are a couple of issues with that. For one thing, I can’t just pass in a single string, I have to pass in an array of Strings, and it’s a little more verbose than I’d like to create that on the fly in Java. Maybe I could work around that by writing a little function to split the single string into an array, though – that would be easy enough to do, and might save me a fair amount of test-writing time. The other issue is that the above isn’t actually the syntax that I use for the current SQL interface – instead of passing in the name of the author, I have to pass in the entity ID of the author, something like “author 3” instead of “author Fred”. So there’s some fiddly counting to do, and I lose type safety, neither of which thrills me. (It’s a problem with the CLI tool as well.)

Another possible issue is that I was worrying about the tests being too incestuous, not being tied to the external realities of the database format enough. But, thinking about it more, I’m not worried about that any more: my tests for the Editor object (which writes out the SQL database) really do test the SQL that is generated. So it’s fine for me if my tests for Book, for example, whose main job is to generate HTML, aren’t so closely tied to the details of the SQL: as long as they accept whatever Editor outputs, it should be fine. In fact, it’s arguably superior to have that linkage be enforced in the unit tests. (This is all being enforced in the acceptance tests, too.)

I’ve got enough other cleanups to do that I don’t have to decide this one way or another for a few more weeks. I’m leaning towards getting rid of MemoryCollection and using SqlCollection exclusively, but I could still change my mind.