gsmiller commented on a change in pull request #578:
URL: https://github.com/apache/lucene/pull/578#discussion_r779174169
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -74,11 +74,6 @@ protected boolean useHashTable(FacetsCollector fc,
TaxonomyReader taxoReader) {
return sumTotalHits < maxDoc / 10;
}
- /** Increment the count for this ordinal by 1. */
- protected void increment(int ordinal) {
Review comment:
I think we ought to leave this in. Removing it is a
backwards-compatibility concern since it's possible (likely) that users have
sub-classed `intTaxonomyFacets` and rely on this. I think it's also nice to
keep in general so that sub-classes can rely on this instead of having to
manage direct access to the dense/sparse structures if they choose to.
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -32,9 +32,9 @@
public abstract class IntTaxonomyFacets extends TaxonomyFacets {
/** Per-ordinal value. */
- private final int[] values;
+ final int[] values;
Review comment:
I'd suggest adding some javadoc to these two fields mentioning that
they're exposed for sub-classes that want "expert" functionality (e.g., direct
access along with the burden of knowing which one is being used). The doc could
point users to `#increment` and `#getValue` for more typical use-cases that
don't want the burden of directly accessing these.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
