views:

130

answers:

3

Is the following java implementation of the visitor pattern using generics, general enough to be useful? (I suppose it is).

Could it be improved in some way? It's important to be easily call-able using anonymous classes. Thanks.

(Example of use):

Vector<Number> numbers = new Vector<Number>();

        numbers.add(new Double(1.2));
        numbers.add(new Float(-1.2));
        numbers.add(new Double(4.8));
        numbers.add(new Float(-3.4));
        numbers.add(new Long(123456));
        numbers.add(new Short("14"));

        For.each(numbers, new Visitor<Number>() {
            public void doIt(Double n) {
                System.out.println("doIt() for double: " + n);
            }
            public void doIt(Float n) {
                System.out.println("doIt() for float: " + n);
            }
            public void doIt(Number n) {
                System.out.println("doIt() for Number: " + n);
            }
        });

        Visitor<Number> visi =  new Visitor<Number>() {
            private StringBuffer  all = new StringBuffer ();
            public void doIt(Number n) {
                System.out.println("doIt() for Number: " + n);
                all.append(n.toString() + " ");
            }
            public Object getResult () {
                return all;
            }
        };

        For.each(numbers, visi);

        System.out.println ("all -> " + visi.getResult());

Definitions:

//............................................
abstract class Visitor<T> {
    public void visit(T n) {
        try {
            this.getClass().getDeclaredMethod("doIt", n.getClass()).invoke(this, n);
        } catch (Exception ex) {
            doIt((T) n);
        }
    }
    public void doIt(T n) {
        System.out.println("doIt() for base " + n);
    }
    public Object getResult() {
        return null;
    }
} // class

//............................................
class For {
    public static <T> void each (Collection<T> c, Visitor<T> f) {
        for (T v : c) {
            f.visit(v);
        }
    } // ()
} // class
+4  A: 

This is not the Visitor Pattern.

Visitor is characterized by the visitee having an accept(Visitor v) method that interacts with an visit method in the visitor taking as parameter the visitee and overloaded for the varying type of the visitee, forming a "double dispatch" mechanism.

Quoting from the "Applicability" section for Visitor in Design Patterns:

Use the Visitor pattern when
  • an object structure contains many classes of objects with differing interfaces, and you want to perform operations on these objects that depend on their concrete classes.
  • many distinct and unrelated operations need to be performed on objects in an object structure, and you want to avoid "polluting" their classes with these operations. Visitor lets you keep related operations together by defining them in one class. When the object structure is shared by many applications, use Visitor to put operations in just those applications that need them.
  • the classes defining the object structure rarely change, but you often want to define new operations over the structure. Changing the object structure classes requires redefining the interface to all visitors, which is potentially costly. If the object structure classes change often, then it's probably better to define the operations in those classes.

So this pattern is for dealing with similar opertaions on objects of multiple types. In your examples the objects you're calling visitors can only deal with one type.

In your answer revising to use reflection to handle multiple types (which by the way would be better done as an edit to the question or as a separate question), you're avoiding creating an accept(Visitor v) method in the visited classes by using reflection, which is to a degree accomplishing the same goal, but somewhat awkwardly. I still would resist calling it an implementation of Visitor.

If code in the style you've written here is useful to you, by all means use it, but please don't call it a Visitor.

This is more like a Strategy Pattern or a Function Object, and if you rename the generic class in a way that reflects that, it's in fact useful, and your usage is similar to common patterns of list handling in functional languages.

What I would likely do with the code from the question is rename your Visitor<T> to Operation<T> and rename your visit(T t) to execute(T t) or apply(T t), thinking of an Operation as a Function without a return value. I have in fact used exactly this in ways similar to what you're doing, and used similar tactics for collection "mapping" using generic Function<Domain, Range> objects. I'm not sure what pattern name actually fits it, but it's not Visitor. It's bringing functional list-comprehension style to an OO world where functions are not naturally first-class objects.

Don Roby
Clearly, I have to accept that I don't understand the need of an accept() method and of that double dispatch mechanism. What's the benefit we get?
cibercitizen1
I correct myself. I see that it is a trick to have the visited elements be examined by a different function depending on its type. But it does not seem very clean to me having to modify the visitees with an accept method. In addition that is not necessary if all elements in the collections are really the same type.
cibercitizen1
@donroby Is the new version now the visitor pattern? (See below).
cibercitizen1
@cibercitizen1 I still wouldn't call this visitor (see my edited answer) but it's certainly closer and sort of deals with the same issue. I think double-dispatch is an essential part of the pattern.
Don Roby
@donroby I do appreciate your answers. Thanks. Nevertheless, I think that if the accept() in the visitor pattern is only for processing each element in a collection in a way depending on its type (first point of your quote), then my code also does that. And, for me, then it is preferable as I don't have to modify the visitees to add them the accept(). And the solution can be awkard (matter of taste?) but it's transparent and clean for the user code.
cibercitizen1
@cibercitizen1 That's fine. There's certainly no requirement that you use the visitor pattern if you don't like it or don't think it fits your situation. I only object to calling something a visitor pattern that really is something else.
Don Roby
A: 

how about

    for(String s : words)
        System.out.println (s.toUpperCase());

    int total = 0;
    for(String s : words)
        total = total + s.length();
    System.out.println (" sum of lengths = " + total);
irreputable
Simpler is better, but I was interested in the visitor pattern. Thanks.
cibercitizen1
A: 

Thanks to the answer of donroby about my initial code not implementing the visitor pattern I came to this new version.

I suppose it now implements the visitor pattern without the need of modifying the visited element with an accept() method. Anyway, it is able to call the right method depending on the element type (I guess that's the mission for the accept()), thanks to reflection.

First, an example of use:

Vector<Number> numbers = new Vector<Number>();

    numbers.add(new Double(1.2));
    numbers.add(new Float(-1.2));
    numbers.add(new Double(4.8));
    numbers.add(new Float(-3.4));
    numbers.add(new Long(123456));
    numbers.add(new Short("14"));

    For.each(numbers, new Visitor<Number>() {
        public void doIt(Double n) {
            System.out.println("doIt() for double: " + n);
        }
        public void doIt(Float n) {
            System.out.println("doIt() for float: " + n);
        }
        public void doIt(Number n) {
            System.out.println("doIt() for Number: " + n);
        }
    });

That produces this output

doIt() for double: 1.2
doIt() for float: -1.2
doIt() for double: 4.8
doIt() for float: -3.4
doIt() for Number: 123456
doIt() for Number: 14

And finally the code

abstract class Visitor<T> {
public void visit(T n) {
    try {
        this.getClass().getDeclaredMethod("doIt", n.getClass()).invoke(this, n);
    } catch (Exception ex) {
        doIt((T) n);
    }
}
public void doIt(T n) {
    System.out.println("doIt() for base " + n);
}
public Object getResult() {
    return null;
}

}

class For {
public static <T> void each (Collection<T> c, Visitor<T> f) {
    for (T v : c) {
        f.visit(v);
    }
} // ()

}

cibercitizen1
Yes this implements the visitor pattern with reflection instead of double dispatch. One problem: the line doIt((T)n) will always result in doIt(Object) being called since the generic type will be erased by the compiler.
josefx
Since you asked how you could improve your code, instead of storing a result within your visitor (your first example), you could use a local variable in each() to store it, hand it to the visitor as parameter and return it at the end of each(). This way you wont have any state in the visitor.
josefx
@josefx perhaps I'm missing something but doIt(Object) is not called (afaik). In my Vector<Number> example doIt(Double) and doIt(Float) are called for doubles and floats. doIt(Number) is called for the other types.
cibercitizen1
@josefx Good hint about the getResult(). I'll try to use it and post the code (not now :-) )
cibercitizen1
@josefx. I think doIt(Object) is not called because although the type of the reference being returned out of the collection is Object, the actual pointed object is still a Double or a Float. Therefore n.getClass() does not return Object but the right type and so, getDeclaredMethod() finds the correct one.
cibercitizen1