[GitHub] [lucene] jpountz commented on a diff in pull request #12040: Minor refactoring and cleanup to BooleanQuery code

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

jpountz commented on code in PR #12040:
URL: https://github.com/apache/lucene/pull/12040#discussion_r1057563242


##########
lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java:
##########
@@ -204,11 +204,11 @@ BooleanQuery rewriteNoScoring(IndexSearcher indexSearcher) throws IOException {
     for (BooleanClause clause : clauses) {
       Query query = clause.getQuery();
       Query rewritten = new ConstantScoreQuery(query).rewrite(indexSearcher);
-      if (rewritten instanceof ConstantScoreQuery) {
-        rewritten = ((ConstantScoreQuery) rewritten).getQuery();
+      if (rewritten instanceof ConstantScoreQuery constantScoreQuery) {
+        rewritten = constantScoreQuery.getQuery();
       }
       BooleanClause.Occur occur = clause.getOccur();
-      if (occur == Occur.SHOULD && keepShould == false) {
+      if (occur == Occur.SHOULD && !keepShould) {

Review Comment:
   we use `== false` intentionally across the codebase for better readability, can you keep it and other places in this class too?



##########
lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java:
##########
@@ -346,21 +338,15 @@ public Query rewrite(IndexSearcher indexSearcher) throws IOException {
     if (clauseSets.get(Occur.FILTER).size() > 0) {
       final Set<Query> filters = new HashSet<>(clauseSets.get(Occur.FILTER));
       boolean modified = false;
-      if (filters.size() > 1 || clauseSets.get(Occur.MUST).isEmpty() == false) {
+      if (filters.size() > 1 || !clauseSets.get(Occur.MUST).isEmpty()) {
         modified = filters.remove(new MatchAllDocsQuery());
       }
       modified |= filters.removeAll(clauseSets.get(Occur.MUST));
       if (modified) {
         BooleanQuery.Builder builder = new BooleanQuery.Builder();
         builder.setMinimumNumberShouldMatch(getMinimumNumberShouldMatch());
-        for (BooleanClause clause : clauses) {
-          if (clause.getOccur() != Occur.FILTER) {
-            builder.add(clause);
-          }
-        }
-        for (Query filter : filters) {
-          builder.add(filter, Occur.FILTER);
-        }
+        clauses.stream().filter(clause -> clause.getOccur() != Occur.FILTER).forEach(builder::add);
+        filters.forEach(filter -> builder.add(filter, Occur.FILTER));

Review Comment:
   I think using streams above to compute `clauseCount` is fine as it makes the code less verbose while remaining easy to read. I'm less sure about this change: it's shorter, but it also reads harder for me.



##########
lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java:
##########
@@ -408,19 +393,15 @@ public Query rewrite(IndexSearcher indexSearcher) throws IOException {
       if (shouldClauses.size() != clauseSets.get(Occur.SHOULD).size()) {
         BooleanQuery.Builder builder =
             new BooleanQuery.Builder().setMinimumNumberShouldMatch(minimumNumberShouldMatch);
-        for (Map.Entry<Query, Double> entry : shouldClauses.entrySet()) {
-          Query query = entry.getKey();
-          float boost = entry.getValue().floatValue();
-          if (boost != 1f) {
-            query = new BoostQuery(query, boost);
-          }
-          builder.add(query, Occur.SHOULD);
-        }
-        for (BooleanClause clause : clauses) {
-          if (clause.getOccur() != Occur.SHOULD) {
-            builder.add(clause);
-          }
-        }
+        shouldClauses.forEach(
+            (query, value) -> {
+              float boost = value.floatValue();
+              if (boost != 1f) {
+                query = new BoostQuery(query, boost);
+              }
+              builder.add(query, Occur.SHOULD);
+            });
+        clauses.stream().filter(clause -> clause.getOccur() != Occur.SHOULD).forEach(builder::add);

Review Comment:
   Likewise here, streams don't make the code easier to read?



-- 
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