This is old and proven code which isn't needing a refactoring at this time,

What has the fact that is proven code to do with it. That's not an argument in this case imo. The code as is is confusing and imo counter intuitive. Each time I come across it I have to spend extra time understanding it, because it does not meet expectations.

in fact we have large parts to rewrite for Lucene4 but I'd hope at least this infrastructure would stay stable.

I would like to see this addressed asap. Latest for Search 5. But I agree, it is of course not a bug. I forgot to change the type when creating the issue.

Also I'm not convinced this is not a Visitor: look at the pattern, not the methods names:

I am. How do you explain then that there is even a subclass FlushStrategySelector which implements this "visitor"? In the code we mix freely the terms delegate, strategy and visitor. All of which are design patterns. These patterns are there so that if I hear one of these names I can assume a certain type of design/architecture. If this is not met people are confused and it takes even longer to understand the code, compared to use any other arbitrary name.

I'm not aware of a rule that says the Visitor needs a "visit" named method, in fact you can have multiple methods to take advantage of it.

It's about managing expectations. in most cases you would expect a visit, but it could be names differently. I am not so sure whether multiple methods makes sense. Would that not better solved by distinct visitors? How does a node accepting a visitor know which if the "visit" method is should call? Secondly, it is imo uncommon to return something from the visit method. Often the visitor accumulates state from visiting the object structure and in the end you call visitor.getFoo(). This is not the case here.

Call it as you like, it's a double dispatch.. I'll try to refresh my patterns naming skills

Then maybe we should call it WorkDispatcher. Still imo the strategy pattern (where we set the appropriate strategy on the work instance) would be a good fit.

We can discuss changing names or clarify javadocs if you really want to but I'm negative on creating objects on this hot path, can your context idea be reformulated to be static?

You always argue with performance concerns first. As far as I can tell my strategy approach would not create any more objects than the current approach. Also I believe in first first rule of program optimization: Don’t do it. Aim for simple, readable code first.

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