After looking at the code a bit more I am not even sure this is a visitor at all. Really the interaction of the visitor should be:
node.accept(vistor);
// where the accept method basically calls
vistor.accept(vistor);
// the vistor is then doing something and in the end I go
visitor.giveMe();
This is not the case in WorkVisitor. There is no visit method, but rather a getDelegate which returns something.
The more I think about this, the more I think that what we really have is a Strategy - http://java.dzone.com/articles/design-patterns-strategy. The LuceneWork instances should implement methods like performStreamOperation, performWork and flush, however, the execution should always be passed on to a strategy which is set depending on the context. Something like:
// in streaming context
work.setStreamingStrategy(new StreamingSelectionStrategy());
work.performStreamOperation(...)
...
I think this would be much easier to understand with a lot less indirection. The strategy would probably still need a switch to distinguish between the different work types, but it still is worth it imo.
Funny enough we have already in the existing code the following class definition:
privatestatic class FlushStrategySelector implements WorkVisitor<FlushStrategyNeed>
So here the visitor becomes already a strategy. Somehow we need to sort this out.
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
After looking at the code a bit more I am not even sure this is a visitor at all. Really the interaction of the visitor should be:
This is not the case in WorkVisitor. There is no visit method, but rather a getDelegate which returns something.
The more I think about this, the more I think that what we really have is a Strategy - http://java.dzone.com/articles/design-patterns-strategy. The LuceneWork instances should implement methods like performStreamOperation, performWork and flush, however, the execution should always be passed on to a strategy which is set depending on the context. Something like:
I think this would be much easier to understand with a lot less indirection. The strategy would probably still need a switch to distinguish between the different work types, but it still is worth it imo.
Funny enough we have already in the existing code the following class definition:
So here the visitor becomes already a strategy. Somehow we need to sort this out.