Having to write an accept method each time specially in larger hierarchies
that statically maps to the right overloading visit counterpart is indeed
error-prone e.g. you just might forget to call visit.
That would be a remarkable thing to forget, given that that call is the
only thing accept has to do. This is accept:
public void accept(Visitor v) {
v.visit(this);
}
So to forget to call visit, you'd have to write this:
public void accept(Visitor v) {
}
Which would be quite a surprising thing to write!
Unless you're thinking of the style of visitor which deals with composite
hierarchies, where elements deal with traversal of their children
(although your own code doesn't do this, so i assume this is not what you
mean), in which case the correct method looks like:
public void accept(Visitor v) {
v.visit(this);
// next bit could be more complicated
for (Element child: getChildren()) {
child.accept(v);
}
}
In which case you might accidentally write:
public void accept(Visitor v) {
// next bit could be more complicated
for (Element child: getChildren()) {
child.accept(v);
}
}
Which would be more understandable, but is still a pretty unlikely error,
i'd say.
It is simply boilerplate code that you will have to write and maintain.
Write, yes, but it'd not something that takes much maintenance, is it?
Attempts to make generic accepts in the Element hierarchy lead to type
casts and really type-unsafe operations.
Agreed - i don't think there's any way around the need to write accept in
each element class.
The goal of an Element hierarchy is to provide implementations for well
defined abstractions and not an accept method that might cover a ghost
requirement.
"Might cover a ghost requirement"! If you're adding visitor support to the
hierarchy, it's because you have a concrete requirement for it. I
certainly don't advocate going round adding visitor support to hierarchies
willy-nilly!
The accept method is counter-intuitive
How so?
and fills only the artificial need for a double-dispatching capability
missing natively in languages like Java.
True. In a language which can do dispatch based on parameter types, you
only need one accept method (although i don't know how that could be
typesafe). But the fact is that java can't, so dismissing it as an
'artificial need' is sophistry.
Besides, in the manual way:
1-. You can not create visitors for Element hierarchies for which you do not
have access to the source code
True. If i couldn't change the hierarchy, i would have to consider an
alternative strategy, and i might well consider one like yours. But when i
can change the hierarchy, the classical visitor pattern is a better
solution.
2-. Adding new Elements in the hierarchy imposes a high maintenance toll in
existing Visitor implementations.
Absolute nonsense! This "high maintenance toll" means adding methods to
deal with the new element types - which you *have to do* to maintain the
correctness of your program! Any way of avoiding this "toll" is simply a
way of writing an incorrect program! That is generally not considered a
good thing!
That people want to enjoy the Burger does not mean that they want to meet
the Cow.
It's not simply the ugliness of the cow that bothers me, it's the fact
that the cow is being raised in unsanitary conditions, and is likely to
give its consumers CJD.
If we go back to design principles and a client-provider contract, I give
you an interface that offers:
- double dispatch so you can attach externalizable concerns rather than
imposse them to a (maybe existing) Element hierarchy.
This is good. It's definitely useful to be able to do this to an existing
hierarchy.
- type safety defined as: no matter what parameter combination you try to
attack this interface with, there will be no type-unsafety translated in
potential for a ClassCastException.
Inadequate. Type safety, in this case, is not just about avoiding
ClassCastExceptions, it's about statically ensuring that all the input
types are dealt with properly. Your code does not do that, and it cannot
be made to do it.
- reasonable performance. Note that the "pretty shocking reflective
strategy" does memoization. The matching methods corresponding to the
second dispatch are cached and this may even be faster than the "manual"
implementation.
It may not involve a reflective lookup, but it still involves a map
lookup. In fact, *three* map lookups, although it could easily be reduced
to one - where you currently do (eliding the details of creating a
visitor):
if (!theLookup.containsKey(myElementClassName)) {
theLookup.put(myElementClassName, createVisitor());
}
if (theLookup.containsKey(myElementClassName)) {
theLookup.get(myElementClassName).visit(anElement);
}
You could do:
IVisitor<E> visitor = theLookup.get(myElementClassName);
if (visitor == null) {
visitor = createVisitor();
if (visitor != null) theLookup.put(myElementClassName, visitor);
}
if (visitor != null) visitor.visit(anElement);
Even if you did that, a map lookup and a reflective invocation is still a
lot slower than two virtual invocations.
It's not just the performance which is shocking, though. There's also the
fact that findVisitorMethod is so unselective - it will pick any method
with the right parameter and return type as a visitor method, which means
that i have to be careful when writing my visitor class not to introduce
any methods which might be mistaken for visitor methods.
A couple of other queries. Firstly, why does VisitorDelegator override
getReturn with an implementation that simply does return
super.getReturn()? That's no different to not overriding it. Secondly, why
does isValidReturn do
aTestMethod.getReturnType().isAssignableFrom(aReturnClass), rather than
the other way round? If the specified return type is, say, String, then
that means that a method which returns Object will be classed as having a
valid return type. And that if the specified return type is Object, then
only methods declared as returning Object will be classed as valid, and
not any subtypes of Object!
Thanks to this thread I am encouraged to write for the next release a
benchmarking test to try to proof exactly this.
Glad to hear it.
Not true. I encourage you to write a test case that uses this Visitor
via its public API and produces a ClassCastException. I might even pay
you for that
I don't claim to be able to produce a ClassCastException, but a test case
that breaks your code is easy. Write a new class:
public class Gearbox implements ICarPart {}
Modify Car to add:
private final Gearbox theGearbox = new Gearbox();
public Gearbox getGearbox() {
return theGearbox;
}
Modify PrintVisitor.visitCar to add, at the end:
visit(aCar.getGearbox());
This will compile and run without error. However, there is no visitor
method which takes a Gearbox, so it won't be visited, and that's a bug.
The idea - unfortunately not mine - comes from scientific research in the
field of Software Engineering. The relevant information can be found here:
http://se.ethz.ch/~meyer/publications/computer/visitor.pdf
But then you need a bit of understanding of Eiffel which I don't expect from
a Biologist?
Wow, an argument from authority and an ad hominem attack in the same
sentence. Impressive. Now see if you can get three fallacies in one
sentence!
Bertrand Meyer is a terribly famous guy, but it doesn't change the fact
that he's quite wrong in this case. Amusingly, he tries to spin the safety
hole that i've pointed out as an advantage - if you're not interested in
visiting a given type, you just don't write a method for it! A fine
example of why academics shouldn't try to give advice on the production of
software.
Nonetheless, if you do have a situation where you want to ignore all types
of element except a select few, that's easily dealt with with the
classical visitor. You just define a class NullVisitor or IgnoringVisitor,
which implements the visitor interface with no-op methods. Visitor classes
which want to be selective extend this, and override the methods for the
types they're interested in. Any other types, including any types added
after the visitor was written, will be ignored.
tom
