stefanvodita commented on code in PR #13414: URL: https://github.com/apache/lucene/pull/13414#discussion_r1617221613
########## lucene/facet/src/java/org/apache/lucene/facet/LabelAndValue.java: ########## @@ -24,10 +24,21 @@ public final class LabelAndValue { /** Value associated with this label. */ public final Number value; - /** Sole constructor. */ + /** Number of occurrences for this label. */ + public final int count; + + /** Constructor with unspecified count, we assume the value is a count. */ public LabelAndValue(String label, Number value) { this.label = label; this.value = value; + this.count = value.intValue(); Review Comment: I think that would be reasonable too. The reason I set it to `value` is because with our existing`Facets` implementations it would give us correct `count`. That's not necessarily true for someone's custom implementation, which is where setting a default value would be more correct. On the other hand, if someone has custom `Facets`, they wouldn't expect the `count` to just be there if they hadn't computed it. Overall, I like setting `count` to `value` here. I'm much less happy with the duplication aspect of this change. -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org