10

I am working on a GUI application which generates a configuration file. I have a class hierarchy for the configuration model and I use an object tree of that hierarchy in several different contexts. Currently, I use the Visitor pattern to avoid polluting my model classes with context specific code.

interface IConfigurationElement {
    void acceptVisitor(IConfigurationElementVisitor visitor);
}

In an earlier version I used chains of instanceof conditions instead of the Visitor. Comparing the two approaches I see the following tradeofs.

Visitor

  • It is easier and safer to add new IConfigurationElement. Just add a new declaration to IConfigurationElementVisitor and the compiler generates errors for all visitor implementations. With instanceof chains you have to remember all the places you have to extend with the new configuration element. Basically instanceof violates the DRY principle as it duplicates logic in several places.
  • The visitor pattern is more efficient than a chain of instanceofconditions

instanceof

  • The great advantage of instanceof is its flexibility. For example instanceof allows me to define special solutions for different subsets of IConfigurationElement implementations which need to be handled similarilly in some cases. In contrast, Visitor forces me to implement a method for each implementation class every time.

Is there a common solution for this kind of problem? Can I adapt the Visitor somehow, so I can provide a common solution for some cases?

1
  • I would highly recommend you this blog tutorial which compares the different visitor style (instanceOf etc) and provides very interesting answers, it helped me I hope it woks for you too: apocalisp.wordpress.com/2009/08/21/… Commented Jan 30, 2013 at 9:02

3 Answers 3

1

You could use visitor with instanceOf

interfaces:

interface Visitable {
  void accept(Object visitor);
}
interface AVisitor {
  void visitA(A a);
}
interface BVisitor {
  void visitB(B b);
}
interface CVisitor {
  void visitB(C c);
}

Visitables:

class C implements Visitable {
  public void accept(Object visitor) {
    if (visitor instanceof CVisitor) {
      ((BVisitor)vistor).visitC(this);
    }
  }
}

class B implements Visitable {
  public void accept(Object visitor) {
    if (visitor instanceof BVisitor) {
      ((BVisitor)vistor).visitB(this);
    }
  }
}

class A extends B implements Visitable {
  public void accept(Object visitor) {
    super.accept(visitor);
    if (visitor instanceof AVisitor) {
      ((AVisitor)vistor).visitA(this);
    }
  }
}

Visitors:

class PrintBs implements BVisitor {
  public void visitB(B b) {
    system.out.println(b);
  }
}

class PrintAs implements AVisitor {
  public void visitA(A a) {
    system.out.println(a);
  }
}

class PrintCs implements CVisitor {
  public void visitC(C c) {
    system.out.println(c);
  }
}
class PrintAsAndCs implements CVisitor, AVisitor{
  public void visitA(A a) {
    system.out.println(a);
  }
  public void visitC(C c) {
    system.out.println(c);
  }
}

each class knows only about its related interfaces, so adding new vistors nor visitables requires changing everything in that category (visitor/visitable) (for visitor, it does not require changing anything, for visitable it requires creating new visitor interface, but again, no changing of existing objects).

This way, there is no chain of instanceof tests and visitor for subset does not need to even know about types outside this subset.

Question is what to do with situation where A extends B (and B is Visitable too), in that case, you could just add super.accept(visitor) into accept (thus it would be short chain of instanceof-s, but onlu as long as hierarchy is deep, and it should not be too deep to matter, abd you don't need to write it whole manually).

3
  • If I understand you correctly, you propose to remove IConfigurationElementVisitor and just check for specific visitor types in Visitable. While this is a possible solution I see some drawbacks. First this would remove the stability of Visitor I talked about and second I would pull knowledge about visitors into my Visitable implementations, which to avoid was part of the reason to use Visitor in the first place. Commented Jan 30, 2013 at 8:41
  • Not in Visitable (that should be only simple interface vith accept(Object visitor) ), but in implementations of Visitable. So each Class checks only for its visitor, so it knows only about fact that there exists interface for visiting it. I will modify my answer to make this clear.
    – user470365
    Commented Jan 30, 2013 at 9:32
  • Ok, your approach allows to have visitors which visit only specific subtypes of my structure. This is a nice idea. What i want is a little different (I should probably update my description). I want a compile time guarantee that all elements of my structure where visited but at the same time i want to treat some elements equally and some different. It is like @AndreasScheinert put it. Basically I want pattern matching with a compiler warning if the match is not exhaustive (like Scala's match with case classes). Commented Jan 30, 2013 at 10:14
0

Well, yes, you could. Capture the commonalities of the many configuration elements by assigning roles to them. You may end up with instanceof, but not in a way that violates the DRY principle, but rather as a way around Java's static typing.

class ConfigurationElementA implements IConfigurationElement, ICommittable {
}

class ConfigurationElementB implements IConfigurationElement, IVerifiable {
}

class Visitor {
    void accept(IConfigurationElement element) {
        if (element instanceof ICommittable) {
            // ...
        }

        // Note: not a chain of instanceofs.

        if (element instanceof IVerifiable) {
            // ...
        }
    }
}

In other words, you let the visitor accept configuration elements generically, and act on groups of implementations via roles. Note that you can go as specific as you need by modeling your configuration elements accordingly.

You may recognize here Martin Fowler's RoleInterface pattern.

3
  • 1
    You basically use interfaces to define subsets of IConfigurationElement which can be handled specifically based on set membership. Unfortunately, as soon as you distinguish objects with instanceof your compiler won't be able to help you if you forget to update one of your visitors. Your solution has the benefit that all instanceof operators are embedded in a common type, which helps finding them by searching for the type. Commented Feb 4, 2013 at 8:04
  • My proposal comes from two observations about your use case: Firstly, you want to avoid a proliferation of accept method signatures. For this, I suggested a catch all approach, one that you can adapt to suit your needs, going more or less specific around your IConfigurationElement implementation. Second, you wanted the flexibility of instanceof, presumably (by the first case) because you have common traits in your implementation classes - otherwise, no reason to avoid accept proliferation to begin with. That's where I suggested RoleInterface which uses instanceof differently. Commented Feb 4, 2013 at 18:34
  • (You can still avoid instanceof: Make IConfigurationElement implement IRoleEnabled and have the visitor visit each configuration element as a role enabled item calling visitRoleEnabled, and having visitRoleEnabled in each implementation class call back into the visitor for each of the implemented roles. But with this we're starting to go wild on instanceof when it's really doing just fine.) Commented Feb 4, 2013 at 18:36
0

I can think of some potential solutions:

  1. create a private method in the Visitor implementation, and have multiple visit methods in the Visitor implementation call that private method.

  2. if the above is repeated in many places, you can consider creating an abstract class that implements Visitor, and redirects a subset of visit implementations to a common protected abstract method.

  3. create multiple Visitor interfaces:

    interface AOrB extends IConfigurationElementVisitor {
        <Result> Result accept(AOrBVisitor<Result> visitor);
    
        interface AOrBVisitor<Result> {
            Result visit(A a);
            Result visit(B b);
        }
    }
    
    interface ThisCouldBeOfTypeB extends IConfigurationElementVisitor {
        <Result> Result accept(ThisCouldBeOfTypeBVisitor<Result> visitor);
    
        interface ThisCouldBeOfTypeBVisitor<Result> {
            Result visit(B b);
            Result visit(ThisCouldBeOfTypeB visitable);
        }
    }
    
    class A implements AOrB, ThisCouldBeOfTypeB {...}
    
    class B implements AOrB, ThisCouldBeOfTypeB {...}
    
    class C implements ThisCouldBeOfTypeB {...}
    

I think 3 is what you are looking for. it is 100% static polymorphism, and generates compiler warnings if a type isn't handled. But the only disadvantage I can think of 3 is that maintaining the Visitable* implementations could become complex if there are a lot of different Visitable* interfaces.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.