<style>
/* Changing the layout to use less space for mobiles */
@media screen and (max-device-width: 480px), screen and (-webkit-min-device-pixel-ratio: 2) {
#email-body { min-width: 30em !important; }
#email-page { padding: 8px !important; }
#email-banner { padding: 8px 8px 0 8px !important; }
#email-avatar { margin: 1px 8px 8px 0 !important; padding: 0 !important; }
#email-fields { padding: 0 8px 8px 8px !important; }
#email-gutter { width: 0 !important; }
}
</style>
<div id="email-body">
<table id="email-wrap" align="center" border="0" cellpadding="0" cellspacing="0" style="background-color:#f0f0f0;color:#000000;width:100%;">
<tr valign="top">
<td id="email-page" style="padding:16px !important;">
<table align="center" border="0" cellpadding="0" cellspacing="0" style="background-color:#ffffff;border:1px solid #bbbbbb;color:#000000;width:100%;">
<tr valign="top">
<td bgcolor="#3e4c4e" style="background-color:#3e4c4e;color:#ffffff;font-family:Arial,FreeSans,Helvetica,sans-serif;font-size:12px;line-height:1;"><img src="https://www.jboss.org/dms/hibernate/images/jira/jiraheader_hibernate.png" alt="" style="vertical-align:top;" /></td>
</tr><tr valign="top">
<td id="email-banner" style="padding:32px 32px 0 32px;">
<table align="left" border="0" cellpadding="0" cellspacing="0" width="100%" style="width:100%;">
<tr valign="top">
<td style="color:#505050;font-family:Arial,FreeSans,Helvetica,sans-serif;padding:0;">
<img id="email-avatar" src="https://secure.gravatar.com/avatar/12df4da7e3351be801bc16b66caf8038?d=mm&s=48" alt="" height="48" width="48" border="0" align="left" style="padding:0;margin: 0 16px 16px 0;" />
<div id="email-action" style="padding: 0 0 8px 0;font-size:12px;line-height:18px;">
<a class="user-hover" rel="hardy.ferentschik" id="email_hardy.ferentschik" href="https://hibernate.atlassian.net/secure/ViewProfile.jspa?name=hardy.ferentschik" style="color:#6c797f;">Hardy Ferentschik</a>
commented on <img src="https://hibernate.atlassian.net/images/icons/issuetypes/improvement.png" height="16" width="16" border="0" align="absmiddle" alt="Improvement"> <a style='color:#6c797f;text-decoration:none;' href='https://hibernate.atlassian.net/browse/HSEARCH-1356'>HSEARCH-1356</a>
</div>
<div id="email-summary" style="font-size:16px;line-height:20px;padding:2px 0 16px 0;">
<a style='color:#6c797f;text-decoration:none;' href='https://hibernate.atlassian.net/browse/HSEARCH-1356'><strong>Refactor WorkVisitor to match more the expectations on a Visitor pattern</strong></a>
</div>
</td>
</tr>
</table>
</td>
</tr>
<tr valign="top">
<td id="email-fields" style="padding:0 32px 32px 32px;">
<table border="0" cellpadding="0" cellspacing="0" style="padding:0;text-align:left;width:100%;" width="100%">
<tr valign="top">
<td id="email-gutter" style="width:64px;white-space:nowrap;"></td>
<td>
<table border="0" cellpadding="0" cellspacing="0" width="100%">
<tr valign="top">
<td colspan="2" style="color:#000000;font-family:Arial,FreeSans,Helvetica,sans-serif;font-size:12px;padding:0 0 16px 0;width:100%;">
<div class="comment-block" style="background-color:#edf5ff;border:1px solid #dddddd;color:#000000;padding:12px;"><blockquote>
<p>This is old and proven code which isn't needing a refactoring at this time,</p></blockquote>
<p>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. </p>
<blockquote>
<p>in fact we have large parts to rewrite for Lucene4 but I'd hope at least this infrastructure would stay stable.</p></blockquote>
<p>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.</p>
<blockquote>
<p>Also I'm not convinced this is not a Visitor: look at the pattern, not the methods names:</p></blockquote>
<p>I am. How do you explain then that there is even a subclass <em>FlushStrategySelector</em> 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. </p>
<blockquote>
<p>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.</p></blockquote>
<p>It's about managing expectations. in most cases you would expect a <em>visit</em>, 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 <em>visitor.getFoo()</em>. This is not the case here.</p>
<blockquote>
<p>Call it as you like, it's a double dispatch.. I'll try to refresh my patterns naming skills</p></blockquote>
<p>Then maybe we should call it <em>WorkDispatcher</em>. Still imo the strategy pattern (where we set the appropriate strategy on the work instance) would be a good fit.</p>
<blockquote>
<p>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?</p></blockquote>
<p>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.</p></div>
<div style="color:#505050;padding:4px 0 0 0;"> </div>
</td>
</tr>
</table>
</td>
</tr>
</table>
</td>
</tr>
</table>
</td><!-- End #email-page -->
</tr>
<tr valign="top">
<td style="color:#505050;font-family:Arial,FreeSans,Helvetica,sans-serif;font-size:10px;line-height:14px;padding: 0 16px 16px 16px;text-align:center;">
This message is automatically generated by JIRA.<br />
If you think it was sent incorrectly, please contact your JIRA administrators<br />
For more information on JIRA, see: <a style='color:#6c797f;' href='http://www.atlassian.com/software/jira'>http://www.atlassian.com/software/jira</a>
</td>
</tr>
</table><!-- End #email-wrap -->
</div><!-- End #email-body -->