Monday, August 16, 2004

Recently I've been tending to write these the day after the events described. Hopefully they won't be suffering too much from the lack of attention. It's partly due to needing to help with Luc in the evenings, and also because of my preparations for this year's Noosa.

Friday's problem was tracked down through the extra logging I threw into the exception trace. My first problem was because I hadn't implemented cloning on the ConstraintIs class. As a result, the clone() method from the ConstraintImpl superclass was being called, which converted the constraint into a ConstraintImpl. This meant that the wrong constraint resolution method was being called, which was the basis of the problem.

Adding the clone() method fixed this, but not quite as well as I'd hoped, with a number of failures still occurring in the tests. At this point, I decided that I was running into problems associated with inheriting methods (such as clone) from ConstraintImpl, and so I removed that inheritance, and copied a lot of code from ConstraintImpl to ConstraintIs.

It was while copying the code, that I found the problem. This meant that I could probably go back to inheritance, but AN expressed a preference to split the constraint classes up anyway, so I was happy to leave it. Perhaps an abstract constraint class could be created to have them all inherit from, rather than having them all inherit from ConstraintImpl as I had initially done.

The problem I found was fairly simple. I had implemented clone by cloning all the nodes except the KOWARI_IS node, as this will always be fixed. However, I forgot to anticipate that the constraint could be localized, meaning that the node would be changed to a local node, with only a long integer as an identifier. Once I realised this I changed the code to clone the predicate if it isn't equal to KOWARI_IS, and voila, another couple of tests passed.

The next problem was similar to the cloning issue. SubqueryAnswer was creating a new constraint with its own version of a KOWARI_IS predicate. In one instance the predicate was global, and in the other instance it was local. An existing constructor would work for the former, but the latter needed a new method created which would accept the local node. The "query" package that ConstraintIs lies within does not have access to the database, so this node has to be pre-allocated and passed in, rather than having the ConstraintIs class find the node for itself.

Given that subqueries only needed to allocate new ConstraintIs objects, I create a factory method for these new requirements, and put it directly on the ConstraintIs class.

Blank Nodes
The final two failing tests had me stumped for several hours. They were both in JRDFGraphUnitTest and should not have been impacted by any kind of changes to constraints.

I finally discovered that these tests were expecting that a new blank node allocated for a fresh graph with a predetermined number of statements should have an ID of "_node58". Since I had changed the database to no longer allocate nodes for some of the magic predicates, this number was changed.

I spoke with the culprit, and he confessed to thinking that using hard-coded node IDs was lazy as he wrote the code that did this. I know he'll read this, so hopefully he won't be so lazy next time. :-)

The fix was to put markers into the array that is compared against the blank nodes, and replace those markers with the correct IDs as soon as the blank nodes are allocated. That way any further changes will not affect these tests.

No comments: