I use the Visitor pattern all the time
The Visitor pattern is often maligned as too complex. I disagree. I use the visitor pattern all the time. I find it to be a very nice way to separate things that change for different reasons.For example, let's say I've got a list of employees and I want to create the following report of hourly employees:
| Emp# | Name | QTD-hours | QTD-pay |
| 1429 | Bob Martin | 432 | $22,576 |
| 1532 | James Grenning | 490 | $28,776 |
| ... | |||
I could simply add a method to the Employee class that produced that employee's corresponding line on the report.
public class Employee {
public abstract String reportQtdHoursAndPay();
}
public class HourlyEmployee extends Employee {
public String reportQtdHoursAndPay() {
//generate the line for this hourly employee
}
}
public class SalariedEmployee extends Employee {
public String reportQtdHoursAndPay() {} // do nothing
}
Then I could generate the report by iterating over all the Employee objects and calling reportQtdHoursAndPay().
The problem with this is that we have coupled the format and content of a report to the Employee object. This violates the Single Reponsibility Principle because it causes Employee to be changed every time the format or content of the report changes, or when any new reports are added.
To fix this, I can use the Visitor pattern as follows:
public class Employee {
public abstract void accept(EmployeeVisitor v);
}
public class HourlyEmployee extends Employee {
public void accept(EmployeeVisitor v) {
v.visit(this);
}
}
interface EmployeeVisitor {
public void visit(HourlyEmployee he);
public void visit(SalariedEmployee se);
}
public class QtdHoursAndPayReport implements EmployeeVisitor {
public void visit(HourlyEmployee he) {
// generate the line of the report.
}
public void visit(SalariedEmployee se) {} // do nothing
}
Now I can generate the report by creating a QtdHoursAndPayReport[?] instance, and then iterating over all the Employee instances and calling accept and passing in the visitor:
QtdHoursAndPayReport v = new QtdHoursAndPayReport();
for (...) // each employee e
{
e.accept(v);
}
This solves the Single Responsibility Principle by moving the report generation into a new object. Moreover, each new report will be added to a new derivative of EmployeeVisitor[?].
Append your comments below:
I use an HTML parser that comes with a visitor interface. You write your own visitors that magically get calls for every node in the doc. Far better than writing code to iterate the tree over and over and over. I got so happy I wrote this.
Nice clear example. It helped me. Thank you.
One web-page issue: For me, the main page window was not refreshing properly. I had to scroll vertically past and then back, to see all text. To reproduce with MS IE 6.0 on XP, resize the IE window diagonaly to numerous sizes and notice that the upperleft text disappears.
One web-page issue: For me, the main page window was not refreshing properly. I had to scroll vertically past and then back, to see all text. To reproduce with MS IE 6.0 on XP, resize the IE window diagonaly to numerous sizes and notice that the upperleft text disappears.
We use the visitor in a lot of our code, but it seems to me there’s a question of bounding, how much do you put in the visitor and how much do you put into the class being visited?
We have a quad-tree of “entities” that we need to draw, so we use a visitor to traverse the tree to see who can be drawn, and then ask the entity to draw itself. But I could see how you could say that the draw method might be more appropriate to be in the visitor. Hmm…
We have a quad-tree of “entities” that we need to draw, so we use a visitor to traverse the tree to see who can be drawn, and then ask the entity to draw itself. But I could see how you could say that the draw method might be more appropriate to be in the visitor. Hmm…
I think I understand this example reasonably well.
I have a concern about what to do if I need to write a report that is to be run every week, but that has slightly different display fields or format.
From the given design, it appears that I must create an interface called “EmployeeWeeklyVisitor[?]” and add an abstract method named, for example, acceptWeekly.
This seems like too much work. Is there another solution?
I think the key to understanding this problem is your statement: I could simply add a method to the Employee class that produced that employee’s corresponding line on the report.
Why not add this method in the QtdReport[?] class? This class already has a method to get all the Employees from the data source. So, all values needed to calculate the report are already there. Just have a method that knows how to use the Employee data to generate the report line.
For example, in the QtdReport[?] class, we could have a method getReportLine(Employee e). When a weekly report is desired, just add another class and method to handle it.
In this particular case, I do not see the point of using the Visitor pattern.
I have a concern about what to do if I need to write a report that is to be run every week, but that has slightly different display fields or format.
From the given design, it appears that I must create an interface called “EmployeeWeeklyVisitor[?]” and add an abstract method named, for example, acceptWeekly.
This seems like too much work. Is there another solution?
I think the key to understanding this problem is your statement: I could simply add a method to the Employee class that produced that employee’s corresponding line on the report.
Why not add this method in the QtdReport[?] class? This class already has a method to get all the Employees from the data source. So, all values needed to calculate the report are already there. Just have a method that knows how to use the Employee data to generate the report line.
For example, in the QtdReport[?] class, we could have a method getReportLine(Employee e). When a weekly report is desired, just add another class and method to handle it.
In this particular case, I do not see the point of using the Visitor pattern.
I had a similar feeling about visitor at first: Why the double dispatch (which IS more complex than a single dispatch) when I can just loop through the collection of employees? The answer (at least the one that satisfies me) lies in the fact that not all of the employees are treated the same way in each report. A TimeCardReport[?] doesn't apply to salaried employees, so that method now needs to have a switch (on the type of employee) in it. Adding new types of employees may require a change to that switch. Multiply this out by n reports w/ x switches over y types of employees and you are exposing yourself to an awful lot of change of existing code. The key to the double dispatch is that the second half of it is polymorphic - the runtime dispatches to the correct method (in this example for the correct type of employee). So you don't have to go back and adjust your switches as the shape of the system changes. You just add NEW methods and classes.
You'll have to pry Visitor away from my cold dead fingers. I too use it all the time and consider it the most versatile, most powerful pattern in my arsenal. Its a big hammer and eveyrthing looks like a nail.
One of the keys to keeping software robust is to make sure that layers at the bottom that are reusable have no knowledge of the layers that use them.
For example, the Logger classes have no knowledge of any other class that uses them. This lets the class designers focus on the Logging functionality.
I see Reports as a tool/application that uses the Employee class, among others. As such, the Employee class should not know anything about the Reporting application that uses it. The Visitor pattern violates this by forcing the Employee to be aware of **all** the Reports classes that use it. This means that everytime we want a slightly different report, we'll have to modify the Employee class to be able to accept that type of Visitor.
Imagine, for a moment that we are using an Enployee class that has been designed by a software vendor that has not given us the source code, or that is declared as final. How, then, do we implement the Visitor pattern?
At least in this case, I see the Visitor pattern as more of a hindrance that an asset.
In general, the Visitor pattern seems to violate the fundamental programming practice that a "core" class/module should have no knowledge of its users. This leads to frequent, avoidable code changes.
Therefore, I believe that the Visitor pattern usefulness is debatable, at best.
For example, the Logger classes have no knowledge of any other class that uses them. This lets the class designers focus on the Logging functionality.
I see Reports as a tool/application that uses the Employee class, among others. As such, the Employee class should not know anything about the Reporting application that uses it. The Visitor pattern violates this by forcing the Employee to be aware of **all** the Reports classes that use it. This means that everytime we want a slightly different report, we'll have to modify the Employee class to be able to accept that type of Visitor.
Imagine, for a moment that we are using an Enployee class that has been designed by a software vendor that has not given us the source code, or that is declared as final. How, then, do we implement the Visitor pattern?
At least in this case, I see the Visitor pattern as more of a hindrance that an asset.
In general, the Visitor pattern seems to violate the fundamental programming practice that a "core" class/module should have no knowledge of its users. This leads to frequent, avoidable code changes.
Therefore, I believe that the Visitor pattern usefulness is debatable, at best.
I have never used the Visitor pattern, but the example seems to illustrate why you would NOT have to change the Employee class every time you needed a slightly, or even dramatically, different kind of report.
The Employee class does not depend on any of the Report classes at all, but instead depends on the EmployeeVisitor[?] abstraction. The fact that each Report class is an implementation of the EmployeeVistor[?] abstraction allows the Employee class to use the Report classes polymorphically. It will never have to change because a new report is created.
As a matter of fact, this scheme would allow you to add dramatically different functionality altogether. For example, if you needed to notify all employees that their paycheck has been issued, you could have a PaymentNotification[?] class that implemented the EmployeeVisitor[?] abstraction. Again, the Employee class does not have to change at all to work with this new type of Visitor.
If I'm not mistaken, this is the Dependency Inversion Principle in action because Employee depends on neither Report nor PaymentNotification[?] implementation details, but instead they all depend on the EmployeeVisitor[?] abstraction.
The Employee class does not depend on any of the Report classes at all, but instead depends on the EmployeeVisitor[?] abstraction. The fact that each Report class is an implementation of the EmployeeVistor[?] abstraction allows the Employee class to use the Report classes polymorphically. It will never have to change because a new report is created.
As a matter of fact, this scheme would allow you to add dramatically different functionality altogether. For example, if you needed to notify all employees that their paycheck has been issued, you could have a PaymentNotification[?] class that implemented the EmployeeVisitor[?] abstraction. Again, the Employee class does not have to change at all to work with this new type of Visitor.
If I'm not mistaken, this is the Dependency Inversion Principle in action because Employee depends on neither Report nor PaymentNotification[?] implementation details, but instead they all depend on the EmployeeVisitor[?] abstraction.
I love the visitor pattern, ever since I ran into JJTree (http://lml.ls.fi.upm.es/manuales/javaccdocs/). I find it a usefull pattern for seperating "the men from the boys" so to speak...
Well, please note that the "accept" method of Employee has to be added after the fact, that is, after the Employee class has been designed and tested. This class delegates to the "visit" method of the EmployeeVisitor[?] class. Also, all sub-classes of Employee that are needed for the report must implement the "accept" method.
[Feb 11, 2005, Roger Glover] I have comments on this entry added out of time sequence to answer specific questions and to correct flawed analysis that are not addressed elsewhere in the discussion. Comments will be denoted [2005-02-11,RG].
What exactly does the EmployeeVisitor.[?]visit method do? Does it get the names, age, etc. of the Employee? If so, there are presumably getters in the Employee class that get this information. Why can the QuarterlyReport[?] class not directly access the Employee class and get the required information? Why do we need the EmployeeVisitor[?] class at all? or the "accept" method in Employee class?
[2005-02-11,RG] 1) EmployeeVisitor.[?]visit doesn't do anything. It is an interface method. 2) No, but visit methods in classes that implement EmployeeVisitor[?] probably do those sorts of things. 3) See the "dynamic dispatch" info in this discussion below (look for Feb 8, 2005). 4) It is the interface that all the report classes will need to implement to get "dynamic dispatch" separation of Employee subclasses. 5) same answer.
Is there a design that is simpler by not requiring these classes? If so, are there any extra limitations of this approach compared to the use of the Visitor pattern? An answer to these questions will determine whether to use the Visitor pattern or go for the simpler method.
[2005-02-11,RG] 1) If method dispatch of overloads was determined by dynamic type (actual type of the instantiated object) rather than by static type (declared type of the variable handle) there would not be such a need. However, there are very few OOPLs that are implemented that way. 2) Ravi has an example below (look for Feb 11) that uses method reflection, but I believe that approach has some significant drawbacks (see my comments below, after Ravi's code).
In the original article, Uncle Bob says "I could simply add a method to the Employee class that produced that employee's corresponding line on the report."
And that is where I have a problem. I would not dream of putting this code in the Employee class to begin with. It would be in the QuarterlyReport[?] class using Employee's getters.
[2005-02-11,RG] Ravi and Uncle Bob seem to be in violent agreement.
After using the Visitor pattern, where is this code? Why, in the QuarterlyReport[?] class itself! So , what do we gain by adding the Visitor pattern here? We end up with the invokers of getters in the QuarterlyReport[?] class. This could be done simply without using the Visitor pattern at all.
[2005-02-11,RG] This analysis does not take the "dynamic dispatch" problem into account.
Now let us assume that a new report is to be published involving Employees that requires significantly different data from about Employees. Or suppose, that we want data for SalariedEmployees[?] too. What needs to be done?
We must now modify the EmployeeVisitor.[?]visit(SalariedEmployee[?] se) method. But wait, this will break the existing reports! So what must be done?
[2005-02-11,RG] This analysis is incorrect. We must create a new class (call it NewReport[?]) that implements the EmployeeVisitor[?] interface. Then we must create overriding methods for NewReport.[?]visit(HourlyEmployee[?]) and NewReport.[?]visit(SalariedEmployee[?]). Existing reports (themselves implementers of EmployeeVisitor[?]) will be completely unaffected.
Well, we must add a new EmployeeVisitor[?] type class that has the correct behaviour, have the visit method implemented for the various sub-classes of Employees. *** AND *** we have to change the Employee class to accept a different type of visitor!
As a result of using the Visitor pattern, I need to change the Employee class from time to time.
[2005-02-11,RG] This analysis is also incorrect. Once the abstract method Employee.accept(EmployeeVisitor[?]) has been added and the implemented method accept(EmployeeVisitor[?]) has been added to each Employee subclass, none of the Employee classes need be modified ever again for the purpose of supporting reports. Different reporting needs will be handled by report classes that implement the EmployeeVisitor[?] interface.
Is that good design? I am not convinced. Yes, I agree with Erik that understanding when to use the Visitor pattern does separate the men from the boys!
Moreover, I feel that the original design presented for Employee and the sub-classing of HourlyEmployee[?] and SalariedEmployee[?] itself can be improved. One key is the observation that Hourly and Salaried are "roles" that Employees play. An Employee can be an HourlyEmployee[?] at some point in time, and then become a SalariedEmployee[?].
I would have created an EmployeeRole[?] class and then assigned each Employee zero, one or more non-overlapping (in time) roles. Thus, my design would have involved composition rather than inheritance. As is obvious to the person on the street, Employee and Hourly/Salaried Employee do not form a natural hierarchy. Hence, I would look for alternatives before implementing such an artificial hierarchy. Maybe then, it may turn out that the Visitor pattern is not even required.
[2005-02-11,RG] I agree wholeheartedly with Ravi here. The design of the Employee objects in this sort of as a static heirarchy is flawed. But certainly we can posit the existance of legitimate hierarchies whose reporting we may wish to handle this way.
I am not at all convinced that having the Employee class know about the existence of the QuarterlyReport[?] class is worth violating fundamental software design concepts, whether you call the violation the "Dependency Inversion Principle" or something else.
[2005-02-11,RG] This would be a valid complaint if the Employee class in fact knew about the QuarterlyReport[?] class. Instead, all it knows is that some EmployeeVisitor[?], for purposes unknown, may at some point wish to resolve it's own behavior (denoted by the "visit" method) based on an Employee object's dynamic type.
[Feb 11, 2005, Roger Glover] I have comments on this entry added out of time sequence to answer specific questions and to correct flawed analysis that are not addressed elsewhere in the discussion. Comments will be denoted [2005-02-11,RG].
What exactly does the EmployeeVisitor.[?]visit method do? Does it get the names, age, etc. of the Employee? If so, there are presumably getters in the Employee class that get this information. Why can the QuarterlyReport[?] class not directly access the Employee class and get the required information? Why do we need the EmployeeVisitor[?] class at all? or the "accept" method in Employee class?
[2005-02-11,RG] 1) EmployeeVisitor.[?]visit doesn't do anything. It is an interface method. 2) No, but visit methods in classes that implement EmployeeVisitor[?] probably do those sorts of things. 3) See the "dynamic dispatch" info in this discussion below (look for Feb 8, 2005). 4) It is the interface that all the report classes will need to implement to get "dynamic dispatch" separation of Employee subclasses. 5) same answer.
Is there a design that is simpler by not requiring these classes? If so, are there any extra limitations of this approach compared to the use of the Visitor pattern? An answer to these questions will determine whether to use the Visitor pattern or go for the simpler method.
[2005-02-11,RG] 1) If method dispatch of overloads was determined by dynamic type (actual type of the instantiated object) rather than by static type (declared type of the variable handle) there would not be such a need. However, there are very few OOPLs that are implemented that way. 2) Ravi has an example below (look for Feb 11) that uses method reflection, but I believe that approach has some significant drawbacks (see my comments below, after Ravi's code).
In the original article, Uncle Bob says "I could simply add a method to the Employee class that produced that employee's corresponding line on the report."
And that is where I have a problem. I would not dream of putting this code in the Employee class to begin with. It would be in the QuarterlyReport[?] class using Employee's getters.
[2005-02-11,RG] Ravi and Uncle Bob seem to be in violent agreement.
After using the Visitor pattern, where is this code? Why, in the QuarterlyReport[?] class itself! So , what do we gain by adding the Visitor pattern here? We end up with the invokers of getters in the QuarterlyReport[?] class. This could be done simply without using the Visitor pattern at all.
[2005-02-11,RG] This analysis does not take the "dynamic dispatch" problem into account.
Now let us assume that a new report is to be published involving Employees that requires significantly different data from about Employees. Or suppose, that we want data for SalariedEmployees[?] too. What needs to be done?
We must now modify the EmployeeVisitor.[?]visit(SalariedEmployee[?] se) method. But wait, this will break the existing reports! So what must be done?
[2005-02-11,RG] This analysis is incorrect. We must create a new class (call it NewReport[?]) that implements the EmployeeVisitor[?] interface. Then we must create overriding methods for NewReport.[?]visit(HourlyEmployee[?]) and NewReport.[?]visit(SalariedEmployee[?]). Existing reports (themselves implementers of EmployeeVisitor[?]) will be completely unaffected.
Well, we must add a new EmployeeVisitor[?] type class that has the correct behaviour, have the visit method implemented for the various sub-classes of Employees. *** AND *** we have to change the Employee class to accept a different type of visitor!
As a result of using the Visitor pattern, I need to change the Employee class from time to time.
[2005-02-11,RG] This analysis is also incorrect. Once the abstract method Employee.accept(EmployeeVisitor[?]) has been added and the implemented method accept(EmployeeVisitor[?]) has been added to each Employee subclass, none of the Employee classes need be modified ever again for the purpose of supporting reports. Different reporting needs will be handled by report classes that implement the EmployeeVisitor[?] interface.
Is that good design? I am not convinced. Yes, I agree with Erik that understanding when to use the Visitor pattern does separate the men from the boys!
Moreover, I feel that the original design presented for Employee and the sub-classing of HourlyEmployee[?] and SalariedEmployee[?] itself can be improved. One key is the observation that Hourly and Salaried are "roles" that Employees play. An Employee can be an HourlyEmployee[?] at some point in time, and then become a SalariedEmployee[?].
I would have created an EmployeeRole[?] class and then assigned each Employee zero, one or more non-overlapping (in time) roles. Thus, my design would have involved composition rather than inheritance. As is obvious to the person on the street, Employee and Hourly/Salaried Employee do not form a natural hierarchy. Hence, I would look for alternatives before implementing such an artificial hierarchy. Maybe then, it may turn out that the Visitor pattern is not even required.
[2005-02-11,RG] I agree wholeheartedly with Ravi here. The design of the Employee objects in this sort of as a static heirarchy is flawed. But certainly we can posit the existance of legitimate hierarchies whose reporting we may wish to handle this way.
I am not at all convinced that having the Employee class know about the existence of the QuarterlyReport[?] class is worth violating fundamental software design concepts, whether you call the violation the "Dependency Inversion Principle" or something else.
[2005-02-11,RG] This would be a valid complaint if the Employee class in fact knew about the QuarterlyReport[?] class. Instead, all it knows is that some EmployeeVisitor[?], for purposes unknown, may at some point wish to resolve it's own behavior (denoted by the "visit" method) based on an Employee object's dynamic type.
I'll just agree that the visitor pattern is extremely useful. In fact, with my last contract, I put most complex object relationships into logical trees and used the visitor pattern throughout. I usually finished coding (including tests) within 2 - 3 days and spent the rest of the week looking busy.
Timothy, it is good that you like and use the Visitor pattern.
Could you (or anybody else) please answer the questions that I have raised in my previous post? What benefit does it give you over the simpler option, especially since the simpler option does not require you to change the Employee class ever (no matter what the requested change), uses fewer classes and requires less work to make changes?
Using the Visitor pattern, the EmployeeVisitor[?] class and the Employee class must be changed if we neeed to produce a report that asks for SalariedEmployee[?] information. The current implementation excludes SalariedEmployees[?] from the report. Is there any way to include this new requirement without changing the Employee/EmployeeVisitor[?] classes. I, for one, do not see how it can be done.
I would like to know what advantages the Visitor pattern has in this scenario.
Could you (or anybody else) please answer the questions that I have raised in my previous post? What benefit does it give you over the simpler option, especially since the simpler option does not require you to change the Employee class ever (no matter what the requested change), uses fewer classes and requires less work to make changes?
Using the Visitor pattern, the EmployeeVisitor[?] class and the Employee class must be changed if we neeed to produce a report that asks for SalariedEmployee[?] information. The current implementation excludes SalariedEmployees[?] from the report. Is there any way to include this new requirement without changing the Employee/EmployeeVisitor[?] classes. I, for one, do not see how it can be done.
I would like to know what advantages the Visitor pattern has in this scenario.
To me, the main advantage of the Visitor pattern is increased compile time safety.
First, the Visitor reduces the number of type casts.
Second, and more important, whenever I add a new type of Employee - and therefore add a corresponding visit method to the Visitor interface, the compiler tells me where I need to implement that new method.
Well, there are no explicit type casts in the alternative I proposed, either. Hence, the first point is irrelevant.
As to the second point, whenever we add new functionality, we add new tests in any case. The absence of the method to handle the new type of Employee in the report would become obvious at the very first new test, no matter what code/pattern we use. Therefore, the fact that the compiler points out the absence of the method is of little practical significance.
I have still to get an answer to the question of a different report requiring different data or different rules to decide what type of Employee line gets printed from the Employee class. It seems that we'll need to create a new Visitor interface to that. And modify the Employee class to delegate to the new type of Visitor.
Why is this design superior to the simpler one without Visitor? Especially when the simpler design does not require any change to the Employee class, ever? Eagerly awaiting answers.
My reservations about the Visitor pattern are expressed much more eloquently in the following article:
http://nice.sourceforge.net/visitor.html
And another IBM Research site points to the limitations of the Visitor pattern:
http://www.research.ibm.com/sop/sopcvisp.htm
As to the second point, whenever we add new functionality, we add new tests in any case. The absence of the method to handle the new type of Employee in the report would become obvious at the very first new test, no matter what code/pattern we use. Therefore, the fact that the compiler points out the absence of the method is of little practical significance.
I have still to get an answer to the question of a different report requiring different data or different rules to decide what type of Employee line gets printed from the Employee class. It seems that we'll need to create a new Visitor interface to that. And modify the Employee class to delegate to the new type of Visitor.
Why is this design superior to the simpler one without Visitor? Especially when the simpler design does not require any change to the Employee class, ever? Eagerly awaiting answers.
My reservations about the Visitor pattern are expressed much more eloquently in the following article:
http://nice.sourceforge.net/visitor.html
And another IBM Research site points to the limitations of the Visitor pattern:
http://www.research.ibm.com/sop/sopcvisp.htm
In response to Nov 30, 2004, I may not have been clear about what I meant when I indicated that the Visitor prevents you from having to change the Employee class. I don't consider implementing the Visitor pattern as changing the Employee class. The Visitor implementation is a part of the Employee class.
For the record, I think I am with you on the fact that the issues raised in the example could be resolved with a simpler solution than the Visitor pattern. I was simply pointing out the fact that based on the given example, there is no need to change anything in the Employee class because you have added a new visitor. There need not be another accept method. You just create a new visitor, which in the example, appears to represent the content and format of a report.
In this example, Visitor allows you to separate report data retrieval from report data content and formatting without duplicating the report data retrieval logic. This is possible, I think, because it follows the principle of dependency inversion (essentially, decoupling a high-level object from any lower-level objects it uses through an abstraction). Dependency Inversion appears to me to be a layering technique, which you indicated was fundamental to building robust software.
I think a better example would involve a hierarchy of objects that must be dealt with in a similar manner. Consider the contents of a file system. You may want to output the contents to a console window, a web page, a tree-view control, or a speaker. The variation is not in traversing the hierarchy of folders and files to retrieve the contents, but rather, in producing the contents as output. Visitor could allow you to produce the various forms of output without duplicating the traversal and retrieval logic.
Perhaps, even still, a simpler alternative exists for this scenario. I think Uncle Bob should clarify his use of Visitor. Under what circumstances has he found it a simpler design than other reasonable alternatives? Only he can say; and he should be able to say quite a bit since he says he uses it all the time.
Incidentally, I didn't find the articles you referenced to be of much help. First, they do not say Visitor is unnecessary, but rather, that it has advantages and disadvantages. Object technology in and of itself has advantages and disadvantages, too. Secondly, they appear to promote the use of language extensions (multi-methods, subject-oriented programming) to alleviate the need to use Visitor where it may be the appropriate design. I think Visitor applies to most, if not all, object-oriented languages. I don't plan on switching to Nice or waiting for subject-oriented programming extensions to my language of choice in order to solve my design problems.
For the record, I think I am with you on the fact that the issues raised in the example could be resolved with a simpler solution than the Visitor pattern. I was simply pointing out the fact that based on the given example, there is no need to change anything in the Employee class because you have added a new visitor. There need not be another accept method. You just create a new visitor, which in the example, appears to represent the content and format of a report.
In this example, Visitor allows you to separate report data retrieval from report data content and formatting without duplicating the report data retrieval logic. This is possible, I think, because it follows the principle of dependency inversion (essentially, decoupling a high-level object from any lower-level objects it uses through an abstraction). Dependency Inversion appears to me to be a layering technique, which you indicated was fundamental to building robust software.
I think a better example would involve a hierarchy of objects that must be dealt with in a similar manner. Consider the contents of a file system. You may want to output the contents to a console window, a web page, a tree-view control, or a speaker. The variation is not in traversing the hierarchy of folders and files to retrieve the contents, but rather, in producing the contents as output. Visitor could allow you to produce the various forms of output without duplicating the traversal and retrieval logic.
Perhaps, even still, a simpler alternative exists for this scenario. I think Uncle Bob should clarify his use of Visitor. Under what circumstances has he found it a simpler design than other reasonable alternatives? Only he can say; and he should be able to say quite a bit since he says he uses it all the time.
Incidentally, I didn't find the articles you referenced to be of much help. First, they do not say Visitor is unnecessary, but rather, that it has advantages and disadvantages. Object technology in and of itself has advantages and disadvantages, too. Secondly, they appear to promote the use of language extensions (multi-methods, subject-oriented programming) to alleviate the need to use Visitor where it may be the appropriate design. I think Visitor applies to most, if not all, object-oriented languages. I don't plan on switching to Nice or waiting for subject-oriented programming extensions to my language of choice in order to solve my design problems.
If I add a method to my Employee class that is not strictly required for defining the Employee class, then I suspect that I am not doing something right. Why is the accept method a "proper" part of the Employee class? What does it do? All it does is provide double dispatch method to enable access to Employee attributes via getters. As such, it is totally un-necessary. The getters themselves will suffice.
Once again, I ask: If I were designing an Employee class why would I add a Visitor abstraction? What purpose does it serve that can not be served by already existing methods in a simpler manner?
Is there any advantage over the simpler approach of focussing on the QuarterlyReport[?] class? If <i> Visitor allows you to separate report data retrieval from report data content and formatting without duplicating the report data retrieval logic, </i> even then the Visitor is not required. Your QuarterlyReport[?] Class could invoke a method that retrieves all employees and then calls the appopriate formatting code. You thus get your separation of data retrieval code and formatting with the added advantage that the data retrieval code can be made more sophisticated by allowing filters, and thus be more reusable.
In my opinion, one of the worst things that happened to the OO community was the publication of the Design Pattern book by the Gang of Four. Now, everybody wants to apply patterns to their code, often not realizing that simpler solutions are available. The overuse of the Visitor pattern seems to be typical.
Once again, I ask: If I were designing an Employee class why would I add a Visitor abstraction? What purpose does it serve that can not be served by already existing methods in a simpler manner?
Is there any advantage over the simpler approach of focussing on the QuarterlyReport[?] class? If <i> Visitor allows you to separate report data retrieval from report data content and formatting without duplicating the report data retrieval logic, </i> even then the Visitor is not required. Your QuarterlyReport[?] Class could invoke a method that retrieves all employees and then calls the appopriate formatting code. You thus get your separation of data retrieval code and formatting with the added advantage that the data retrieval code can be made more sophisticated by allowing filters, and thus be more reusable.
In my opinion, one of the worst things that happened to the OO community was the publication of the Design Pattern book by the Gang of Four. Now, everybody wants to apply patterns to their code, often not realizing that simpler solutions are available. The overuse of the Visitor pattern seems to be typical.
This isn't exactly a timely response. I apologize for that. In response to whether or not the accept() method is a "proper" method, I would say it depends entirely on how you intend to use the class. You can argue that there's no need for accept(), but it doesn't make any sense to say accept() isn't part of the Employee class when it's right there in the class definition.
I agreed with you that the example provided was a poor one because there is a simpler solution to this problem. I have personally never used the Visitor pattern. I have never come across a situation where it seemed necessary. But I would not categorically dismiss it as being useless or unnecessary in all cases just because my experience has not presented me with a case that needed it. I guess I've chosen to give the folks who used it prior to its popularization credit for having thought through the practical alternatives before deciding on this particular design.
As I said before, Uncle Bob should provide us with at least one scenario where conventional design approaches fall short, but Visitor fits the bill.
I agreed with you that the example provided was a poor one because there is a simpler solution to this problem. I have personally never used the Visitor pattern. I have never come across a situation where it seemed necessary. But I would not categorically dismiss it as being useless or unnecessary in all cases just because my experience has not presented me with a case that needed it. I guess I've chosen to give the folks who used it prior to its popularization credit for having thought through the practical alternatives before deciding on this particular design.
As I said before, Uncle Bob should provide us with at least one scenario where conventional design approaches fall short, but Visitor fits the bill.
Uncle Bob, could we please have an example of a real Visitor pattern? Like a small simple problem with a good design that does not use the Visitor Pattern? And then the same problem solved using the Visitor Pattern? So that the advantages are there for all to see.
Reading the Visitor Pattern in Design Patterns book (Gang of Four Book) leaves me unconvinced.
Reading the Visitor Pattern in Design Patterns book (Gang of Four Book) leaves me unconvinced.
I think the problem here is the Uncle Bob is focusing on the SingleReponsibiltyPrincple[?], but we don't understand what will happen if it is violated. I'll try...
The "bad" code would require that we modify each Employee type for every new report. I think we can agree this is bad. Ravi suggests we can keep Employee "clean" and just put all the reporting behavior in various report objects. But if the reports want to vary their behavior by Employee type, which is the reason Uncle Bob had the report method on Employee, I smell duplicate switch/case-like code (perhaps using "instanceof") coming on. You could probably reduce the that down to one case statement with some cleverness/inheritance.
I think that most OO practitioners would agree that case statements based on object types is the strongest smell of all. If you want behavior to vary by type of some object, replace that case-like code with polymorphism. So how do we get behavior to vary by Employee type without actually putting that behavior in Employee? Visitor does that. You get the "layering" Ravi wants (SingleReponsibilityPrinciple[?] to Uncle Bob,) and polymorphic behavior. It's not magic that combining these two things requires two indirections, which we call "double-dispatch."
Contrary to Ravi's suggestion, Visitor allows the creation of new reports (new EmployeeVisitor[?] types) without needing to modify Employee or EmployeeVisitor[?]. Each report can vary behavior by Employee type. The other edge of this sword, as Ravi has discovered, is that adding new Employee types (e.g. TemporaryEmployee[?]) requires adding a new method to EmployeeVisitor[?], and therefore requires modifying each report. This is the fundemental trade-off of Visitor, as explained in Design Patterns (GoF[?]).
The EmployeeRole[?] suggestion is good one from a design standpoint, though I would likely use a different name for it, but it doesn't reduce the desire for or effectiveness of Visitor. We want to be able to create new reports without having to modify EmployeeRole[?] (or Employee,) and reports want to vary their behavior by the EmployeeRole[?] without having to resort to case-like constructs. I'd have to write the code to see, but I think adding it to the example complicates things without clarifying the issue.
Hope that helps.
(The wiki idea is clever, but it's annoying you can't see the original article while commenting. The first time I wrote this I botched it because I had to work from (apparently bad) memory.)
The "bad" code would require that we modify each Employee type for every new report. I think we can agree this is bad. Ravi suggests we can keep Employee "clean" and just put all the reporting behavior in various report objects. But if the reports want to vary their behavior by Employee type, which is the reason Uncle Bob had the report method on Employee, I smell duplicate switch/case-like code (perhaps using "instanceof") coming on. You could probably reduce the that down to one case statement with some cleverness/inheritance.
I think that most OO practitioners would agree that case statements based on object types is the strongest smell of all. If you want behavior to vary by type of some object, replace that case-like code with polymorphism. So how do we get behavior to vary by Employee type without actually putting that behavior in Employee? Visitor does that. You get the "layering" Ravi wants (SingleReponsibilityPrinciple[?] to Uncle Bob,) and polymorphic behavior. It's not magic that combining these two things requires two indirections, which we call "double-dispatch."
Contrary to Ravi's suggestion, Visitor allows the creation of new reports (new EmployeeVisitor[?] types) without needing to modify Employee or EmployeeVisitor[?]. Each report can vary behavior by Employee type. The other edge of this sword, as Ravi has discovered, is that adding new Employee types (e.g. TemporaryEmployee[?]) requires adding a new method to EmployeeVisitor[?], and therefore requires modifying each report. This is the fundemental trade-off of Visitor, as explained in Design Patterns (GoF[?]).
The EmployeeRole[?] suggestion is good one from a design standpoint, though I would likely use a different name for it, but it doesn't reduce the desire for or effectiveness of Visitor. We want to be able to create new reports without having to modify EmployeeRole[?] (or Employee,) and reports want to vary their behavior by the EmployeeRole[?] without having to resort to case-like constructs. I'd have to write the code to see, but I think adding it to the example complicates things without clarifying the issue.
Hope that helps.
(The wiki idea is clever, but it's annoying you can't see the original article while commenting. The first time I wrote this I botched it because I had to work from (apparently bad) memory.)
It is frustrating to realize that nobody seems to understand what I am saying. Hans Ridder's assumptions about my presumed approach are full of inaccuracies. So, in a final attempt to clarify this, I'll give the code from the original article and the suggested replacement. Hopefully, at the end of the exercise, we'll be able to see that the suggested approch requires less code, has none of the drawbacks of the Visitor pattern, and at the same time results in better code that is easily extensible.
And Hans, please do note that there are no case/switch statements or if/then constructs.
Just because a "thought leader" or the "Design Patterns" book suggests the use of a pattern does not mean that we stop thinking. No matter who the advice comes from it is our duty to ensure that it makes sense for us.
Those design patterns guys, are much smarter than you, paying attention to what they have to say, doesn't make you stop thinking, it makes you think harder. An OO software developer ignoring design patterns, is akin to a crank wannabe scientist ignoring established research, you're simply wasting your own time badly reinventing the wheel.
(Ravi) Thank you for your kind advice. I did state earlier that I have gone through the design patterns book and am not impressed with many of the patterns shown there. A lot of the patterns arise because of the limitations of the languages used. The Strategy pattern, for example, would not be needed in a lnaguage like Python that has first class methods.
Just because I disagree with the authors of Design Patterns does not mean that I am like a crank scientist. All I am saying that because a pttern exists in the Design Patterns books, does not imply that it is the right tool for the job. There may be other alternative solutions that are simpler.
If everybody always agreed with "authority figures" all the time, we'd probably believe in the flat earth theory. It is my right, nay, responsibility, as a thinking practitioner to evaluate what is presented to me and make my own judgment based on what I know and understand.
As for the Design Patterns authors being very smart, well, ask the Lisp crowd about these patterns. Do visit Peter Norvig's web site for his take on these patterns and why 16 of 23 are unnecessary in LISP because they are directly supported by the language. Or visit Paul Graham's web site to learn about programming in general. Or the Sussman and Abelson's book "Structure and Interpretation of Computer programs" available free on the web site (http://mitpress.mit.edu/sicp/full-text/book/book.html).
The book will definitely open your eyes and show you how restrictive the popular languages are.
Only if you misjudge what design patterns are, those patterns are workarounds for the lack of expressiveness of OO languages. I have read Structure and Interpretation of Computer Programs, studied Lisp, Smalltalk, Ruby, Python, etc... But I don't see design patterns as what should be, only what can be in the weak languages that are popular and that we get paid to work in. Those patterns are fantastic, in their given context, mainstream OO languages. Would first class functions, closures, and macro's make them irrelevant, absolutely, but those languages are outside of the context for the GOF design patterns. You need to judge the patterns book, inside its intended context, not in the context of more advanced languages.
Here's the original code repeated for convenience.
public class Employee {
public abstract void accept(EmployeeVisitor[?] v);
}
public class HourlyEmployee[?] extends Employee {
public void accept(EmployeeVisitor[?] v) {
v.visit(this);
}
}
interface EmployeeVisitor[?] {
public void visit(HourlyEmployee[?] he);
public void visit(SalariedEmployee[?] se);
}
public class QtdHoursAndPayReport[?] implements EmployeeVisitor[?] {
public void visit(HourlyEmployee[?] he) {
// generate the line of the report.
}
public void visit(SalariedEmployee[?] se) {} // do nothing
}
Here's the suggested code.
public class Employee {
}
public class HourlyEmployee[?] extends Employee {
}
public class QtdHoursAndPayReport[?] {
public void printReport(){
for (each Employee e) //pseudo-code to loop through all employees
{
printReportLine(e); //Automatically finds the correct method based on employee type
}
public void printReportLine(HourlyEmployee[?] he) {
// generate the line of the report.
}
public void printReportLine(SalariedEmployee[?] se) {} // do nothing
}
Compare the two.
No extra code in Employee class, or any of its sub-classes.
No need for an extra interface.
The intent of the QtdHoursAndPayReport[?] is much clearer from the code.
Now answer my question: What has the Visitor pattern provided us that this code has not?
And Hans, please do note that there are no case/switch statements or if/then constructs.
Just because a "thought leader" or the "Design Patterns" book suggests the use of a pattern does not mean that we stop thinking. No matter who the advice comes from it is our duty to ensure that it makes sense for us.
Those design patterns guys, are much smarter than you, paying attention to what they have to say, doesn't make you stop thinking, it makes you think harder. An OO software developer ignoring design patterns, is akin to a crank wannabe scientist ignoring established research, you're simply wasting your own time badly reinventing the wheel.
(Ravi) Thank you for your kind advice. I did state earlier that I have gone through the design patterns book and am not impressed with many of the patterns shown there. A lot of the patterns arise because of the limitations of the languages used. The Strategy pattern, for example, would not be needed in a lnaguage like Python that has first class methods.
Just because I disagree with the authors of Design Patterns does not mean that I am like a crank scientist. All I am saying that because a pttern exists in the Design Patterns books, does not imply that it is the right tool for the job. There may be other alternative solutions that are simpler.
If everybody always agreed with "authority figures" all the time, we'd probably believe in the flat earth theory. It is my right, nay, responsibility, as a thinking practitioner to evaluate what is presented to me and make my own judgment based on what I know and understand.
As for the Design Patterns authors being very smart, well, ask the Lisp crowd about these patterns. Do visit Peter Norvig's web site for his take on these patterns and why 16 of 23 are unnecessary in LISP because they are directly supported by the language. Or visit Paul Graham's web site to learn about programming in general. Or the Sussman and Abelson's book "Structure and Interpretation of Computer programs" available free on the web site (http://mitpress.mit.edu/sicp/full-text/book/book.html).
The book will definitely open your eyes and show you how restrictive the popular languages are.
Only if you misjudge what design patterns are, those patterns are workarounds for the lack of expressiveness of OO languages. I have read Structure and Interpretation of Computer Programs, studied Lisp, Smalltalk, Ruby, Python, etc... But I don't see design patterns as what should be, only what can be in the weak languages that are popular and that we get paid to work in. Those patterns are fantastic, in their given context, mainstream OO languages. Would first class functions, closures, and macro's make them irrelevant, absolutely, but those languages are outside of the context for the GOF design patterns. You need to judge the patterns book, inside its intended context, not in the context of more advanced languages.
Here's the original code repeated for convenience.
public class Employee {
public abstract void accept(EmployeeVisitor[?] v);
}
public class HourlyEmployee[?] extends Employee {
public void accept(EmployeeVisitor[?] v) {
v.visit(this);
}
}
interface EmployeeVisitor[?] {
public void visit(HourlyEmployee[?] he);
public void visit(SalariedEmployee[?] se);
}
public class QtdHoursAndPayReport[?] implements EmployeeVisitor[?] {
public void visit(HourlyEmployee[?] he) {
// generate the line of the report.
}
public void visit(SalariedEmployee[?] se) {} // do nothing
}
Here's the suggested code.
public class Employee {
}
public class HourlyEmployee[?] extends Employee {
}
public class QtdHoursAndPayReport[?] {
public void printReport(){
for (each Employee e) //pseudo-code to loop through all employees
{
printReportLine(e); //Automatically finds the correct method based on employee type
}
public void printReportLine(HourlyEmployee[?] he) {
// generate the line of the report.
}
public void printReportLine(SalariedEmployee[?] se) {} // do nothing
}
Compare the two.
No extra code in Employee class, or any of its sub-classes.
No need for an extra interface.
The intent of the QtdHoursAndPayReport[?] is much clearer from the code.
Now answer my question: What has the Visitor pattern provided us that this code has not?
Visitor actually works, go try and compile your code, then ask again.
Ravi,
Your solution does not work. At least not the way you expect. Your "over loaded" methods will not be properly called. The calls are resolved at compile time not at runtime, at least in Java and I think most other OO languages. The compile time type of the object is Employee not Hourly or Salaried employee. The only way to get your solution to work is to code it with if/elses that check the object type. A fundamental tenet of good OO is to eliminate branching. Visitor eliminates branching on object type. That is what makes it so good.
Smalltalk would be the most notable exception to to Rick's "calls are resolved at compile time" rule. Smalltalk is "dynamically typed" to the extent of not even having the concept of a static (compile-time) type, or for that matter the concept of a compiler :). A good friend of mine, a converted Smalltalk programmer, once bemoaned the Visitor pattern as a "kludge" required to overcome the "design flaw" of static typing in Java/C++/C# (among many others). Reluctantly, I must agree that the whole "calls are resolved at compile time" rule runs rather contrary to object-oriented intuition; programmers at *all* levels of experience make the mistake Ravi made. However, things being what they are, Rick is absolutely right. The Visitor pattern is a far better solution to this shortcoming than the naive approach using fragile chains of "instanceof" conditions.
that the calls are resolved at compile time, it's that most OO languages only do dynamic dispatch on the first argument to a method, the hidden this/self reference, Smalltalk included. When you need to dispatch on more than one type to resolve which method to run, you either use a language with multiple dispatch ... genericFunction(arg1, arg2), or you resort to double dispatch with the visitor pattern arg1.acceptVisit(arg2), arg2.visit(arg1), an obvious, but quite useful hack to dispatch on both types. Ravi mistakenly seems to think multiple dispatch is a common feature, and goes for the obvious solution, great, we all want that too, if only it were so!
I talked to my friend; it turns out he learned object tech using CLOS before he learned Smalltalk. CLOS really does have completely dynamic dispatch; it gets around the compile-time argument overloading problem by not having the concept of arguments. I mistakenly believed his static dispatch complaints were based on his Smalltalk background, hence my mistaken assumption about Smalltalk.
As for the mechanism for dynamic dispatch in C++, in all implementations I have seen it is based on each object containing a pointer to a class metaobject which contains an array of pointers to function. Only the array index is determined at compile time, based on function signature (name and arguments for overload). The rest is based on the type of the invoking object as instantiated at run time. I would guess (and it is only a guess) that C# and Java use a similar system.
As for the mechanism for dynamic dispatch in C++, in all implementations I have seen it is based on each object containing a pointer to a class metaobject which contains an array of pointers to function. Only the array index is determined at compile time, based on function signature (name and arguments for overload). The rest is based on the type of the invoking object as instantiated at run time. I would guess (and it is only a guess) that C# and Java use a similar system.
Actually I think it is a matter of calling overloaded vs overriden methods. I made a mistake in my previous comment (now corrected).
In the original article, there is some code that does the same thing hinted at by my comment:
for (each Employee e) //pseudo-code to loop through all employees
The original article's code is:
QtdHoursAndPayReport[?] v = new QtdHoursAndPayReport[?]();
for (...) // each employee e
{
e.accept(v);
}
At this point, both in the original article code and the version that I've shown, "e" will be of the appropriate type.
If not, neither the original article's code nor mine can work.
Wrong, both your code and the original at this point will pass employee as the base Employee type, which makes your code fail.
What code determines the type of Employee, e, in the original pattern? My alternative code that loops through the employees will use the same logic used in the missing code.
Determining the runtime type of employee can only happen by the subclass calling back the visitor and passing the this pointer, this is the second dispatch that makes the overloaded methods pick the correct one, and this is what you're solution is mission, the second dispatch.
public class HourlyEmployee[?] extends Employee {
public void accept(EmployeeVisitor[?] v) {
v.visit(this);
}
}
I believe this gets rid of the criticism of double dispatch and my lack of understanding and my naive approach.
The overridden methods will most certainly be called when the correct sub-class of Employee is selected in the loop. Else, neither the original code nor my suggestion will work.
You are wrong, and you still don't understand visitor. Rather than talking about it, fire up your compiler and do the necessary work to understand why you are wrong. Your solution won't work in Java, nor CSharp.
Oops! Something seems to have gone wrong when I saved this message. Hope it gets fixed soon.
(Ravi)
Ravi, have you actually implemented a working version of your solution? It will not work, at least not in Java. I made a mistake (now corrected) in my previous comment. Your solution uses method overloading not overriding. That is the critical difference. With Visitor it is the target object that calls an overridden method in the caller. This call is alwasy correctly disptached on the type of "this". In your code the caller attempts to call an overloaded method on itself. In your code the object always has the type Employee when the call is made, it will never be Hourly or Salaried.
Ravi, please listen to Rick. You truly don't understand. Your approach will not work (as a simple 10-minutes-to-code-and-run test will prove). By the way, I was not calling your approach naive. Once a programmer breaks through the conceptual wall you are about to break through, her first thought is typically to write some sort of linked chain of conditionals to cover all the types of Employees. That would be naive. If the programmer realizes the fragility of this naive approach, she usually looks around for a different solution that avoids chains of conditionals. She would then discover the Visitor pattern or some other similar method of accomplishing multiple dynamic dispatch. Here is a link to an academic presentation that discusses the dispatch issue more thoroughly; it mentions the Visitor pattern about halfway through:
http://www.cis.ksu.edu/Department/Seminars/leavens-mm-talk.pdf
http://www.cis.ksu.edu/Department/Seminars/leavens-mm-talk.pdf
It has been a very intersting discussion for me.
(Please scroll up to see my reponse to an earlier comment by somebody else dated Feb 3.)
While I understand that my proposed solution will not work, as stated, in Java, it may work in dynamically typed languages.
Again, this does not imply that I do not understand the Visitor pattern, just that I was unaware of the limitations imposed by the rigid compile-time checking imposed by statically types languages like Java. (Have I got enough buzzwords in? :-) )
Minor correction, it has nothing to do with manifest typed langauges, most dynamic languages also lack multiple dispatch, your solution would not work in python, ruby, smalltalk, javascript, etc. It'd work great in CLOS, but CLOS has multiple dispatch, the issue is single and multiple dispatch, not static and dynamic languages.
That said, even in Java, I would like to be able to write something like I suggested:
So would we, and various flavors of Java, Nice for example, could do this, but they aren't mainstream either.
public void printReport(){
for (each Employee e) //pseudo-code to loop through all employees
{
printReportLine(e); //Automatically finds the correct method based on employee type
}
This has the advantage that it is closer to the problem domain, one of the so called advantages of Object Oriented programming. It does not require me to create extra classes; the Employee class and its sub-classes do not have any knowledge of a "Visitor"; since it is not the core intent behind the creation of the Employee class. I consider it bad design when the Employee class has to become aware of a Visitor class totally unrelated to its purpose. That violates the Single Responsibility Principle, does it not, Uncle Bob?
The question that now arises is: Can we do what I want without resorting to switch or if/then constructs?
I believe that the answer is YES! It can be done.
The key is to realize that at run time, the JVM knows the exact sub-class of Employee "e" in the line:
printReportLine(e);
Hence, we can write a bit of code that uses introspection (reflection) to execute the method dynamically. Something simple such as: (excluding the try catch exception handlers)
public class QtdReport[?]
{
QtdReport[?](){}
private void printReportLine(HourlyEmployee[?] he)
{ system.out.println("Hourly!"); }
private void printReportLine(SalariedEmployee[?] e){} // do nothing
public void printReport()
{
QtdReport[?] report = new QtdReport[?]();
Employee[] emps = new Employee[2];
emps[0] = new HourlyEmployee[?]();
emps[1] = new SalariedEmployee[?]();
for (Employee e: emps) //Java 5 syntax
// ignore try catch blocks for simplicity of presentation
Method m = this.getClass().getDeclaredMethod("printReportLine", e.getClass());
m.invoke(report, e); //Java 5 usage
}
}
Although this has a line more than the original suggestion, it has many advantages. All the code for printing the report is confined to this class. This does not entail changing other clsses and violating fundamental software development principles.
Plus, it is closer to the intent of what we are trying to achieve.
Neither the Visitor pattern nor the if-then or switch blocks option is acceptable to me. I'd prefer the overhead of reflection since it does not violate good design principles.
Te Visitor pattern is simply a kludge necessitated by the limitations of statically typed languages that force one to write unnecessary code and violate other good design principles.
(Please scroll up to see my reponse to an earlier comment by somebody else dated Feb 3.)
While I understand that my proposed solution will not work, as stated, in Java, it may work in dynamically typed languages.
Again, this does not imply that I do not understand the Visitor pattern, just that I was unaware of the limitations imposed by the rigid compile-time checking imposed by statically types languages like Java. (Have I got enough buzzwords in? :-) )
Minor correction, it has nothing to do with manifest typed langauges, most dynamic languages also lack multiple dispatch, your solution would not work in python, ruby, smalltalk, javascript, etc. It'd work great in CLOS, but CLOS has multiple dispatch, the issue is single and multiple dispatch, not static and dynamic languages.
That said, even in Java, I would like to be able to write something like I suggested:
So would we, and various flavors of Java, Nice for example, could do this, but they aren't mainstream either.
public void printReport(){
for (each Employee e) //pseudo-code to loop through all employees
{
printReportLine(e); //Automatically finds the correct method based on employee type
}
This has the advantage that it is closer to the problem domain, one of the so called advantages of Object Oriented programming. It does not require me to create extra classes; the Employee class and its sub-classes do not have any knowledge of a "Visitor"; since it is not the core intent behind the creation of the Employee class. I consider it bad design when the Employee class has to become aware of a Visitor class totally unrelated to its purpose. That violates the Single Responsibility Principle, does it not, Uncle Bob?
The question that now arises is: Can we do what I want without resorting to switch or if/then constructs?
I believe that the answer is YES! It can be done.
The key is to realize that at run time, the JVM knows the exact sub-class of Employee "e" in the line:
printReportLine(e);
Hence, we can write a bit of code that uses introspection (reflection) to execute the method dynamically. Something simple such as: (excluding the try catch exception handlers)
public class QtdReport[?]
{
QtdReport[?](){}
private void printReportLine(HourlyEmployee[?] he)
{ system.out.println("Hourly!"); }
private void printReportLine(SalariedEmployee[?] e){} // do nothing
public void printReport()
{
QtdReport[?] report = new QtdReport[?]();
Employee[] emps = new Employee[2];
emps[0] = new HourlyEmployee[?]();
emps[1] = new SalariedEmployee[?]();
for (Employee e: emps) //Java 5 syntax
// ignore try catch blocks for simplicity of presentation
Method m = this.getClass().getDeclaredMethod("printReportLine", e.getClass());
m.invoke(report, e); //Java 5 usage
}
}
Although this has a line more than the original suggestion, it has many advantages. All the code for printing the report is confined to this class. This does not entail changing other clsses and violating fundamental software development principles.
Plus, it is closer to the intent of what we are trying to achieve.
Neither the Visitor pattern nor the if-then or switch blocks option is acceptable to me. I'd prefer the overhead of reflection since it does not violate good design principles.
Te Visitor pattern is simply a kludge necessitated by the limitations of statically typed languages that force one to write unnecessary code and violate other good design principles.
the visitor pattern. It is a hack, designed to get around lack of multiple dispatch, without resorting to reflection. You seem to be under the impression that we want visitor, or prefer it over simply having the language dispatch correctly for us, as in CLOS, and you couldn't be more wrong. Anyone who's ever used visitor, would absolutely love it, if they could just write a simple for loop and let the language do the work for us, but this is a hack for lesser languages, we know it, you know it, so accept it for what it is, a nice hack for certain situations, in weak languages, nothing more.
But don't just hop into a situation tossing out code that won't even work, and act like we're ignorant for not wanting to do it right, at least take the time to compile it and realize it's your understanding of Java, flawed as it may be, that created the confusion. Actually, of single and multiple dispatch, since this won't work in most languages. Most languages aren't CLOS, and don't have multiple dispatch and generic functions.
But don't just hop into a situation tossing out code that won't even work, and act like we're ignorant for not wanting to do it right, at least take the time to compile it and realize it's your understanding of Java, flawed as it may be, that created the confusion. Actually, of single and multiple dispatch, since this won't work in most languages. Most languages aren't CLOS, and don't have multiple dispatch and generic functions.
Ravi, in this case I find reflection to be a much bigger kludge than the Visitor pattern. To begin with, your use of method reflection breaks encapsulation, in that the Method class must access the private printReportLine methods from QtdReport[?]. To perform this black magic you must add several incantations not shown in your code: You must mark the method object "accessible", an operation with security implications, not guaranteed to work. You must be prepared to handle 4 different non-runtime exceptions (mandatory) and 2 different runtime exceptions (optional) and an error(!). I know you mentioned that you had excluded exception handling, but surely it is fair for me to document how much extra exception handling you excluded. I think it is safe to say that this is bit more work than "a line more than the original". It is true that if you make the printReportLine methods public (as you would to implement the Visitor pattern), you would not have to mark the method "accessible". However, you would still have all the exception handling overhead I mentioned, because at compile time the Method class does not know the difference. Because of the well-known performance implications, because of the security implications, because of the corollary portability implications, I consider method reflection a fairly serious feature, not to be added to code lightly.
I readily agree that the Visitor pattern is a kludge. In fact, I used the word "kludge" before you did. But it is a low-risk, low-overhead kludge. Compare that with method reflection, which is a medium-risk, high-overhead kludge. Sure, Visitor represents a kludgy design choice, but reflection represents a kludgy architectural choice. In my world the latter is a more serious mistake than the former. I guess if you are already using a good deal of method reflection in your code, I can see taking your approach. But I would not choose this situation as my first use of method reflection.
I readily agree that the Visitor pattern is a kludge. In fact, I used the word "kludge" before you did. But it is a low-risk, low-overhead kludge. Compare that with method reflection, which is a medium-risk, high-overhead kludge. Sure, Visitor represents a kludgy design choice, but reflection represents a kludgy architectural choice. In my world the latter is a more serious mistake than the former. I guess if you are already using a good deal of method reflection in your code, I can see taking your approach. But I would not choose this situation as my first use of method reflection.
Actually, I use reflection a bit. In some web applications, I use reflection to invoke beans's methods. In this particular case, I would probably have a single method in a class outside the QtdReport[?] class, say something like a DynamicMethod[?] class. The line in the loop could be as simple as
DynamicMethod.[?]invoke(this, "printReportLine", e);
(I hate Java's checked exceptions that cause your code to be littered with try catch clauses.)
The error handling would be encapsulated in the class DynamicMethod[?]. The printReport method would actually be closer to the intent. Also, the DynamicMethod[?] would be reusable in other places where a visitor type need arises. I use a lot of small tools like these to program at a higher level of abstraction where I focus on the problem domain, and not on kludges like the Visitor Pattern. To me, the ability to convey intent is a very important feature of software.
Given Java's (and other statically typed languages') limitations, reflection is a necessity. All frameworks (like Tomcat) use reflection heavily. I do use quite a bit of reflection in my work.
Therefore, I do not feel that reflection is an architectural kludge. In most serious applications, you will see the use of reflection.
The use of the Visitor pattern, requiring the creation of several classes, and modifying existing classes, violates some very fundamental design principles, the Single Responsibility Principle, for one. It also leads to code scattered in unnatural places. Thus, the Visitor Pattern is a "cardinal sin" while reflection, at worst, is a "venal sin."
DynamicMethod.[?]invoke(this, "printReportLine", e);
(I hate Java's checked exceptions that cause your code to be littered with try catch clauses.)
The error handling would be encapsulated in the class DynamicMethod[?]. The printReport method would actually be closer to the intent. Also, the DynamicMethod[?] would be reusable in other places where a visitor type need arises. I use a lot of small tools like these to program at a higher level of abstraction where I focus on the problem domain, and not on kludges like the Visitor Pattern. To me, the ability to convey intent is a very important feature of software.
Given Java's (and other statically typed languages') limitations, reflection is a necessity. All frameworks (like Tomcat) use reflection heavily. I do use quite a bit of reflection in my work.
Therefore, I do not feel that reflection is an architectural kludge. In most serious applications, you will see the use of reflection.
The use of the Visitor pattern, requiring the creation of several classes, and modifying existing classes, violates some very fundamental design principles, the Single Responsibility Principle, for one. It also leads to code scattered in unnatural places. Thus, the Visitor Pattern is a "cardinal sin" while reflection, at worst, is a "venal sin."
And people wonder why Java code is so slow. I fear we will just have to agree to disagree on this point, Ravi.
I want to be able to identify DIPs in a java program and eliminate them
The Visitor pattern* pays off when you add operations to a class hierarchy more often than types. If you add operations more often than you add types you might as well just modify the classes to add the required operations.
But what about when you must frequently add operations *and* types?
* and pattern matching in functional languages; the Visitor pattern achieves the same end.
But what about when you must frequently add operations *and* types?
* and pattern matching in functional languages; the Visitor pattern achieves the same end.
First, off Ravi your statement that the Employee must know about every reports is incorrect. The Employee hierarchy only knows the visitor not all of them. You could use this visitor for many reasons, one being reports. The alternative to using the visitor is to add a new method for, possibly, each report to Employee. In my opinion this creates a sloppy Employee interface with methods that are not part of Emplyees responsibility.
I have found this to be the best explanation of visitor when teaching people about patterns.
"The Visitor pattern is useful when you want to gain functionality from a hierarchy of classes that is not directly related to the classes responsibilities." Michael Bahl 1999
Gof outline some problems with the pattern but I really like it to keep my class' free from alot of methods that don't fit with the class' responsibility.
You may notice I am pretty convince keeping a class/hierchies responsibility clear is the key to design.
my 3 cents
I have found this to be the best explanation of visitor when teaching people about patterns.
"The Visitor pattern is useful when you want to gain functionality from a hierarchy of classes that is not directly related to the classes responsibilities." Michael Bahl 1999
Gof outline some problems with the pattern but I really like it to keep my class' free from alot of methods that don't fit with the class' responsibility.
You may notice I am pretty convince keeping a class/hierchies responsibility clear is the key to design.
my 3 cents
Add Child Page to IuseVisitor