Anonymous or real class definition when using visitor pattern?

When you use the Visitor pattern and you need to get a variable inside visitor method, how to you proceed ?

I see two approaches. The first one uses anonymous class :

// need a wrapper to get the result (which is just a String)
final StringBuild result = new StringBuilder();
final String concat = "Hello ";

myObject.accept(new MyVisitor() {

    @Override
    public void visit(ClassA o)
    {
        // this concatenation is expected here because I've simplified the example
        // normally, the concat var is a complex object (like hashtable) 
        // used to create the result variable 
        // (I know that concatenation using StringBuilder is ugly, but this is an example !)
        result.append(concat + "A");
    }

    @Override
    public void visit(ClassB o)
    {
        result.append(concat + "B");
    }
});

System.out.println(result.toString());

Pros & Cons :

• Pros : you do not need to create a class file for this little behavior
• Cons : I don't like the "final" keyword in this case : the anonymous class is less readable because it calls external variables and you need to use a wrapper to get the requested value (because with the keyword final, you can't reassign the variable)

Another way to do it is to do an external visitor class :

public class MyVisitor
{
    private String result;
    private String concat;

    public MyVisitor(String concat)
    {
        this.concat = concat;
    }

    @Override
    public void visit(ClassA o)
    {
        result = concat + "A";
    }

    @Override
    public void visit(ClassB o)
    {
        result = concat + "B";
    }

    public String getResult()
    {
        return result;
    }
}

MyVisitor visitor = new MyVisitor("Hello ");
myObject.accept(visitor);
System.out.println(visitor.getResult());

Pros & Cons :

• Pros : all variab开发者_JS百科les are defined in a clean scope, you don't need a wrapper to encapsulate the requested variable
• Cons : need an external file, the getResult() method must be call after the accept method, this is quite ugly because you need to know the function call order to correctly use the visitor

You, what's your approach in this case ? Preferred method ? another idea ?

Well, both approaches are valid and imo, it really depends on whether you would like to reuse the code or not. By the way, your last 'Con' point is not totally valid since you do not need an 'external file' to declare a class. It might very well be an inner class...

That said, the way I use Visitors is like this:

public interface IVisitor<T extends Object> {
    public T visit(ClassA element) throws VisitorException;
    public T visit(ClassB element) throws VisitorException;
}

public interface IVisitable {
    public <T extends Object> T accept(final IVisitor<T> visitor) throws VisitorException;
}

public class MyVisitor implements IVisitor<String> {
    private String concat;

    public MyVisitor(String concat) {
        this.concat = concat;
    }

    public String visit(ClassA classA) throws VisitorException {
        return this.concat + "A";
    }

    public String visit(ClassB classB) throws VisitorException {
        return this.concat + "B";
    }
}

public class ClassA implements IVisitable {
    public <T> T accept(final IVisitor<T> visitor) throws VisitorException {
        return visitor.visit(this);
    }
}

public class ClassB implements IVisitable {
    public <T> T accept(final IVisitor<T> visitor) throws VisitorException {
        return visitor.visit(this);
    }
}

// no return value needed?
public class MyOtherVisitor implements IVisitor<Void> {
    public Void visit(ClassA classA) throws VisitorException {
        return null;
    }

    public Void visit(ClassB classB) throws VisitorException {
        return null;
    }
}

That way, the visited objects are ignorant of what the visitor wants to do with them, yet they do return whatever the visitor wants to return. Your visitor can even 'fail' by throwing an exception.

I wrote the first version of this a few years ago and so far, it has worked for me in every case.

Disclaimer: I just hacked this together, quality (or even compilation) not guaranteed. But you get the idea... :)

I do not see an interface being implemented in your second example, but I believe it is there. I would add to your interface (or make a sub interface) that has a getResult() method on it.

That would help both example 1 and 2. You would not need a wrapper in 1, because you can define the getResult() method to return the result you want. In example 2, because getResult() is a part of your interface, there is no function that you 'need to know'.

My preference would be to create a new class, unless each variation of the class is only going to be used once. In which case I would inline it anonymously.

From the perspective of a cleaner design, the second approach is preferrable for the same exact reasons you've already stated.

In a normal TDD cycle I would start off with an anonymous class and refactored it out a bit later. However, if the visitor would only be needed in that one place and its complexity would match that of what you've provided in the example (i.e. not complex), I would have left it hanging and refactor to a separate class later if needed (e.g. another use case appeared, complexity of the visitor/surrounding class increased).

I would recommend using the second approach. Having the visitor in its full fledged class also serves the purpose of documentation and clean code. I do not agree with the cons that you have mentioned with the approach. Say you have an arraylist, and you don't add any element to it and do a get, surely you will get a null but that doesn't mean that it is necessarily wrong.

One of the points of the visitor pattern is to allow for multiple visitor types. If you create an anonymous class, you are kind of breaking the pattern.

You should change your accept method to be

public void accept(Visitor visitor) {
   visitor.visit(this);
}

Since you pass this into the visitor, this being the object that is visited, the visitor can access the object's property according to the standard access rules.