Friday, August 13, 2004

Visitor Pattern
After getting through some email this morning, my first task was to clean up my modifications (ie. clean up code that is calling methods I've yet to write) and make sure it all works. During this period I started wondering about the use of the visitor pattern for resolving constraints, and if this would work for the subclass of a constraint.

Binding the function to call is done at compile time, and my concern was that ConstraintIs might not be passed into the visit() method correctly. ConstraintIs extends the ConstraintImpl class. The problem happens because of an overloaded method (in the ConstraintResolver class) which takes a ConstraintImpl in one implementation, and a ConstraintIs in the other. If code in the resolver class takes a ConstraintImpl object and passes this object into the overloaded method, then the compiler will make a selection of the method to call based on the type of that parameter. The compiler has to choose one method or the other, and it has no idea that I might be passing in a ConstraintIs, so it chooses the method which takes a ConstraintImpl.

The only way that Java can select a method at runtime is when it is calling a method on a class reference. The reference to the class provides the indirection necessary to find the method. For instance, this is how I can call toString() on any class derived from Object. Using overloaded methods on another class, and choosing to pass in differing objects at run time provides no mechanism for Java to work out where to go.

Fortunately, my concerns were unfounded and demonstrated my lack of understanding of the use of the visitor pattern. This pattern is specifically designed to circumvent the very problem I was concerned about. By calling accept on the destination class (either ConstraintImpl or ConstraintIs) then the compiler is able to go through the reference to the correct class. The implementation of the accept method is a trivial call to visit(this) on the originating class, providing the correct type (via the this parameter) for the compiler to statically bind.

AM took me through much of this, and once I "got it" I realised that I hadn't implemented the trivial accept method on ConstraintIs. This meant that it would inherit the accept method from ConstraintImpl which was not what I wanted. It's always nice to find bugs before you run them.

I was still curious about the use of the visitor pattern, as I didn't see why the constraint resolver would want to have a different method to resolve each different type of constraint, and not call a method on a constraint to do the work for it. This would remove the method binding problem as well. It turns out that the forthcoming resolver interface will want to provide different mechanisms for resolution, meaning that we need numerous classes able to do the constraint resolution, on identical constraint objects. Sure, it can be done without the visitor pattern, but then the constraint objects will need to know far too much about the internal workings of the classes which are to do the resolving.

After getting everything tidied up and compiling, I had a go at testing the new setup. This didn't work, and it was because of the tucana:is predicate.

Logging during tests is particularly bad, as most logs get swallowed up, with just an exception string being saved in the output. So I added a lot more information (including a stack trace) to the exceptions I was seeing, and this helped me track down the problem to the clone method on ConstraintImpl. Since I had ConstraintIs extending ConstraintImpl it was picking up the inherited clone method, and hence lost all type meaning. Reimplementing clone at the ConstraintIs level changed the error I was seeing, but didn't fix the problem. I'll be working on that on Monday.

No comments: