Re: Visitor pattern vs if-ladder
Philipp wrote:
Hello,
I think about replacing an if-ladder (with instanceof calls) in my
code with a visitor pattern, but the advantages are not really obvious
to me.
Pros:
- cleaner code (although I think this is debatable)
Cons:
- Needs modification of the original interface and classes (to add the
accept(Visitor) method)
- If new subclasses of the acceptor interface are added, both the
Visitor interface and it's implementation needs change (adding a new
visit(MyNewClass c); to the interface). In the ladder implementation,
you only need to change at one point and add an if(...)
- A bit slower than the ladder implementation (see below)
What advantages am I missing? Why is an if ladder seen as code smell
and not the visitor pattern?
About speed:
I compared the speed of execution of an if-ladder with the visitor
pattern for counting different types of Vehicles.
If ladder:
public void count(Vehicle vehicle){
if (vehicle instanceof Car) {
counts[0]++;
} else if (vehicle instanceof Motorbike) {
counts[1]++;
} else if (vehicle instanceof Bike) {
counts[2]++;
[...]
Visitor pattern:
public void visit(Car car) {
counts[0]++;
}
public void visit(Motorbike motorbike) {
counts[1]++;
}
public void visit(Bike bike) {
counts[2]++;
}
[...]
I ran the counting with 8 types of vehicle on 10^8 randomly produced
vehicles 10 times alternatively (each run is ~16 sec, all 20 runs are
in one JVM execution, so warm up should be accounted for). The ladder
implementation is about 10% faster (mean time per run: ladder 16800ms,
visitor 18380).
I'm not saying that this is a big difference or should influence the
decision very much. It's just FYI.
Phil
Maybe consider adding an enum type-token, and using an
EnumMap<Vehical.Type, CountAccumulator>
where:
public class CountAccumulator {
private int count;
public void increment() { ++count; }
public int get() { return count; }
}
I usually avoid enum for type-tokens, but this seems like an appropriate
place to apply them, since you are looking for a very specific set of
"types". It also would reduce the size of your count method/visitor
dramatically:
Map<Vehical.Type, CountAccumulator> count(Collection<Vehical> vehicals){
Map<Vehical.Type, CountAccumulator> counts =
new EnumMap<Vehical.Type, CountAccumulator>(CarType.class);
for (Vehical.Type type: Vehical.Type.values()) {
counts.put(type, new CountAccumulator());
}
for (Vehical vehical: vehicals) {
counts.get(vehical.getType()).increment();
}
return counts;
}
--
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>