lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless (Commented) (JIRA)" <>
Subject [jira] [Commented] (LUCENE-3778) Create a grouping convenience class
Date Mon, 27 Feb 2012 11:35:49 GMT


Michael McCandless commented on LUCENE-3778:

I really dislike the cancerous chaining (all setters returning
{{this}}): it's poor API design because it creates unnecessary
ambiguity on how to consume it.  It amounts to a denial-of-service
attack on the devs who consume our APIs....  We should strive to have
less, not more, ambiguity in all of our APIs.

But, since others seem to love it, as a compromise, can you write all
consumption of the cancerous chaining as if the methods did not chain?
Ie minimize the cancer: contain it to the API definition, alone.

The litmus test is then simple: if I were to change all methods to
return void instead, everything should compile / tests should pass.

Otherwise, the patch looks great!

One can actually use GroupingSearch in a shard'd env, on each shard,
right?  It's just that then you merge them like normal on the front
end (ie, TopGroups.merge).  Is that the only reason for the "... in a
non distributed environment" javadoc warning?

I think the jdocs for each ctor should explain what kind of grouping
impl will be used (ie, the ctor taking Filter groupEndDocs uses
single-pass block collector, and requires you indexed doc blocks).

Maybe the ctor should take docValuesType / diskResidentValues, instead
of setters to change it?  Ie, so that you are stating up front what
source to group by (DocValues, FC (Term), function, block).

Maybe you should pass the groupSort, groupsOffset, groupsLimit to the
search method (instead of setters)?

> Create a grouping convenience class
> -----------------------------------
>                 Key: LUCENE-3778
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: modules/grouping
>            Reporter: Martijn van Groningen
>         Attachments: LUCENE-3778.patch
> Currently the grouping module has many collector classes with a lot of different options
per class. I think it would be a good idea to have a GroupUtil (Or another name?) convenience
class. I think this could be a builder, because of the many options (sort,sortWithinGroup,groupOffset,groupCount
and more) and implementations (term/dv/function) grouping has.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:!default.jspa
For more information on JIRA, see:


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message