[GitHub] [lucene] Yuti-G commented on a change in pull request #751: LUCENE-10467: Throws IllegalArgumentException for getAllDims and getTopChildren if topN <= 0

[GitBox <gi...@apache.org>]

Yuti-G commented on a change in pull request #751:
URL: https://github.com/apache/lucene/pull/751#discussion_r830443213



##########
File path: lucene/facet/src/java/org/apache/lucene/facet/LongValueFacetCounts.java
##########
@@ -348,6 +348,9 @@ private void increment(long value) {
 
   @Override
   public FacetResult getTopChildren(int topN, String dim, String... path) {
+    if (topN <= 0) {

Review comment:
       Hi @mikemccand, thank you so much for reviewing my PR! 
   
   I tried to factor out a package-private helper method in Facets, but there are subclasses under the subpackage of facet, e.g., `package org.apache.lucene.facet.taxonomy;` that cannot call this method. And therefore, I made the method protected in order to be called in all these places in my new commit. Please let me know how you think. Thanks!
   
   ```
     protected static void validateTopN(int topN) {
       if (topN <= 0) {
         throw new IllegalArgumentException("topN must be > 0 (got: " + topN + ")");
       }
     }
   ```




-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org